[Clang][Concepts] Correct the CurContext for friend declarations

[zyn0217]

FindInstantiatedDecl() relies on the CurContext to find the corresponding class template instantiation for a class template declaration.

Previously, we pushed the semantic declaration context for constraint comparison, which is incorrect for constraints on friend declarations. In issue #78101, the semantic context of the friend is the TU, so we missed the implicit template specialization Template<void, 4> when looking for the instantiation of the primary template Template at the time of checking the member instantiation; instead, we mistakenly picked up the explicit specialization Template<float, 5>, hence the error.

As a bonus, this also fixes a crash when diagnosing constraints. The DeclarationName is not necessarily an identifier, so it's incorrect to call getName() on e.g. overloaded operators. Since the DiagnosticBuilder has correctly handled Decl printing, we don't need to find the printable name ourselves.

Fixes #78101

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

FindInstantiatedDecl() relies on the CurContext to find the corresponding class template instantiation for a class template declaration.

Previously, we pushed the semantic declaration context for constraint comparison, which is incorrect for constraints on friend declarations. In issue #78101, the semantic context of the friend is the TU, so we missed the implicit template specialization Template<void, 4> when looking for the instantiation of the primary template Template at the time of checking the member instantiation; instead, we mistakenly picked up the explicit specialization Template<float, 5>, hence the error.

As a bonus, this also fixes a crash when diagnosing constraints. The DeclarationName is not necessarily an identifier, so it's incorrect to call getName() on e.g. overloaded operators. Since the DiagnosticBuilder has correctly handled Decl printing, we don't need to find the printable name ourselves.

Fixes #78101

---

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

6 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1-1)
• (modified) clang/lib/Sema/SemaConcept.cpp (+4-1)
• (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+1-2)
• (modified) clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp (+1-1)
• (modified) clang/test/SemaTemplate/concepts-friends.cpp (+23)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 98fb0174d4a37e..fc940db4813948 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -338,6 +338,7 @@ Bug Fixes to C++ Support
 - Fixed an assertion failure when converting vectors to int/float with invalid expressions. (#GH105486)
 - Template parameter names are considered in the name lookup of out-of-line class template
   specialization right before its declaration context. (#GH64082)
+- Fixed a constraint comparison bug for friend declarations. (#GH78101)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 66f8890da75e5d..dcb49d8a67604a 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5603,7 +5603,7 @@ def note_checking_constraints_for_function_here : Note<
 def note_constraint_substitution_here : Note<
   "while substituting template arguments into constraint expression here">;
 def note_constraint_normalization_here : Note<
-  "while calculating associated constraint of template '%0' here">;
+  "while calculating associated constraint of template %0 here">;
 def note_parameter_mapping_substitution_here : Note<
   "while substituting into concept arguments here; substitution failures not "
   "allowed in concept arguments">;
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 86d6f308a51cc2..afbfe278680240 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -1012,7 +1012,10 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction(
   // 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())) {
+  if (auto *RD =
+          dyn_cast<CXXRecordDecl>(DeclInfo.getDecl()->getFriendObjectKind()
+                                      ? DeclInfo.getLexicalDeclContext()
+                                      : DeclInfo.getDeclContext())) {
     ThisScope.emplace(S, const_cast<CXXRecordDecl *>(RD), Qualifiers());
     ContextScope.emplace(S, const_cast<DeclContext *>(cast<DeclContext>(RD)),
                          /*NewThisContext=*/false);
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 776297479e141e..c42cc250bb904a 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1226,8 +1226,7 @@ void Sema::PrintInstantiationStack() {
     case CodeSynthesisContext::ConstraintNormalization:
       Diags.Report(Active->PointOfInstantiation,
                    diag::note_constraint_normalization_here)
-          << cast<NamedDecl>(Active->Entity)->getName()
-          << Active->InstantiationRange;
+          << cast<NamedDecl>(Active->Entity) << Active->InstantiationRange;
       break;
     case CodeSynthesisContext::ParameterMappingSubstitution:
       Diags.Report(Active->PointOfInstantiation,
diff --git a/clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp b/clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
index d80710937cdfa1..3992835c444027 100644
--- a/clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
+++ b/clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp
@@ -15,7 +15,7 @@ template<typename T> requires Bar2<T> struct S2 { };
 // expected-note@-1{{template is declared here}}
 template<typename T> requires Bar2<T> && true struct S2<T> { };
 // expected-error@-1{{class template partial specialization is not more specialized than the primary template}}
-// expected-note@-2{{while calculating associated constraint of template 'S2' here}}
+// expected-note@-2{{while calculating associated constraint of template 'S2<T>' here}}
 
 namespace type_pack {
   template<typename... Args>
diff --git a/clang/test/SemaTemplate/concepts-friends.cpp b/clang/test/SemaTemplate/concepts-friends.cpp
index 14b37d78d951dc..d05be423a8cfcd 100644
--- a/clang/test/SemaTemplate/concepts-friends.cpp
+++ b/clang/test/SemaTemplate/concepts-friends.cpp
@@ -525,3 +525,26 @@ struct S {
 };
 
 }
+
+namespace GH78101 {
+
+template <typename T, int i>
+concept True = true;
+
+template <typename T, int I> struct Template {
+  static constexpr int i = I;
+
+  friend constexpr auto operator+(True<i> auto f) { return i; }
+};
+
+template <int I> struct Template<float, I> {
+  static constexpr int i = I;
+
+  friend constexpr auto operator+(True<i> auto f) { return i; }
+};
+
+Template<void, 4> f{};
+
+static_assert(+Template<float, 5>{} == 5);
+
+} // namespace GH78101

[cor3ntin]

LGTM

[cor3ntin]

I wonder if we should have a function that handles that logic of getting a lexical context for friends - we do it in a few places

[zyn0217]

Agreed, probably something like Decl::getFriendDeclContext()