[clang] [Clang][Concepts] Fix a constraint comparison regression for out-of-line ClassTemplateDecls (PR #102587)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 9 21:09:50 PDT 2024
https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/102587
>From 62218a472c88764472ffba69ceca1825686fe1b9 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 9 Aug 2024 16:32:30 +0800
Subject: [PATCH 1/2] [Clang][Concepts] Fix a constraint comparison regression
for out-of-line ClassTemplateDecls
---
clang/docs/ReleaseNotes.rst | 2 ++
clang/lib/Sema/SemaConcept.cpp | 11 ++++++----
.../SemaTemplate/concepts-out-of-line-def.cpp | 21 +++++++++++++++++++
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0f1a4c1851911d..cb92ba25d1633f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -186,6 +186,8 @@ Bug Fixes to C++ Support
substitutions in concepts, so it doesn't incorrectly complain of missing
module imports in those situations. (#GH60336)
- Fix init-capture packs having a size of one before being instantiated. (#GH63677)
+- Clang now properly compares constraints on an out of line class template
+ declaration definition. (#GH102320)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index d4c9d044985e34..2871f78868aea7 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -948,7 +948,7 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
Sema &S, const Sema::TemplateCompareNewDeclInfo &DeclInfo,
const Expr *ConstrExpr) {
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
- DeclInfo.getDecl(), DeclInfo.getLexicalDeclContext(), /*Final=*/false,
+ DeclInfo.getDecl(), DeclInfo.getDeclContext(), /*Final=*/false,
/*Innermost=*/std::nullopt,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr, /*ForConstraintInstantiation=*/true,
@@ -971,9 +971,12 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
// this may happen while we're comparing two templates' constraint
// equivalence.
LocalInstantiationScope ScopeForParameters(S);
- if (auto *FD = DeclInfo.getDecl()->getAsFunction())
- for (auto *PVD : FD->parameters())
- ScopeForParameters.InstantiatedLocal(PVD, PVD);
+ if (const NamedDecl *D = DeclInfo.getDecl()) {
+ const FunctionDecl *FD = D->getAsFunction();
+ if (FD)
+ for (auto *PVD : FD->parameters())
+ ScopeForParameters.InstantiatedLocal(PVD, PVD);
+ }
std::optional<Sema::CXXThisScopeRAII> ThisScope;
diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
index 0142efcdc3ee86..5ea1ff79e572bc 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -599,3 +599,24 @@ template <class DerT>
unsigned long DerivedCollection<DerTs...>::index() {}
} // namespace GH72557
+
+namespace GH102320 {
+
+template <class, class>
+concept Constrained = true;
+
+template <class T> class C {
+ template <Constrained<T>> class D;
+ template <class U>
+ requires Constrained<T, U>
+ class E;
+};
+
+template <class T> template <Constrained<T>> class C<T>::D {};
+
+template <class T>
+template <class U>
+ requires Constrained<T, U>
+class C<T>::E {};
+
+} // namespace GH102320
>From 23bc9374b19b138a767fb8c7cd3d06b23433e699 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sat, 10 Aug 2024 12:09:00 +0800
Subject: [PATCH 2/2] Don't visit out-of-line specializations
---
clang/lib/Sema/SemaConcept.cpp | 35 +++++++++++++++----
.../SemaTemplate/concepts-out-of-line-def.cpp | 15 ++++++++
2 files changed, 43 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 2871f78868aea7..900d3d1616cbe2 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -947,12 +947,35 @@ namespace {
static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
Sema &S, const Sema::TemplateCompareNewDeclInfo &DeclInfo,
const Expr *ConstrExpr) {
+ const NamedDecl *ND = DeclInfo.getDecl();
+ // ND would be absent when we are parsing a template parameter header, and the
+ // template it pertains to is thus unavaliable. We collect the surrounding
+ // template arguments for evaluating constraints, starting from the current
+ // semantic context, in order for out-of-line constrained declarations to be
+ // properly evaluated.
+ //
+ // template <class T>
+ // class A {
+ // template <C<T>> class B;
+ // };
+ //
+ // template <class T> template <C<T> U> class A<T>::B {};
+ //
+ // (This is the case when comparing `template <C<T> U>` against its primary
+ // template parameter list `template <C<T>>`.)
+ //
+ // Parent specializations are also ignored because fully specialized parents
+ // of the out-of-line declaration don't contribute to the depth of templates.
+ //
+ // template <> template <C<void> U> class A<void>::B {};
+ // (U rests in the depth of 0.)
+ //
MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
- DeclInfo.getDecl(), DeclInfo.getDeclContext(), /*Final=*/false,
+ ND, DeclInfo.getDeclContext(), /*Final=*/false,
/*Innermost=*/std::nullopt,
/*RelativeToPrimary=*/true,
/*Pattern=*/nullptr, /*ForConstraintInstantiation=*/true,
- /*SkipForSpecialization*/ false);
+ /*SkipForSpecialization=*/!ND);
if (MLTAL.getNumSubstitutedLevels() == 0)
return ConstrExpr;
@@ -962,7 +985,7 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
Sema::InstantiatingTemplate Inst(
S, DeclInfo.getLocation(),
Sema::InstantiatingTemplate::ConstraintNormalization{},
- const_cast<NamedDecl *>(DeclInfo.getDecl()), SourceRange{});
+ const_cast<NamedDecl *>(ND), SourceRange{});
if (Inst.isInvalid())
return nullptr;
@@ -971,12 +994,10 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
// this may happen while we're comparing two templates' constraint
// equivalence.
LocalInstantiationScope ScopeForParameters(S);
- if (const NamedDecl *D = DeclInfo.getDecl()) {
- const FunctionDecl *FD = D->getAsFunction();
- if (FD)
+ if (ND)
+ if (const FunctionDecl *FD = ND->getAsFunction())
for (auto *PVD : FD->parameters())
ScopeForParameters.InstantiatedLocal(PVD, PVD);
- }
std::optional<Sema::CXXThisScopeRAII> ThisScope;
diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
index 5ea1ff79e572bc..c3eadaa3ceaf27 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -619,4 +619,19 @@ template <class U>
requires Constrained<T, U>
class C<T>::E {};
+#if 0
+// FIXME: Is it conforming? Only Clang rejects it in every released version.
+template <>
+template <Constrained<int> T>
+class C<int>::D<T> {};
+#endif
+
+template <>
+template <Constrained<int>>
+class C<int>::D {};
+
+template <>
+template <class T> requires Constrained<int, T>
+class C<int>::E {};
+
} // namespace GH102320
More information about the cfe-commits
mailing list