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 <[email protected]>
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@-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@-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}}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
