[clang] [clang] Consistently cache failed constraint normalization (PR #227086)

[Nico Weber via cfe-commits]

https://github.com/nico created https://github.com/llvm/llvm-project/pull/227086

If substituting the parameter mappings of a normalized constraint failed, Sema::getNormalizedAssociatedConstraints() returned nullptr, but it stored the partially substituted normal form in NormalizationCache. So the first lookup for such a declaration failed, but every later lookup returned the broken normal form, and subsumption checking and the ambiguous-constraint diagnostics then continued with it.

I believe this wasn't intentional:

- Before #161671 (e9972debc98c), normalization was a single step, and a failure was cached as nullptr.

- #161671 added the parameter mapping substitution step. It inserted the normal form into the cache before substituting, and returned nullptr if the substitution then failed, leaving the non-null normal form in the cache.

- #165352 (2984a8db804e) moved the insertion after the substitution to not use an invalidated iterator, but kept inserting the normal form if the substitution failed.

Instead, cache failed substitution as nullptr, like a failed normalization.

This removes diagnostics that were only emitted because the second lookup continued with the broken normal form. #161671 added these to temp.constr.normal/p1.cpp:

- A second "'type name' declared as a pointer to a reference" error (with its notes) for the same broken concept. For

      template<typename T> concept Foo = True<T*>;
      template<typename T> concept Bar = Foo<T&>;
      template<typename T> requires Bar<T> struct S { };
      template<typename T> requires Bar<T> && true struct S<T> { };

  the error got reported once for the partial specialization, and also for the primary template after. Now, we only have the first report.

- A "similar constraint expressions not considered equivalent" note and its "similar constraint expression here" note, which we computed from the broken normal form. The actual problem in that test is the broken constraint, which is still diagnosed.

This also makes it possible to key the normalization cache by constraint expression without changing which diagnostics are emitted for failures, which I want to do in a follow-up.

>From eabf8fb87cc017816cf007599ba678ba751f4c26 Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis at chromium.org>
Date: Mon, 28 Sep 2026 11:26:36 -0700
Subject: [PATCH] [clang] Consistently cache failed constraint normalization

If substituting the parameter mappings of a normalized constraint failed,
Sema::getNormalizedAssociatedConstraints() returned nullptr, but it stored
the partially substituted normal form in NormalizationCache. So the first
lookup for such a declaration failed, but every later lookup returned the
broken normal form, and subsumption checking and the ambiguous-constraint
diagnostics then continued with it.

I believe this wasn't intentional:

- Before #161671 (e9972debc98c), normalization was a single step, and a failure
  was cached as nullptr.

- #161671 added the parameter mapping substitution step. It inserted the normal
  form into the cache before substituting, and returned nullptr if the
  substitution then failed, leaving the non-null normal form in the cache.

- #165352 (2984a8db804e) moved the insertion after the substitution to not use
  an invalidated iterator, but kept inserting the normal form if the
  substitution failed.

Instead, cache failed substitution as nullptr, like a failed normalization.

This removes diagnostics that were only emitted because the second lookup
continued with the broken normal form. #161671 added these to
temp.constr.normal/p1.cpp:

- A second "'type name' declared as a pointer to a reference" error (with
  its notes) for the same broken concept. For

      template<typename T> concept Foo = True<T*>;
      template<typename T> concept Bar = Foo<T&>;
      template<typename T> requires Bar<T> struct S { };
      template<typename T> requires Bar<T> && true struct S<T> { };

  the error got reported once for the partial specialization, and also for the
  primary template after. Now, we only have the first report.

- A "similar constraint expressions not considered equivalent" note and its
  "similar constraint expression here" note, which we computed from the
  broken normal form. The actual problem in that test is the broken
  constraint, which is still diagnosed.

This also makes it possible to key the normalization cache by constraint
expression without changing which diagnostics are emitted for failures,
which I want to do in a follow-up.
---
 clang/lib/Sema/SemaConcept.cpp                        |  9 ++-------
 .../CXX/temp/temp.constr/temp.constr.normal/p1.cpp    | 11 +++--------
 2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 27c61586020ba..d50fa6deb3375 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -2534,17 +2534,12 @@ const NormalizedConstraint *Sema::getNormalizedAssociatedConstraints(
   if (CacheEntry == NormalizationCache.end()) {
     auto *Normalized = NormalizedConstraint::fromAssociatedConstraints(
         *this, ND, AssociatedConstraints);
-    if (!Normalized) {
-      NormalizationCache.try_emplace(ConstrainedDeclOrNestedReq, nullptr);
-      return nullptr;
-    }
     // substitute() can invalidate iterators of NormalizationCache.
-    bool Failed = SubstituteParameterMappings(*this).substitute(*Normalized);
+    if (Normalized && SubstituteParameterMappings(*this).substitute(*Normalized))
+      Normalized = nullptr;
     CacheEntry =
         NormalizationCache.try_emplace(ConstrainedDeclOrNestedReq, Normalized)
             .first;
-    if (Failed)
-      return nullptr;
   }
   return CacheEntry->second;
 }
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 34c5c5d338bfe..2afc2d760bb75 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
@@ -7,10 +7,9 @@ template<typename T> concept Bar = Foo<T&>;  // #Bar
 template<typename T> requires Bar<T> struct S { }; // #S
 template<typename T> requires Bar<T> && true struct S<T> { }; // #SpecS
 // expected-error at -1 {{class template partial specialization is not more specialized than the primary template}}
-// expected-error@#Foo 2{{'type name' declared as a pointer to a reference of type 'T &'}}
+// expected-error@#Foo {{'type name' declared as a pointer to a reference of type 'T &'}}
 // expected-note@#SpecS {{while substituting into concept arguments here}}
-// expected-note@#S {{while substituting into concept arguments here}}
-// expected-note@#Bar 2{{while substituting into concept arguments here}}
+// expected-note@#Bar {{while substituting into concept arguments here}}
 // expected-note@#S {{template is declared here}}
 
 
@@ -86,11 +85,9 @@ requires true struct S3; // expected-note {{template is declared here}}
 template <True T, True U>
 requires true struct S3<T, U>;
 // expected-error at -1 {{class template partial specialization is not more specialized than the primary template}}
-// expected-error@#Foo2 2{{'type name' declared as a pointer to a reference of type 'T &'}}
-// expected-note@#SpecS2_1 {{while substituting into concept arguments here}}
+// expected-error@#Foo2 {{'type name' declared as a pointer to a reference of type 'T &'}}
 // expected-note@#SpecS2_2 {{while substituting into concept arguments here}}
 // expected-note@#S3_Header {{while substituting into concept arguments here}}
-// expected-note@#Bar2 {{while substituting into concept arguments here}}
 
 
 // Same as above, for the second position (but this was already working).
@@ -102,8 +99,6 @@ requires true struct S4<T, U>; // #S4-spec
 // expected-error@#Foo2 {{'type name' declared as a pointer to a reference of type 'U &'}}
 // expected-note@#S4_Header {{while substituting into concept arguments here}}
 // expected-note@#S4 {{template is declared here}}
-// expected-note@#S4 {{similar constraint expressions not considered equivalent}}
-// expected-note@#S4-spec {{similar constraint expression here}}