[clang] Bailout when the substitution of template parameter mapping is invalid.

[hokein]

Fixes #86757

We missed to handle the invalid case when substituting into the parameter mapping of an constraint during normalization.
The constructor of InstantiatingTemplate will bail out (no CodeSynthesisContext will be added to the instantiation stack) if there was a fatal error, consequently we should stop doing any further template instantiations.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

Fixes #86757

We missed to handle the invalid case when substituting into the parameter mapping of an constraint during normalization.

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/Sema/SemaConcept.cpp (+2)
• (added) clang/test/SemaTemplate/concepts-GH86757.cpp (+13)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0fdd9e3fb3eee2..1411e15a649ae0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -455,6 +455,7 @@ Bug Fixes to C++ Support
 - Fix an issue where a namespace alias could be defined using a qualified name (all name components
   following the first `::` were ignored).
 - Fix an out-of-bounds crash when checking the validity of template partial specializations. (part of #GH86757).
+- Fix an issue caused by not handling invalid cases when substituting into the parameter mapping of a constraint. Fixes (#GH86757).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index b6c4d3d540ef50..a2d8ba9a96d7a4 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -1281,6 +1281,8 @@ substituteParameterMappings(Sema &S, NormalizedConstraint &N,
       S, InstLocBegin,
       Sema::InstantiatingTemplate::ParameterMappingSubstitution{}, Concept,
       {InstLocBegin, InstLocEnd});
+  if (Inst.isInvalid())
+    return true;
   if (S.SubstTemplateArguments(*Atomic.ParameterMapping, MLTAL, SubstArgs))
     return true;
 
diff --git a/clang/test/SemaTemplate/concepts-GH86757.cpp b/clang/test/SemaTemplate/concepts-GH86757.cpp
new file mode 100644
index 00000000000000..3122381b20359e
--- /dev/null
+++ b/clang/test/SemaTemplate/concepts-GH86757.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -Wfatal-errors -verify %s
+
+template <typename> int a;
+template <typename... b> concept c = a<b...>;
+template <typename> concept e = c<>;
+
+// must be a fatal error to trigger the crash
+undefined; // expected-error {{a type specifier is required for all declarations}}
+
+template <typename d> concept g = e<d>;
+template <g> struct h
+template <g d>
+struct h<d>;

[Sirraide]

LGTM

[Sirraide]

Side note: Now it’s starting to make sense why something like an unknown include was triggering this otherwise entirely unrelated failure: The first thing that the constructor of InstantiatingTemplate does is bail out if there was a fatal error.

[zyn0217]

Thanks! This reminds me that I should also check the validity of InstantiatingTemplate after introducing a new CodeSynthesisContext in my patch of fixing lambda calls within type alias decls.

[shafik]

LGTM, interesting it looks like we don't do this check in fromContraintExpr either.

[hokein]

LGTM, interesting it looks like we don't do this check in fromContraintExpr either.

Yes, I think it is an oversight, we should do the check for it as well.