[concepts] Push a CurContext before substituting into out-of-line constraints for comparison

[zyn0217]

InjectedClassNameType is such that every template specialization within the Record scope ought to canonicalize to it, as outlined above the definition of that Type.

This invariant is maintained during the tree transformation at the rebuilding stage for a template specialization type; see RebuildTemplateSpecializationType. In that, we attempt to retrieve the current instantiation from Sema.CurContext, and if that fails, the transformation proceeds silently.

In terms of this issue, we previously set no CurContext other than that set by Parser, who had left the FunctionDecl before performing the constraint comparison. As a result, we would profile these types (i.e. InjectedClassNameType and its specialization type) into different values and fail to consider two expressions equivalent, although they are.

In passing, this also fixes a crash while attempting to dump the transformed expression. We failed to look into the template parameters from a CXXRecordDecl, which we should have done since the Decl we bound to a SubstTemplateTypeParmType can be of a CXXRecord type per the call to getTemplateInstantiationArgs within SubstituteConstraintExpressionWithoutSatisfaction.

This closes #56482.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

InjectedClassNameType is such that every template specialization within the Record scope ought to canonicalize to it, as outlined above the definition of that Type.

This invariant is maintained during the tree transformation at the rebuilding stage for a template specialization type; see RebuildTemplateSpecializationType. In that, we attempt to retrieve the current instantiation from Sema.CurContext, and if that fails, the transformation proceeds silently.

In terms of this issue, we previously set no CurContext other than that set by Parser, who had left the FunctionDecl before performing the constraint comparison. As a result, we would profile these types (i.e. InjectedClassNameType and its specialization type) into different values and fail to consider two expressions equivalent, although they are.

In passing, this also fixes a crash while attempting to dump the transformed expression. We failed to look into the template parameters from a CXXRecordDecl, which we should have done since the Decl we bound to a SubstTemplateTypeParmType can be of a CXXRecord type per the call to getTemplateInstantiationArgs within SubstituteConstraintExpressionWithoutSatisfaction.

This closes #56482.

---

Full diff: https://github.com/llvm/llvm-project/pull/79985.diff

4 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/lib/AST/DeclTemplate.cpp (+4)
• (modified) clang/lib/Sema/SemaConcept.cpp (+13-1)
• (modified) clang/test/SemaTemplate/concepts-out-of-line-def.cpp (+23)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f0dea0c9bc89..eb35782b8185 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -135,6 +135,9 @@ Bug Fixes to C++ Support
   parameter where we did an incorrect specialization of the initialization of
   the default parameter.
   Fixes (`#68490 <https://github.com/llvm/llvm-project/issues/68490>`_)
+- Addressed an issue where constraints involving injected class types are perceived
+  distinct from its specialization types.
+  (`#56482 <https://github.com/llvm/llvm-project/issues/56482>`_)
 - Fixed a bug where variables referenced by requires-clauses inside
   nested generic lambdas were not properly injected into the constraint scope.
   (`#73418 <https://github.com/llvm/llvm-project/issues/73418>`_)
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 7d7556e670f9..946a34ea8830 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -1583,6 +1583,10 @@ void TemplateParamObjectDecl::printAsInit(llvm::raw_ostream &OS,
 
 TemplateParameterList *clang::getReplacedTemplateParameterList(Decl *D) {
   switch (D->getKind()) {
+  case Decl::Kind::CXXRecord:
+    return cast<CXXRecordDecl>(D)
+        ->getDescribedTemplate()
+        ->getTemplateParameters();
   case Decl::Kind::ClassTemplate:
     return cast<ClassTemplateDecl>(D)->getTemplateParameters();
   case Decl::Kind::ClassTemplateSpecialization: {
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 19a460f41175..5028879eda22 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -807,8 +807,20 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
       ScopeForParameters.InstantiatedLocal(PVD, PVD);
 
   std::optional<Sema::CXXThisScopeRAII> ThisScope;
-  if (auto *RD = dyn_cast<CXXRecordDecl>(DeclInfo.getDeclContext()))
+
+  // See TreeTransform::RebuildTemplateSpecializationType. A context scope is
+  // essential for having an injected class as the canonical type for a template
+  // specialization type at the rebuilding stage. This guarantees that, for
+  // out-of-line definitions, injected class name types and their equivalent
+  // template specializations can be profiled to the same value, which makes it
+  // possible that e.g. constraints involving C<Class<T>> and C<Class> are
+  // perceived identical.
+  std::optional<Sema::ContextRAII> ContextScope;
+  if (auto *RD = dyn_cast<CXXRecordDecl>(DeclInfo.getDeclContext())) {
     ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers());
+    ContextScope.emplace(S, const_cast<DeclContext *>(cast<DeclContext>(RD)),
+                         /*NewThisContext=*/false);
+  }
   ExprResult SubstConstr = S.SubstConstraintExprWithoutSatisfaction(
       const_cast<clang::Expr *>(ConstrExpr), MLTAL);
   if (SFINAE.hasErrorOccurred() || !SubstConstr.isUsable())
diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
index 7323ad8d9ef2..b6fea2e0b4b3 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -537,6 +537,29 @@ template <class T>
 void X<T>::bar(decltype(requires { requires is_not_same_v<T, int>; })) {}
 } // namespace GH74314
 
+namespace GH56482 {
+template <typename SlotMap>
+concept slot_map_has_reserve = true;
+
+template <typename T> struct Slot_map {
+  constexpr void reserve() const noexcept
+    requires slot_map_has_reserve<Slot_map>;
+
+  constexpr void reserve(int) const noexcept
+    requires slot_map_has_reserve<Slot_map<T>>;
+};
+
+template <typename T>
+constexpr void Slot_map<T>::reserve() const noexcept
+  requires slot_map_has_reserve<Slot_map<T>>
+{}
+
+template <typename T>
+constexpr void Slot_map<T>::reserve(int) const noexcept
+  requires slot_map_has_reserve<Slot_map>
+{}
+} // namespace GH56482
+
 namespace GH74447 {
 template <typename T> struct S {
   template <typename... U, int V>

[zyn0217]

I plan to backport this patch and #79698 to clang 18 a few days later to see if we're actually not breaking anything.