[clang] [clang] concepts: perform parameter mapping subsitution in correct context (PR #101745)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-modules

Author: Matheus Izvekov (mizvekov)

<details>
<summary>Changes</summary>

Prior to this patch, during constraint normalization we could forget from which declaration an atomic constraint was normalized from.

Subsequently when performing parameter mapping substitution for that atomic constraint with an incorrect context, we couldn't correctly recognize which declarations are supposed to be visible.

Fixes #<!-- -->60336

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


5 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+3) 
- (modified) clang/include/clang/Sema/SemaConcept.h (+3-2) 
- (modified) clang/lib/Sema/SemaConcept.cpp (+3-3) 
- (added) clang/test/Modules/GH60336-2.cpp (+44) 
- (modified) clang/test/Modules/GH60336.cpp (+2-11) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 25f5bd37bbe94..e8973002b3059 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -173,6 +173,9 @@ Bug Fixes to C++ Support
 - Clang now correctly parses potentially declarative nested-name-specifiers in pointer-to-member declarators.
 - Fix a crash when checking the initialzier of an object that was initialized
   with a string literal. (#GH82167)
+- Clang now correctly recognizes the correct context for parameter
+  suibstitutions in concepts, so it doesn't incorrectly complain of missing
+  module imports in those situations. (#GH60336)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/SemaConcept.h b/clang/include/clang/Sema/SemaConcept.h
index 03791962b2dc0..4b1abccb7741a 100644
--- a/clang/include/clang/Sema/SemaConcept.h
+++ b/clang/include/clang/Sema/SemaConcept.h
@@ -30,10 +30,11 @@ enum { ConstraintAlignment = 8 };
 
 struct alignas(ConstraintAlignment) AtomicConstraint {
   const Expr *ConstraintExpr;
+  NamedDecl *ConstraintDecl;
   std::optional<ArrayRef<TemplateArgumentLoc>> ParameterMapping;
 
-  AtomicConstraint(Sema &S, const Expr *ConstraintExpr) :
-      ConstraintExpr(ConstraintExpr) { };
+  AtomicConstraint(const Expr *ConstraintExpr, NamedDecl *ConstraintDecl)
+      : ConstraintExpr(ConstraintExpr), ConstraintDecl(ConstraintDecl) {};
 
   bool hasMatchingParameterMapping(ASTContext &C,
                                    const AtomicConstraint &Other) const {
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 9e16b67284be4..7d7a94e9fd637 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -1457,8 +1457,8 @@ substituteParameterMappings(Sema &S, NormalizedConstraint &N,
           : ArgsAsWritten->arguments().front().getSourceRange().getEnd();
   Sema::InstantiatingTemplate Inst(
       S, InstLocBegin,
-      Sema::InstantiatingTemplate::ParameterMappingSubstitution{}, Concept,
-      {InstLocBegin, InstLocEnd});
+      Sema::InstantiatingTemplate::ParameterMappingSubstitution{},
+      Atomic.ConstraintDecl, {InstLocBegin, InstLocEnd});
   if (Inst.isInvalid())
     return true;
   if (S.SubstTemplateArguments(*Atomic.ParameterMapping, MLTAL, SubstArgs))
@@ -1632,7 +1632,7 @@ NormalizedConstraint::fromConstraintExpr(Sema &S, NamedDecl *D, const Expr *E) {
         Kind, std::move(*Sub), FE->getPattern()}};
   }
 
-  return NormalizedConstraint{new (S.Context) AtomicConstraint(S, E)};
+  return NormalizedConstraint{new (S.Context) AtomicConstraint(E, D)};
 }
 
 bool FoldExpandedConstraint::AreCompatibleForSubsumption(
diff --git a/clang/test/Modules/GH60336-2.cpp b/clang/test/Modules/GH60336-2.cpp
new file mode 100644
index 0000000000000..9740c744b7b7b
--- /dev/null
+++ b/clang/test/Modules/GH60336-2.cpp
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x c++ -std=c++20 %s -verify -fmodules -fmodules-cache-path=%t
+// expected-no-diagnostics
+
+#pragma clang module build std
+module std {
+  module concepts {}
+  module functional {}
+}
+#pragma clang module contents
+#pragma clang module begin std
+
+template <class _Tp> struct common_reference {
+  using type = _Tp;
+};
+
+#pragma clang module end
+#pragma clang module begin std.concepts
+#pragma clang module import std
+
+template <class _Tp>
+concept same_as = __is_same(_Tp, _Tp);
+
+template <class _Tp>
+concept common_reference_with =
+    same_as<typename common_reference<_Tp>::type>;
+
+#pragma clang module end
+#pragma clang module begin std.functional
+#pragma clang module import std.concepts
+
+template <class, class _Ip>
+concept sentinel_for = common_reference_with<_Ip>;
+
+constexpr bool ntsf_subsumes_sf(sentinel_for<char *> auto)
+  requires true
+{
+  return true;
+}
+bool ntsf_subsumes_sf(sentinel_for<char *> auto);
+static_assert(ntsf_subsumes_sf(""));
+
+#pragma clang module end
+#pragma clang module endbuild
diff --git a/clang/test/Modules/GH60336.cpp b/clang/test/Modules/GH60336.cpp
index fefbd37b7926c..e181c24904217 100644
--- a/clang/test/Modules/GH60336.cpp
+++ b/clang/test/Modules/GH60336.cpp
@@ -1,5 +1,7 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -x c++ -std=c++20 %s -verify -fmodules -fmodules-cache-path=%t
+// expected-no-diagnostics
+
 #pragma clang module build std
 module std   [system] {
   module concepts     [system] {
@@ -65,14 +67,3 @@ constexpr bool ntsf_subsumes_sf(std::nothrow_sentinel_for<char*> auto) requires
 }
 constexpr bool ntsf_subsumes_sf(std::sentinel_for<char*> auto);
 static_assert(ntsf_subsumes_sf("foo"));
-
-// Note: Doing diagnostics verify lines in the individual modules isn't
-// permitted, and using 'bookmarks' in a module also doesn't work, so we're 
-// forced to diagnose this by line-number.
-//
-// Check to ensure that this error happens, prior to a revert of a concepts
-// sugaring patch, this diagnostic didn't happen correctly.
-
-// expected-error@* {{partial specialization of 'common_reference<_Tp, _Up>' must be imported from module 'std.type_traits' before it is required}}
-// expected-note at 63 {{while substituting into concept arguments here}}
-// expected-note@*{{partial specialization declared here is not reachable}}

``````````

</details>


https://github.com/llvm/llvm-project/pull/101745