[Clang] Reapply CWG2369 "Ordering between constraints and substitution" #122423
+289
−62
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…substitution"" (llvm#122130) This reverts commit 3972ed5.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesThe previous approach broke code generation for the MS ABI due to an unintended code path during constraint substitution. This time we address the issue by inspecting the CodeSynthesisContexts and thereby avoiding that code path. This reapplies 96eced6. Patch is 41.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/122423.diff 25 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a41f16f6dc8c9b..87d9a335763e31 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13062,6 +13062,7 @@ class Sema final : public SemaBase {
///
/// \param SkipForSpecialization when specified, any template specializations
/// in a traversal would be ignored.
+ ///
/// \param ForDefaultArgumentSubstitution indicates we should continue looking
/// when encountering a specialized member function template, rather than
/// returning immediately.
@@ -13073,6 +13074,17 @@ class Sema final : public SemaBase {
bool SkipForSpecialization = false,
bool ForDefaultArgumentSubstitution = false);
+ /// Apart from storing the result to \p Result, this behaves the same as
+ /// another overload.
+ void getTemplateInstantiationArgs(
+ MultiLevelTemplateArgumentList &Result, const NamedDecl *D,
+ const DeclContext *DC = nullptr, bool Final = false,
+ std::optional<ArrayRef<TemplateArgument>> Innermost = std::nullopt,
+ bool RelativeToPrimary = false, const FunctionDecl *Pattern = nullptr,
+ bool ForConstraintInstantiation = false,
+ bool SkipForSpecialization = false,
+ bool ForDefaultArgumentSubstitution = false);
+
/// RAII object to handle the state changes required to synthesize
/// a function body.
class SynthesizedFunctionScope {
@@ -13169,6 +13181,10 @@ class Sema final : public SemaBase {
// FIXME: Should we have a similar limit for other forms of synthesis?
unsigned NonInstantiationEntries;
+ /// The number of \p CodeSynthesisContexts that are not constraint
+ /// substitution.
+ unsigned NonConstraintSubstitutionEntries;
+
/// The depth of the context stack at the point when the most recent
/// error or warning was produced.
///
@@ -13342,7 +13358,7 @@ class Sema final : public SemaBase {
ExprResult
SubstConstraintExpr(Expr *E,
const MultiLevelTemplateArgumentList &TemplateArgs);
- // Unlike the above, this does not evaluates constraints.
+ // Unlike the above, this does not evaluate constraints.
ExprResult SubstConstraintExprWithoutSatisfaction(
Expr *E, const MultiLevelTemplateArgumentList &TemplateArgs);
@@ -13491,6 +13507,11 @@ class Sema final : public SemaBase {
return CodeSynthesisContexts.size() > NonInstantiationEntries;
}
+ /// Determine whether we are currently performing constraint substitution.
+ bool inConstraintSubstitution() const {
+ return CodeSynthesisContexts.size() > NonConstraintSubstitutionEntries;
+ }
+
using EntityPrinter = llvm::function_ref<void(llvm::raw_ostream &)>;
/// \brief create a Requirement::SubstitutionDiagnostic with only a
@@ -14463,10 +14484,10 @@ class Sema final : public SemaBase {
const MultiLevelTemplateArgumentList &TemplateArgs,
SourceRange TemplateIDRange);
- bool CheckInstantiatedFunctionTemplateConstraints(
- SourceLocation PointOfInstantiation, FunctionDecl *Decl,
- ArrayRef<TemplateArgument> TemplateArgs,
- ConstraintSatisfaction &Satisfaction);
+ bool CheckFunctionTemplateConstraints(SourceLocation PointOfInstantiation,
+ FunctionDecl *Decl,
+ ArrayRef<TemplateArgument> TemplateArgs,
+ ConstraintSatisfaction &Satisfaction);
/// \brief Emit diagnostics explaining why a constraint expression was deemed
/// unsatisfied.
diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h
index 9800f75f676aaf..59a0575ca98036 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -522,6 +522,12 @@ enum class TemplateSubstitutionKind : char {
llvm::PointerUnion<Decl *, DeclArgumentPack *> *
findInstantiationOf(const Decl *D);
+ /// Similar to \p findInstantiationOf(), but it wouldn't assert if the
+ /// instantiation was not found within the current instantiation scope. This
+ /// is helpful for on-demand declaration instantiation.
+ llvm::PointerUnion<Decl *, DeclArgumentPack *> *
+ findInstantiationUnsafe(const Decl *D);
+
void InstantiatedLocal(const Decl *D, Decl *Inst);
void InstantiatedLocalPackArg(const Decl *D, VarDecl *Inst);
void MakeInstantiatedLocalArgPack(const Decl *D);
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index abb46d3a84e74e..b7b154e55b3db9 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -263,7 +263,8 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
TyposCorrected(0), IsBuildingRecoveryCallExpr(false), NumSFINAEErrors(0),
AccessCheckingSFINAE(false), CurrentInstantiationScope(nullptr),
InNonInstantiationSFINAEContext(false), NonInstantiationEntries(0),
- ArgumentPackSubstitutionIndex(-1), SatisfactionCache(Context) {
+ NonConstraintSubstitutionEntries(0), ArgumentPackSubstitutionIndex(-1),
+ SatisfactionCache(Context) {
assert(pp.TUKind == TUKind);
TUScope = nullptr;
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 539de00bd104f5..10f4920a761f3c 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -846,7 +846,7 @@ bool Sema::CheckFunctionConstraints(const FunctionDecl *FD,
bool ForOverloadResolution) {
// Don't check constraints if the function is dependent. Also don't check if
// this is a function template specialization, as the call to
- // CheckinstantiatedFunctionTemplateConstraints after this will check it
+ // CheckFunctionTemplateConstraints after this will check it
// better.
if (FD->isDependentContext() ||
FD->getTemplatedKind() ==
@@ -1111,12 +1111,55 @@ bool Sema::EnsureTemplateArgumentListConstraints(
return false;
}
-bool Sema::CheckInstantiatedFunctionTemplateConstraints(
+static bool CheckFunctionConstraintsWithoutInstantiation(
+ Sema &SemaRef, SourceLocation PointOfInstantiation,
+ FunctionTemplateDecl *Template, ArrayRef<TemplateArgument> TemplateArgs,
+ ConstraintSatisfaction &Satisfaction) {
+ SmallVector<const Expr *, 3> TemplateAC;
+ Template->getAssociatedConstraints(TemplateAC);
+ if (TemplateAC.empty()) {
+ Satisfaction.IsSatisfied = true;
+ return false;
+ }
+
+ LocalInstantiationScope Scope(SemaRef);
+
+ FunctionDecl *FD = Template->getTemplatedDecl();
+ // Collect the list of template arguments relative to the 'primary'
+ // template. We need the entire list, since the constraint is completely
+ // uninstantiated at this point.
+
+ // FIXME: Add TemplateArgs through the 'Innermost' parameter once
+ // the refactoring of getTemplateInstantiationArgs() relands.
+ MultiLevelTemplateArgumentList MLTAL;
+ MLTAL.addOuterTemplateArguments(Template, std::nullopt, /*Final=*/false);
+ SemaRef.getTemplateInstantiationArgs(
+ MLTAL, /*D=*/FD, FD,
+ /*Final=*/false, /*Innermost=*/std::nullopt, /*RelativeToPrimary=*/true,
+ /*Pattern=*/nullptr, /*ForConstraintInstantiation=*/true);
+ MLTAL.replaceInnermostTemplateArguments(Template, TemplateArgs);
+
+ Sema::ContextRAII SavedContext(SemaRef, FD);
+ std::optional<Sema::CXXThisScopeRAII> ThisScope;
+ if (auto *Method = dyn_cast<CXXMethodDecl>(FD))
+ ThisScope.emplace(SemaRef, /*Record=*/Method->getParent(),
+ /*ThisQuals=*/Method->getMethodQualifiers());
+ return SemaRef.CheckConstraintSatisfaction(
+ Template, TemplateAC, MLTAL, PointOfInstantiation, Satisfaction);
+}
+
+bool Sema::CheckFunctionTemplateConstraints(
SourceLocation PointOfInstantiation, FunctionDecl *Decl,
ArrayRef<TemplateArgument> TemplateArgs,
ConstraintSatisfaction &Satisfaction) {
// In most cases we're not going to have constraints, so check for that first.
FunctionTemplateDecl *Template = Decl->getPrimaryTemplate();
+
+ if (!Template)
+ return ::CheckFunctionConstraintsWithoutInstantiation(
+ *this, PointOfInstantiation, Decl->getDescribedFunctionTemplate(),
+ TemplateArgs, Satisfaction);
+
// Note - code synthesis context for the constraints check is created
// inside CheckConstraintsSatisfaction.
SmallVector<const Expr *, 3> TemplateAC;
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 1c1f6e30ab7b83..acd1151184e42f 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -3936,18 +3936,6 @@ TemplateDeductionResult Sema::FinishTemplateArgumentDeduction(
Result != TemplateDeductionResult::Success)
return Result;
- // C++ [temp.deduct.call]p10: [DR1391]
- // If deduction succeeds for all parameters that contain
- // template-parameters that participate in template argument deduction,
- // and all template arguments are explicitly specified, deduced, or
- // obtained from default template arguments, remaining parameters are then
- // compared with the corresponding arguments. For each remaining parameter
- // P with a type that was non-dependent before substitution of any
- // explicitly-specified template arguments, if the corresponding argument
- // A cannot be implicitly converted to P, deduction fails.
- if (CheckNonDependent())
- return TemplateDeductionResult::NonDependentConversionFailure;
-
// Form the template argument list from the deduced template arguments.
TemplateArgumentList *SugaredDeducedArgumentList =
TemplateArgumentList::CreateCopy(Context, SugaredBuilder);
@@ -3977,6 +3965,39 @@ TemplateDeductionResult Sema::FinishTemplateArgumentDeduction(
FD = const_cast<FunctionDecl *>(FDFriend);
Owner = FD->getLexicalDeclContext();
}
+ // C++20 [temp.deduct.general]p5: [CWG2369]
+ // If the function template has associated constraints, those constraints
+ // are checked for satisfaction. If the constraints are not satisfied, type
+ // deduction fails.
+ //
+ // FIXME: We haven't implemented CWG2369 for lambdas yet, because we need
+ // to figure out how to instantiate lambda captures to the scope without
+ // first instantiating the lambda.
+ bool IsLambda = isLambdaCallOperator(FD) || isLambdaConversionOperator(FD);
+ if (!IsLambda && !IsIncomplete) {
+ if (CheckFunctionTemplateConstraints(
+ Info.getLocation(),
+ FunctionTemplate->getCanonicalDecl()->getTemplatedDecl(),
+ CanonicalBuilder, Info.AssociatedConstraintsSatisfaction))
+ return TemplateDeductionResult::MiscellaneousDeductionFailure;
+ if (!Info.AssociatedConstraintsSatisfaction.IsSatisfied) {
+ Info.reset(Info.takeSugared(),
+ TemplateArgumentList::CreateCopy(Context, CanonicalBuilder));
+ return TemplateDeductionResult::ConstraintsNotSatisfied;
+ }
+ }
+ // C++ [temp.deduct.call]p10: [CWG1391]
+ // If deduction succeeds for all parameters that contain
+ // template-parameters that participate in template argument deduction,
+ // and all template arguments are explicitly specified, deduced, or
+ // obtained from default template arguments, remaining parameters are then
+ // compared with the corresponding arguments. For each remaining parameter
+ // P with a type that was non-dependent before substitution of any
+ // explicitly-specified template arguments, if the corresponding argument
+ // A cannot be implicitly converted to P, deduction fails.
+ if (CheckNonDependent())
+ return TemplateDeductionResult::NonDependentConversionFailure;
+
MultiLevelTemplateArgumentList SubstArgs(
FunctionTemplate, CanonicalDeducedArgumentList->asArray(),
/*Final=*/false);
@@ -4011,8 +4032,8 @@ TemplateDeductionResult Sema::FinishTemplateArgumentDeduction(
// ([temp.constr.decl]), those constraints are checked for satisfaction
// ([temp.constr.constr]). If the constraints are not satisfied, type
// deduction fails.
- if (!IsIncomplete) {
- if (CheckInstantiatedFunctionTemplateConstraints(
+ if (IsLambda && !IsIncomplete) {
+ if (CheckFunctionTemplateConstraints(
Info.getLocation(), Specialization, CanonicalBuilder,
Info.AssociatedConstraintsSatisfaction))
return TemplateDeductionResult::MiscellaneousDeductionFailure;
diff --git a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
index d42c3765aa534f..5d6c11a75303df 100644
--- a/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
+++ b/clang/lib/Sema/SemaTemplateDeductionGuide.cpp
@@ -902,10 +902,12 @@ Expr *buildIsDeducibleConstraint(Sema &SemaRef,
Context.getTrivialTypeSourceInfo(
Context.getDeducedTemplateSpecializationType(
TemplateName(AliasTemplate), /*DeducedType=*/QualType(),
- /*IsDependent=*/true)), // template specialization type whose
- // arguments will be deduced.
+ /*IsDependent=*/true),
+ AliasTemplate->getLocation()), // template specialization type whose
+ // arguments will be deduced.
Context.getTrivialTypeSourceInfo(
- ReturnType), // type from which template arguments are deduced.
+ ReturnType, AliasTemplate->getLocation()), // type from which template
+ // arguments are deduced.
};
return TypeTraitExpr::Create(
Context, Context.getLogicalOperationType(), AliasTemplate->getLocation(),
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index fb0f38df62a744..6db6d52e27d6dc 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -475,6 +475,21 @@ MultiLevelTemplateArgumentList Sema::getTemplateInstantiationArgs(
assert((ND || DC) && "Can't find arguments for a decl if one isn't provided");
// Accumulate the set of template argument lists in this structure.
MultiLevelTemplateArgumentList Result;
+ getTemplateInstantiationArgs(
+ Result, ND, DC, Final, Innermost, RelativeToPrimary, Pattern,
+ ForConstraintInstantiation, SkipForSpecialization,
+ ForDefaultArgumentSubstitution);
+ return Result;
+}
+
+void Sema::getTemplateInstantiationArgs(
+ MultiLevelTemplateArgumentList &Result, const NamedDecl *ND,
+ const DeclContext *DC, bool Final,
+ std::optional<ArrayRef<TemplateArgument>> Innermost, bool RelativeToPrimary,
+ const FunctionDecl *Pattern, bool ForConstraintInstantiation,
+ bool SkipForSpecialization, bool ForDefaultArgumentSubstitution) {
+ assert((ND || DC) && "Can't find arguments for a decl if one isn't provided");
+ // Accumulate the set of template argument lists in this structure.
using namespace TemplateInstArgsHelpers;
const Decl *CurDecl = ND;
@@ -535,14 +550,12 @@ MultiLevelTemplateArgumentList Sema::getTemplateInstantiationArgs(
}
if (R.IsDone)
- return Result;
+ return;
if (R.ClearRelativeToPrimary)
RelativeToPrimary = false;
assert(R.NextDecl);
CurDecl = R.NextDecl;
}
-
- return Result;
}
bool Sema::CodeSynthesisContext::isInstantiationRecord() const {
@@ -815,6 +828,9 @@ void Sema::pushCodeSynthesisContext(CodeSynthesisContext Ctx) {
if (!Ctx.isInstantiationRecord())
++NonInstantiationEntries;
+ if (Ctx.Kind != CodeSynthesisContext::ConstraintSubstitution)
+ ++NonConstraintSubstitutionEntries;
+
// Check to see if we're low on stack space. We can't do anything about this
// from here, but we can at least warn the user.
StackHandler.warnOnStackNearlyExhausted(Ctx.PointOfInstantiation);
@@ -827,6 +843,11 @@ void Sema::popCodeSynthesisContext() {
--NonInstantiationEntries;
}
+ if (Active.Kind != CodeSynthesisContext::ConstraintSubstitution) {
+ assert(NonConstraintSubstitutionEntries > 0);
+ --NonConstraintSubstitutionEntries;
+ }
+
InNonInstantiationSFINAEContext = Active.SavedInNonInstantiationSFINAEContext;
// Name lookup no longer looks in this template's defining module.
@@ -1349,6 +1370,12 @@ namespace {
// Whether an incomplete substituion should be treated as an error.
bool BailOutOnIncomplete;
+ private:
+ // CWG2770: Function parameters should be instantiated when they are
+ // needed by a satisfaction check of an atomic constraint or
+ // (recursively) by another function parameter.
+ bool maybeInstantiateFunctionParameterToScope(ParmVarDecl *OldParm);
+
public:
typedef TreeTransform<TemplateInstantiator> inherited;
@@ -1405,12 +1432,20 @@ namespace {
ArrayRef<UnexpandedParameterPack> Unexpanded,
bool &ShouldExpand, bool &RetainExpansion,
std::optional<unsigned> &NumExpansions) {
- return getSema().CheckParameterPacksForExpansion(EllipsisLoc,
- PatternRange, Unexpanded,
- TemplateArgs,
- ShouldExpand,
- RetainExpansion,
- NumExpansions);
+ if (SemaRef.CurrentInstantiationScope &&
+ SemaRef.inConstraintSubstitution()) {
+ for (UnexpandedParameterPack ParmPack : Unexpanded) {
+ NamedDecl *VD = ParmPack.first.dyn_cast<NamedDecl *>();
+ if (!isa_and_present<ParmVarDecl>(VD))
+ continue;
+ if (maybeInstantiateFunctionParameterToScope(cast<ParmVarDecl>(VD)))
+ return true;
+ }
+ }
+
+ return getSema().CheckParameterPacksForExpansion(
+ EllipsisLoc, PatternRange, Unexpanded, TemplateArgs, ShouldExpand,
+ RetainExpansion, NumExpansions);
}
void ExpandingFunctionParameterPack(ParmVarDecl *Pack) {
@@ -1911,9 +1946,62 @@ Decl *TemplateInstantiator::TransformDecl(SourceLocation Loc, Decl *D) {
// template parameter.
}
+ if (SemaRef.CurrentInstantiationScope) {
+ if (SemaRef.inConstraintSubstitution() && isa<ParmVarDecl>(D) &&
+ maybeInstantiateFunctionParameterToScope(cast<ParmVarDecl>(D)))
+ return nullptr;
+ }
+
return SemaRef.FindInstantiatedDecl(Loc, cast<NamedDecl>(D), TemplateArgs);
}
+bool TemplateInstantiator::maybeInstantiateFunctionParameterToScope(
+ ParmVarDecl *OldParm) {
+ if (SemaRef.CurrentInstantiationScope->findInstantiationUnsafe(OldParm))
+ return false;
+ // We're instantiating a function parameter whose associated function template
+ // has not been instantiated at this point for constraint evaluation, so make
+ // sure the instantiated parameters are owned by a function declaration such
+ // that they can be correctly 'captured' in tryCaptureVariable().
+ Sema::ContextRAII Context(SemaRef, OldParm->getDeclContext());
+
+ if (!OldParm->isParameterPack())
+ return !TransformFunctionTypeParam(OldParm, /*indexAdjustment=*/0,
+ /*NumExpansions=*/std::nullopt,
+ /*ExpectParameterPack=*/false);
+
+ SmallVector<UnexpandedParameterPack, 2> Unexpanded;
+
+ // Find the parameter packs that could be expanded.
+ TypeLoc TL = OldParm->getTypeSourceInfo()->getTypeLoc();
+ PackExpansionTypeLoc ExpansionTL = TL.castAs<PackExpansionTypeLoc>();
+ TypeLoc Pattern = ExpansionTL.getPatternLoc();
+ SemaRef.collectUnexpandedParameterPacks(Pattern, Unexpanded);
+ assert(!Unexpanded.empty() && "Pack expansion without parameter packs?");
+
+ bool ShouldExpand = false;
+ bool RetainExpansion = false;
+ std::optional<unsigned> OrigNumExpansions =
+ ExpansionTL.getTypePtr()->getNumExpansions();
+ std::optional<unsigned> NumExpansions = OrigNumExpansions;
+ if (TryExpandParameterPacks(ExpansionTL.getEllipsisLoc(),
+ Pattern.getSourceRange(), Unexpanded,
+ ShouldExpand, RetainExpansion, NumExpansions))
+ return true;
+
+ assert(ShouldExpand && !RetainExpansion &&
+ "Shouldn't preserve pack expansion when evaluating constraints");
+ ExpandingFunctionParameterPack(OldParm);
+ ...
[truncated]
|
zyn0217
force-pushed
the
cwg-2369-again
branch
3 times, most recently
from
375253d to
65829ef
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous approach broke code generation for the MS ABI due to an unintended code path during constraint substitution. This time we address the issue by inspecting the CodeSynthesisContexts and thereby avoiding that code path.
This reapplies 96eced6.