https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/75697
>From a6e01586f5d406b51492d973cb693ba32cc63d99 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Sat, 16 Dec 2023 18:49:59 +0800 Subject: [PATCH 1/4] [Concepts] Avoid substituting into constraints for invalid TemplateDecls Fixes https://github.com/llvm/llvm-project/issues/73885. Substituting into constraints for invalid TemplateDecls might still yield dependent expressions and end up crashing later while attempting evaluation. --- clang/docs/ReleaseNotes.rst | 3 +++ clang/lib/Sema/SemaConcept.cpp | 4 ++++ clang/test/SemaTemplate/instantiate-requires-expr.cpp | 10 ++++++++++ 3 files changed, 17 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b4b5352a306c1..83f98b9792525 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -831,6 +831,9 @@ Bug Fixes to C++ Support - Fix crash when parsing nested requirement. Fixes: (`#73112 <https://github.com/llvm/llvm-project/issues/73112>`_) +- Fixed a crash when substituting into constraint expressions for invalid variable templates. + Fixes: (`#73885 <https://github.com/llvm/llvm-project/issues/73885>`_) + Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ - Fixed an import failure of recursive friend class template. diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index 719c6aab74e01..73683fee7c148 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -353,6 +353,10 @@ static ExprResult calculateConstraintSatisfaction( if (Inst.isInvalid()) return ExprError(); + // An empty expression for substitution failure messages. + if (Template && Template->isInvalidDecl()) + return ExprEmpty(); + llvm::FoldingSetNodeID ID; if (Template && DiagRecursiveConstraintEval(S, ID, Template, AtomicExpr, MLTAL)) { diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp index ba82fc1313fc9..fb4127182d1cb 100644 --- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp +++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp @@ -227,3 +227,13 @@ struct r6 {}; using r6i = r6<int>; // expected-error@-1 {{constraints not satisfied for class template 'r6' [with T = int]}} + +namespace GH73885 { +template <typename T> // expected-error {{extraneous template parameter list}} +template <typename T> // expected-error {{'T' shadows template parameter}} + // expected-note@-2 {{template parameter is declared here}} +requires(T{}) +T e_v; +double e = e_v<double>; +// expected-error@-1 {{constraints not satisfied for variable template 'e_v' [with T = double]}} +} >From 4085b4fa5868f62f970d2f3e2b15b663bf4f7a3a Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Fri, 12 Jul 2024 22:33:05 +0800 Subject: [PATCH 2/4] Revert "[Concepts] Avoid substituting into constraints for invalid TemplateDecls" This reverts commit a6e01586f5d406b51492d973cb693ba32cc63d99. --- clang/docs/ReleaseNotes.rst | 3 --- clang/lib/Sema/SemaConcept.cpp | 4 ---- clang/test/SemaTemplate/instantiate-requires-expr.cpp | 10 ---------- 3 files changed, 17 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index fa79d27cc84d4..838cb69f647ee 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -976,9 +976,6 @@ Bug Fixes to C++ Support - Fixed a bug where references to lambda capture inside a ``noexcept`` specifier were not correctly instantiated. (#GH95735). -- Fixed a crash when substituting into constraint expressions for invalid variable templates. - Fixes: (`#73885 <https://github.com/llvm/llvm-project/issues/73885>`_) - Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ - Clang now properly preserves ``FoundDecls`` within a ``ConceptReference``. (#GH82628) diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index 52628bdcc304b..202dd86c67f62 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -353,10 +353,6 @@ static ExprResult calculateConstraintSatisfaction( if (Inst.isInvalid()) return ExprError(); - // An empty expression for substitution failure messages. - if (Template && Template->isInvalidDecl()) - return ExprEmpty(); - llvm::FoldingSetNodeID ID; if (Template && DiagRecursiveConstraintEval(S, ID, Template, AtomicExpr, MLTAL)) { diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp index 90f16ce4c6f33..516708bf4c875 100644 --- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp +++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp @@ -227,13 +227,3 @@ struct r6 {}; using r6i = r6<int>; // expected-error@-1 {{constraints not satisfied for class template 'r6' [with T = int]}} - -namespace GH73885 { -template <typename T> // expected-error {{extraneous template parameter list}} -template <typename T> // expected-error {{'T' shadows template parameter}} - // expected-note@-2 {{template parameter is declared here}} -requires(T{}) -T e_v; -double e = e_v<double>; -// expected-error@-1 {{constraints not satisfied for variable template 'e_v' [with T = double]}} -} >From 1680fa415f8b871b71bf7b76e78177ea5bf54b8d Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Wed, 17 Jul 2024 14:41:15 +0800 Subject: [PATCH 3/4] Move the check to CheckConstraintSatisfaction() --- clang/docs/ReleaseNotes.rst | 1 + clang/lib/Sema/SemaConcept.cpp | 7 +++++++ .../SemaTemplate/instantiate-requires-expr.cpp | 17 +++++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 615e8879ac474..0c9d4de3d709a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -1039,6 +1039,7 @@ Bug Fixes to C++ Support - Fixed failed assertion when resolving context of defaulted comparison method outside of struct. (#GH96043). - Clang now diagnoses explicit object parameters in member pointers and other contexts where they should not appear. Fixes (#GH85992). +- Fixed a crash-on-invalid bug involving extraneous template parameter with concept substitution. (#GH73885) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index 54891150da20f..e58232856bd2d 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -483,6 +483,13 @@ bool Sema::CheckConstraintSatisfaction( *this, nullptr, ConstraintExprs, ConvertedConstraints, TemplateArgsLists, TemplateIDRange, OutSatisfaction); } + // Invalid templates could make their way here. Substituting them could result + // in dependent expressions. + // FIXME: Say something in the diagnostic note? + if (Template->isInvalidDecl()) { + OutSatisfaction.IsSatisfied = false; + return true; + } // A list of the template argument list flattened in a predictible manner for // the purposes of caching. The ConstraintSatisfaction type is in AST so it diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp index 516708bf4c875..934f22ce65887 100644 --- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp +++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp @@ -227,3 +227,20 @@ struct r6 {}; using r6i = r6<int>; // expected-error@-1 {{constraints not satisfied for class template 'r6' [with T = int]}} + +namespace GH73885 { + +template <class> // expected-error {{extraneous}} +template <class T> requires(T{}) +bool e_v = true; + +static_assert(e_v<bool>); + +// We do accept the following as an extension. See Sema::MatchTemplateParametersToScopeSpecifier() +template <> // expected-warning {{extraneous}} +template <class T> requires(T{}) +bool e = true; + +static_assert(e_v<bool>); + +} // namespace GH73885 >From 139b6820c22da0e324ca52cc37c9e8a13a0cd2bd Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Wed, 17 Jul 2024 14:45:02 +0800 Subject: [PATCH 4/4] fixup --- clang/test/SemaTemplate/instantiate-requires-expr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp index 934f22ce65887..835e04d6a2911 100644 --- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp +++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp @@ -241,6 +241,6 @@ template <> // expected-warning {{extraneous}} template <class T> requires(T{}) bool e = true; -static_assert(e_v<bool>); +static_assert(e<bool>); } // namespace GH73885 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
