[clang] [CONCEPTS]Corrected comparison of constraints with out of line CTD (PR #69244)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 16 13:09:20 PDT 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Erich Keane (erichkeane)
<details>
<summary>Changes</summary>
Out of line class template declaration specializations aren't created at the time they have their template arguments checked, so we previously weren't doing any amount of work to substitute the constraints before comparison. This resulted in the out of line definition's difference in 'depth' causing the constraints to compare differently.
This patch corrects that. Additionally, it handles ClassTemplateDecl when collecting template arguments.
Fixes: #<!-- -->61763
---
Patch is 21.28 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/69244.diff
9 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+4)
- (modified) clang/include/clang/Sema/Sema.h (+72-19)
- (modified) clang/include/clang/Sema/Template.h (+3-1)
- (modified) clang/lib/Sema/SemaConcept.cpp (+22-17)
- (modified) clang/lib/Sema/SemaTemplate.cpp (+13-9)
- (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+1-1)
- (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+16-4)
- (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+6-3)
- (modified) clang/test/SemaTemplate/concepts-out-of-line-def.cpp (+37)
``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 68172d5317a13ba..e7ef8d219de2c41 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -359,6 +359,10 @@ Bug Fixes to C++ Support
reference. Fixes:
(`#64162 <https://github.com/llvm/llvm-project/issues/64162>`_)
+- Clang now properly compares constraints on an out of line class template
+ declaration definition. Fixes:
+ (`#61763 <https://github.com/llvm/llvm-project/issues/61763>`_)
+
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
- Fixed an import failure of recursive friend class template.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 712db0a3dd895d5..7541e087f4ec77f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3816,17 +3816,6 @@ class Sema final {
// the purposes of [temp.friend] p9.
bool FriendConstraintsDependOnEnclosingTemplate(const FunctionDecl *FD);
- // Calculates whether two constraint expressions are equal irrespective of a
- // difference in 'depth'. This takes a pair of optional 'NamedDecl's 'Old' and
- // 'New', which are the "source" of the constraint, since this is necessary
- // for figuring out the relative 'depth' of the constraint. The depth of the
- // 'primary template' and the 'instantiated from' templates aren't necessarily
- // the same, such as a case when one is a 'friend' defined in a class.
- bool AreConstraintExpressionsEqual(const NamedDecl *Old,
- const Expr *OldConstr,
- const NamedDecl *New,
- const Expr *NewConstr);
-
enum class AllowedExplicit {
/// Allow no explicit functions to be used.
None,
@@ -8590,8 +8579,48 @@ class Sema final {
TPL_TemplateParamsEquivalent,
};
+ // A struct to represent the 'new' declaration, which is either itself just
+ // the named decl, or the important information we need about it in order to
+ // do constraint comparisions.
+ class TemplateCompareNewDeclInfo {
+ const NamedDecl *ND = nullptr;
+ const DeclContext *DC = nullptr;
+ const DeclContext *LexicalDC = nullptr;
+ SourceLocation Loc;
+
+ public:
+ TemplateCompareNewDeclInfo(const NamedDecl *ND) : ND(ND) {}
+ TemplateCompareNewDeclInfo(const DeclContext *DeclCtx,
+ const DeclContext *LexicalDeclCtx,
+ SourceLocation Loc)
+
+ : DC(DeclCtx), LexicalDC(LexicalDeclCtx), Loc(Loc) {
+ assert(DC && LexicalDC &&
+ "Constructor only for cases where we have the information to put "
+ "in here");
+ }
+
+ // If this was constructed with no information, we cannot do substitution
+ // for constraint comparison, so make sure we can check that.
+ bool isInvalid() const { return !ND && !DC; }
+
+ const NamedDecl *getDecl() const { return ND; }
+
+ bool ContainsDecl(const NamedDecl *ND) const { return this->ND == ND; }
+
+ const DeclContext *getLexicalDeclContext() const {
+ return ND ? ND->getLexicalDeclContext() : LexicalDC;
+ }
+
+ const DeclContext *getDeclContext() const {
+ return ND ? ND->getDeclContext() : DC;
+ }
+
+ SourceLocation getLocation() const { return ND ? ND->getLocation() : Loc; }
+ };
+
bool TemplateParameterListsAreEqual(
- const NamedDecl *NewInstFrom, TemplateParameterList *New,
+ const TemplateCompareNewDeclInfo &NewInstFrom, TemplateParameterList *New,
const NamedDecl *OldInstFrom, TemplateParameterList *Old, bool Complain,
TemplateParameterListEqualKind Kind,
SourceLocation TemplateArgLoc = SourceLocation());
@@ -8604,6 +8633,31 @@ class Sema final {
Kind, TemplateArgLoc);
}
+ // Calculates whether two constraint expressions are equal irrespective of a
+ // difference in 'depth'. This takes a pair of optional 'NamedDecl's 'Old' and
+ // 'New', which are the "source" of the constraint, since this is necessary
+ // for figuring out the relative 'depth' of the constraint. The depth of the
+ // 'primary template' and the 'instantiated from' templates aren't necessarily
+ // the same, such as a case when one is a 'friend' defined in a class.
+ bool AreConstraintExpressionsEqual(const NamedDecl *Old,
+ const Expr *OldConstr,
+ const TemplateCompareNewDeclInfo &New,
+ const Expr *NewConstr);
+
+// // Calculates whether two constraint expressions are equal irrespective of a
+// // difference in 'depth'. This overload takes the information for the 'New'
+// // declaration separately in order to support situations where we do not yet
+// // have a declaration (such as when we are doing the checking of a partial
+// // specialization.
+// bool AreConstraintExpressionsEqual(const NamedDecl *Old,
+// const Expr *OldConstr,
+// const DeclContext *NewLexicalContext,
+// const DeclContext *NewDeclContext,
+// SourceLocation NewDeclLocation,
+// const TemplateArgumentList *NewArgs,
+// const Expr *NewConstr);
+//
+
bool CheckTemplateDeclScope(Scope *S, TemplateParameterList *TemplateParams);
/// Called when the parser has parsed a C++ typename
@@ -9343,13 +9397,12 @@ class Sema final {
// C++ Template Instantiation
//
- MultiLevelTemplateArgumentList
- getTemplateInstantiationArgs(const NamedDecl *D, bool Final = false,
- const TemplateArgumentList *Innermost = nullptr,
- bool RelativeToPrimary = false,
- const FunctionDecl *Pattern = nullptr,
- bool ForConstraintInstantiation = false,
- bool SkipForSpecialization = false);
+ MultiLevelTemplateArgumentList getTemplateInstantiationArgs(
+ const NamedDecl *D, const DeclContext *DC = nullptr, bool Final = false,
+ const TemplateArgumentList *Innermost = nullptr,
+ bool RelativeToPrimary = false, const FunctionDecl *Pattern = nullptr,
+ bool ForConstraintInstantiation = false,
+ bool SkipForSpecialization = false);
/// A context in which code is being synthesized (where a source location
/// alone is not sufficient to identify the context). This covers template
diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h
index 1de2cc6917b424a..29fab7e10dd0cf8 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -213,7 +213,9 @@ enum class TemplateSubstitutionKind : char {
"substituted args outside retained args?");
assert(getKind() == TemplateSubstitutionKind::Specialization);
TemplateArgumentLists.push_back(
- {{AssociatedDecl->getCanonicalDecl(), Final}, Args});
+ {{AssociatedDecl ? AssociatedDecl->getCanonicalDecl() : nullptr,
+ Final},
+ Args});
}
void addOuterTemplateArguments(ArgList Args) {
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 036548b68247bfa..f12df4795af3c35 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -657,11 +657,11 @@ Sema::SetupConstraintCheckingTemplateArgumentsAndScope(
// 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.
- MLTAL =
- getTemplateInstantiationArgs(FD, /*Final=*/false, /*Innermost=*/nullptr,
- /*RelativeToPrimary=*/true,
- /*Pattern=*/nullptr,
- /*ForConstraintInstantiation=*/true);
+ MLTAL = getTemplateInstantiationArgs(FD, FD->getLexicalDeclContext(),
+ /*Final=*/false, /*Innermost=*/nullptr,
+ /*RelativeToPrimary=*/true,
+ /*Pattern=*/nullptr,
+ /*ForConstraintInstantiation=*/true);
if (SetupConstraintScope(FD, TemplateArgs, MLTAL, Scope))
return std::nullopt;
@@ -736,7 +736,8 @@ static unsigned
CalculateTemplateDepthForConstraints(Sema &S, const NamedDecl *ND,
bool SkipForSpecialization = false) {
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
- ND, /*Final=*/false, /*Innermost=*/nullptr, /*RelativeToPrimary=*/true,
+ ND, ND->getLexicalDeclContext(), /*Final=*/false, /*Innermost=*/nullptr,
+ /*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/true, SkipForSpecialization);
return MLTAL.getNumLevels();
@@ -770,28 +771,31 @@ namespace {
};
} // namespace
-static const Expr *SubstituteConstraintExpression(Sema &S, const NamedDecl *ND,
- const Expr *ConstrExpr) {
+static const Expr *
+SubstituteConstraintExpression(Sema &S,
+ const Sema::TemplateCompareNewDeclInfo &DeclInfo,
+ const Expr *ConstrExpr) {
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
- ND, /*Final=*/false, /*Innermost=*/nullptr,
+ DeclInfo.getDecl(), DeclInfo.getLexicalDeclContext(), /*Final=*/false,
+ /*Innermost=*/nullptr,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr, /*ForConstraintInstantiation=*/true,
/*SkipForSpecialization*/ false);
+
if (MLTAL.getNumSubstitutedLevels() == 0)
return ConstrExpr;
Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false);
Sema::InstantiatingTemplate Inst(
- S, ND->getLocation(),
+ S, DeclInfo.getLocation(),
Sema::InstantiatingTemplate::ConstraintNormalization{},
- const_cast<NamedDecl *>(ND), SourceRange{});
-
+ const_cast<NamedDecl *>(DeclInfo.getDecl()), SourceRange{});
if (Inst.isInvalid())
return nullptr;
std::optional<Sema::CXXThisScopeRAII> ThisScope;
- if (auto *RD = dyn_cast<CXXRecordDecl>(ND->getDeclContext()))
+ if (auto *RD = dyn_cast<CXXRecordDecl>(DeclInfo.getDeclContext()))
ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers());
ExprResult SubstConstr =
S.SubstConstraintExpr(const_cast<clang::Expr *>(ConstrExpr), MLTAL);
@@ -802,13 +806,13 @@ static const Expr *SubstituteConstraintExpression(Sema &S, const NamedDecl *ND,
bool Sema::AreConstraintExpressionsEqual(const NamedDecl *Old,
const Expr *OldConstr,
- const NamedDecl *New,
+ const TemplateCompareNewDeclInfo &New,
const Expr *NewConstr) {
if (OldConstr == NewConstr)
return true;
// C++ [temp.constr.decl]p4
- if (Old && New && Old != New &&
- Old->getLexicalDeclContext() != New->getLexicalDeclContext()) {
+ if (Old && !New.isInvalid() && !New.ContainsDecl(Old) &&
+ Old->getLexicalDeclContext() != New.getLexicalDeclContext()) {
if (const Expr *SubstConstr =
SubstituteConstraintExpression(*this, Old, OldConstr))
OldConstr = SubstConstr;
@@ -1252,7 +1256,8 @@ static bool substituteParameterMappings(Sema &S, NormalizedConstraint &N,
TemplateArgumentList TAL{TemplateArgumentList::OnStack,
CSE->getTemplateArguments()};
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
- CSE->getNamedConcept(), /*Final=*/false, &TAL,
+ CSE->getNamedConcept(), CSE->getNamedConcept()->getLexicalDeclContext(),
+ /*Final=*/false, &TAL,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/true);
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 994d89e25b3848e..e4e4aa30215fef2 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1994,10 +1994,13 @@ DeclResult Sema::CheckClassTemplate(
// for a friend in a dependent context: the template parameter list itself
// could be dependent.
if (!(TUK == TUK_Friend && CurContext->isDependentContext()) &&
- !TemplateParameterListsAreEqual(TemplateParams,
- PrevClassTemplate->getTemplateParameters(),
- /*Complain=*/true,
- TPL_TemplateMatch))
+ !TemplateParameterListsAreEqual(
+ TemplateCompareNewDeclInfo(SemanticContext ? SemanticContext
+ : CurContext,
+ CurContext, KWLoc),
+ TemplateParams, PrevClassTemplate,
+ PrevClassTemplate->getTemplateParameters(), /*Complain=*/true,
+ TPL_TemplateMatch))
return true;
// C++ [temp.class]p4:
@@ -6204,7 +6207,7 @@ bool Sema::CheckTemplateArgumentList(
CXXThisScopeRAII(*this, RD, ThisQuals, RD != nullptr);
MultiLevelTemplateArgumentList MLTAL = getTemplateInstantiationArgs(
- Template, /*Final=*/false, &StackTemplateArgs,
+ Template, NewContext, /*Final=*/false, &StackTemplateArgs,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr,
/*ForConceptInstantiation=*/true);
@@ -8012,7 +8015,8 @@ Sema::BuildExpressionFromIntegralTemplateArgument(const TemplateArgument &Arg,
/// Match two template parameters within template parameter lists.
static bool MatchTemplateParameterKind(
- Sema &S, NamedDecl *New, const NamedDecl *NewInstFrom, NamedDecl *Old,
+ Sema &S, NamedDecl *New,
+ const Sema::TemplateCompareNewDeclInfo &NewInstFrom, NamedDecl *Old,
const NamedDecl *OldInstFrom, bool Complain,
Sema::TemplateParameterListEqualKind Kind, SourceLocation TemplateArgLoc) {
// Check the actual kind (type, non-type, template).
@@ -8100,8 +8104,8 @@ static bool MatchTemplateParameterKind(
// For template template parameters, check the template parameter types.
// The template parameter lists of template template
// parameters must agree.
- else if (TemplateTemplateParmDecl *OldTTP
- = dyn_cast<TemplateTemplateParmDecl>(Old)) {
+ else if (TemplateTemplateParmDecl *OldTTP =
+ dyn_cast<TemplateTemplateParmDecl>(Old)) {
TemplateTemplateParmDecl *NewTTP = cast<TemplateTemplateParmDecl>(New);
if (!S.TemplateParameterListsAreEqual(
NewInstFrom, NewTTP->getTemplateParameters(), OldInstFrom,
@@ -8205,7 +8209,7 @@ void DiagnoseTemplateParameterListArityMismatch(Sema &S,
/// \returns True if the template parameter lists are equal, false
/// otherwise.
bool Sema::TemplateParameterListsAreEqual(
- const NamedDecl *NewInstFrom, TemplateParameterList *New,
+ const TemplateCompareNewDeclInfo &NewInstFrom, TemplateParameterList *New,
const NamedDecl *OldInstFrom, TemplateParameterList *Old, bool Complain,
TemplateParameterListEqualKind Kind, SourceLocation TemplateArgLoc) {
if (Old->size() != New->size() && Kind != TPL_TemplateTemplateArgumentMatch) {
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 31ea7be2975e496..7471a5a81152559 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -2889,7 +2889,7 @@ CheckDeducedArgumentConstraints(Sema &S, TemplateDeclT *Template,
CanonicalDeducedArgs};
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
- Template, /*Final=*/false,
+ Template, Template->getDeclContext(), /*Final=*/false,
/*InnerMost=*/NeedsReplacement ? nullptr : &DeducedTAL,
/*RelativeToPrimary=*/true, /*Pattern=*/
nullptr, /*ForConstraintInstantiation=*/true);
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 00a36696cf90450..ae026ebc9755df2 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -312,6 +312,10 @@ Response HandleGenericDeclContext(const Decl *CurDecl) {
/// \param ND the declaration for which we are computing template instantiation
/// arguments.
///
+/// \param DC In the event we don't HAVE a declaration yet, we instead provide
+/// the decl context where it will be created. In this case, the `Innermost`
+/// should likely be provided. If ND is non-null, this is ignored.
+///
/// \param Innermost if non-NULL, specifies a template argument list for the
/// template declaration passed as ND.
///
@@ -331,10 +335,11 @@ Response HandleGenericDeclContext(const Decl *CurDecl) {
/// arguments on an enclosing class template.
MultiLevelTemplateArgumentList Sema::getTemplateInstantiationArgs(
- const NamedDecl *ND, bool Final, const TemplateArgumentList *Innermost,
- bool RelativeToPrimary, const FunctionDecl *Pattern,
- bool ForConstraintInstantiation, bool SkipForSpecialization) {
- assert(ND && "Can't find arguments for a decl if one isn't provided");
+ const NamedDecl *ND, const DeclContext *DC, bool Final,
+ const TemplateArgumentList *Innermost, bool RelativeToPrimary,
+ const FunctionDecl *Pattern, bool ForConstraintInstantiation,
+ bool SkipForSpecialization) {
+ 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;
@@ -346,6 +351,11 @@ MultiLevelTemplateArgumentList Sema::getTemplateInstantiationArgs(
CurDecl = Response::UseNextDecl(ND).NextDecl;
}
+ if (!ND) {
+ assert(DC);
+ CurDecl = Decl::castFromDeclContext(DC);
+ }
+
while (!CurDecl->isFileContextDecl()) {
Response R;
if (const auto *VarTemplSpec =
@@ -369,6 +379,8 @@ MultiLevelTemplateArgumentList Sema::getTemplateInstantiationArgs(
R = HandleImplicitConceptSpecializationDecl(CSD, Result);
} else if (const auto *FTD = dyn_cast<FunctionTemplateDecl>(CurDecl)) {
R = HandleFunctionTemplateDecl(FTD, Result);
+ } else if (const auto *CTD = dyn_cast<ClassTemplateDecl>(CurDecl)) {
+ R = Response::ChangeDecl(CTD->getLexicalDeclContext());
} else if (!isa<DeclContext>(CurDecl)) {
R = Response::DontClearRelativeToPrimaryNextDecl(CurDecl);
if (CurDecl->getDeclContext()->isTranslationUnit()) {
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index fa839e9b71a3cf9..7b22022690be0a5 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4601,7 +4601,8 @@ bool Sema::InstantiateDefaultArgument(SourceLocation CallLoc, FunctionDecl *FD,
// template<typename T>
// A<T> Foo(int a = A<T>::FooImpl());
MultiLevelTemplateArgumentList TemplateArgs = getTemplateInstantiationArgs(
- FD, /*Final=*/false, nullptr, /*RelativeToPrimary=*/true);
+ FD, FD->getLexicalDeclContext(), /*Final=*/false, nullptr,
+ /*RelativeToPrimary=*/true);
if (SubstDefaultArgument(CallLoc, Param, TemplateArgs, /*ForCallExpr*/ true))
return true;
@@ -4640,7 +4641,8 @@ void Sema::InstantiateExceptionSpec(SourceLocation PointOfInstantiation,
LocalInstantiationScope Scope(*this);
MultiLevelTemplateArgumentList TemplateArgs = getTemplateInstantiationArgs(
- Decl, /*Final=*/false, nullptr, /*RelativeToPrimary*/ true);
+ Decl, Decl->getLexicalDeclContext(), /*Final=*/false, nullptr,
+ /*RelativeToPrimary*/ true);
// FIXME: We can't use getTemplateInstantiationPattern(false) in general
// here, because for a non-defining friend declaration in a class template,
@@ -5082,7 +5084,8 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
SetDeclDefaulted(Function, PatternDecl->getLocation());
} else {
MultiLevelTemplateArgumentList TemplateArgs = getTemplateI...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/69244
More information about the cfe-commits
mailing list