[libcxx-commits] [lld] [clang] [llvm] [lldb] [clang-tools-extra] [libc] [compiler-rt] [libcxx] [flang] [concepts] Push a CurContext before substituting into out-of-line constraints for comparison (PR #79985)

[Younan Zhang via libcxx-commits]

https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/79985

>From 7ee1874af426d7879d218c6b8852fca0a87b1836 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Tue, 30 Jan 2024 18:49:08 +0800
Subject: [PATCH] [concepts] Push a CurContext before substituting into
 out-of-line constraints for comparison

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 addresses https://github.com/llvm/llvm-project/issues/56482.
---
 clang/docs/ReleaseNotes.rst                   |  3 +++
 clang/lib/AST/DeclTemplate.cpp                |  4 +++
 clang/lib/Sema/SemaConcept.cpp                | 14 ++++++++++-
 .../SemaTemplate/concepts-out-of-line-def.cpp | 25 +++++++++++++++++++
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index aa06e2b60ce9..52fe4fabfb61 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -128,6 +128,9 @@ Bug Fixes to C++ Support
 - 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>`_)
+- 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>`_)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
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 88fc846c89e4..b0974ca7f4a4 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -798,8 +798,20 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
     return nullptr;
 
   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 c4e8e6f720c4..3a0c59f01217 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -536,3 +536,28 @@ void X<T>::bar(decltype(requires { requires something_interesting<T>; })) {}
 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