[concepts] Set up an instantiation scope for constraint expression comparison

[zyn0217]

This is a follow-up for the comparison of constraints on out-of-line function template definitions. We require the instantiation of a ParmVarDecl while transforming the expression if that Decl gets referenced by a DeclRefExpr. However, we're not actually performing the class or function template instantiation at the time of such comparison. Therefore, let's map these parameters to themselves so that they get preserved after the substitution.

Fixes #74447.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

This is a follow-up for the comparison of constraints on out-of-line function template definitions. We require the instantiation of a ParmVarDecl while transforming the expression if that Decl gets referenced by a DeclRefExpr. However, we're not actually performing the class or function template instantiation at the time of such comparison. Therefore, let's map these parameters to themselves so that they get preserved after the substitution.

Fixes #74447.

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/lib/Sema/SemaConcept.cpp (+9)
• (modified) clang/test/SemaTemplate/concepts-out-of-line-def.cpp (+22)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index aa06e2b60ce915..3c5856788e717e 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>`_)
+- Fixed a crash where substituting into a requires-expression that refers to function
+  parameters during the equivalence determination of two constraint expressions.
+  (`#74447 <https://github.com/llvm/llvm-project/issues/74447>`)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 88fc846c89e42c..19a460f4117579 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -797,6 +797,15 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
   if (Inst.isInvalid())
     return nullptr;
 
+  // Set up a dummy 'instantiation' scope in the case of reference to function
+  // parameters that the surrounding function hasn't been instantiated yet. Note
+  // this may happen while we're comparing two templates' constraint
+  // equivalence.
+  LocalInstantiationScope ScopeForParameters(S);
+  if (auto *FD = llvm::dyn_cast<FunctionDecl>(DeclInfo.getDecl()))
+    for (auto *PVD : FD->parameters())
+      ScopeForParameters.InstantiatedLocal(PVD, PVD);
+
   std::optional<Sema::CXXThisScopeRAII> ThisScope;
   if (auto *RD = dyn_cast<CXXRecordDecl>(DeclInfo.getDeclContext()))
     ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers());
diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
index c4e8e6f720c492..7323ad8d9ef2cb 100644
--- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -536,3 +536,25 @@ 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 GH74447 {
+template <typename T> struct S {
+  template <typename... U, int V>
+  void test(T target, U... value)
+    requires requires {
+      target;
+      sizeof...(value) == 1;
+      V == 2;
+    };
+};
+
+template <typename T>
+template <typename... U, int V>
+void S<T>::test(T target, U... value)
+  requires requires {
+    target;
+    sizeof...(value) == 1;
+    V == 2;
+  }
+{}
+} // namespace GH74447

[erichkeane]

I could swear I did this at one point and it caused significant problems... I don't have a problem with it as-is, but we need to make sure this passes all the CI (including the libcxx tests)

[erichkeane]

Please make sure you analyze any libcxx problems on this one.

[zyn0217]

I've seen llvm-libc++-shared.cfg.in :: libcxx/gdb/gdb_pretty_printer_test.sh.cpp was failing intermittently on CI recently, which turned out to be unrelated to Concepts. In case I missed anything, I've run the libcxx tests locally, which are all green.

Testing Time: 1416.40s

Total Discovered Tests: 9636
  Unsupported      :  574 (5.96%)
  Passed           : 9037 (93.78%)
  Expectedly Failed:   25 (0.26%)

[zyn0217]

I'll try the CI again before landing it.