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

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Nico Weber (nico)

<details>
<summary>Changes</summary>

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.

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


2 Files Affected:

- (modified) clang/lib/Sema/SemaConcept.cpp (+2-7) 
- (modified) clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp (+3-8) 


``````````diff
The server is unavailable at this time. Please wait a few minutes before you try again.
``````````

</details>


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