https://github.com/swjng updated https://github.com/llvm/llvm-project/pull/197647
>From c0349bf351c2715ef1cc496353f4693aa706a4a8 Mon Sep 17 00:00:00 2001 From: Soowon Jeong <[email protected]> Date: Thu, 14 May 2026 19:14:01 +0900 Subject: [PATCH 1/5] [clang][Parse] Skip late-parsed attributes after a fatal error When the diagnostic engine has entered a fatal-error state (e.g. after -ferror-limit is exceeded), name lookup performed by the late attribute parser can silently fail, leaving a RecoveryExpr in the attribute's condition. Attaching that value-dependent condition to a non-template declaration later trips an assertion in Expr::EvaluateWithSubstitution when the declaration is referenced (e.g. via diagnose_if). Bail out of ParseLexedAttribute when a fatal error has occurred: drain the cached tokens and return without attaching the half-formed attribute. --- clang/lib/Parse/ParseCXXInlineMethods.cpp | 14 ++++++ .../test/SemaCXX/diagnose-if-fatal-error.cpp | 46 +++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 clang/test/SemaCXX/diagnose-if-fatal-error.cpp diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp index 6189c854e5fbf..e78e3e6ff0c46 100644 --- a/clang/lib/Parse/ParseCXXInlineMethods.cpp +++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp @@ -757,6 +757,20 @@ void Parser::ParseLexedAttribute(LateParsedAttribute &LA, ParsedAttributes Attrs(AttrFactory); + // If a fatal error has occurred, semantic analysis is no longer reliable + // (name lookup may silently fail and wrap references in RecoveryExpr, + // producing value-dependent attribute conditions on non-template decls + // that later confuse the attribute consumers). Skip the parse and drain + // the cached tokens instead. The diagnostics emitted up to this point + // already justify failing the compilation. + if (Actions.getDiagnostics().hasFatalErrorOccurred()) { + while (Tok.isNot(tok::eof)) + ConsumeAnyToken(); + if (Tok.is(tok::eof) && Tok.getEofData() == AttrEnd.getEofData()) + ConsumeAnyToken(); + return; + } + if (LA.Decls.size() > 0) { Decl *D = LA.Decls[0]; NamedDecl *ND = dyn_cast<NamedDecl>(D); diff --git a/clang/test/SemaCXX/diagnose-if-fatal-error.cpp b/clang/test/SemaCXX/diagnose-if-fatal-error.cpp new file mode 100644 index 0000000000000..57dea815e9ac7 --- /dev/null +++ b/clang/test/SemaCXX/diagnose-if-fatal-error.cpp @@ -0,0 +1,46 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++17 -ferror-limit 19 -verify %s + +// After -ferror-limit is exceeded the parser enters a fatal-error state. +// Late-parsed attribute conditions must not be attached to the declaration +// in that state: name lookup of the condition's sub-expressions may fail +// silently and wrap the result in a RecoveryExpr, producing a +// value-dependent condition on a non-template function that later confuses +// the diagnose_if consumer (Expr::EvaluateWithSubstitution asserts on +// value-dependent expressions). +// We test that we do not crash in such cases (#197625). + +#define _diagnose_if(...) __attribute__((diagnose_if(__VA_ARGS__))) + +using size_t = decltype(sizeof(0)); +namespace std { +template <typename T> +struct initializer_list { + const T *p; + size_t s; + constexpr size_t size() const { return s; } +}; +} // namespace std + +template <typename T> +struct E { + void f(int i) _diagnose_if(i, "bad i", "error"); // expected-note 19 {{from 'diagnose_if' attribute on 'f'}} +}; +void blast() { + E<int> e; + e.f(1); e.f(1); e.f(1); e.f(1); e.f(1); // expected-error 5 {{bad i}} + e.f(1); e.f(1); e.f(1); e.f(1); e.f(1); // expected-error 5 {{bad i}} + e.f(1); e.f(1); e.f(1); e.f(1); e.f(1); // expected-error 5 {{bad i}} + e.f(1); e.f(1); e.f(1); e.f(1); e.f(1); // expected-error 4 {{bad i}} expected-error@* {{too many errors emitted}} +} + +struct Foo { + Foo(std::initializer_list<int> l) + _diagnose_if(l.size() == 1, "first", "warning") + _diagnose_if(l.size() == 2, "second", "error") {} +}; + +void run() { + Foo{std::initializer_list<int>{}}; + Foo{std::initializer_list<int>{1}}; + Foo{std::initializer_list<int>{1, 2}}; +} >From 6e9b4be7934f48866376bf1f296237486d39d80b Mon Sep 17 00:00:00 2001 From: Soowon Jeong <[email protected]> Date: Fri, 15 May 2026 10:27:53 +0900 Subject: [PATCH 2/5] [clang][Sema] Guard diagnose_if value-dependent conditions --- clang/lib/Parse/ParseCXXInlineMethods.cpp | 14 -------------- clang/lib/Sema/SemaOverload.cpp | 5 ++++- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp index e78e3e6ff0c46..6189c854e5fbf 100644 --- a/clang/lib/Parse/ParseCXXInlineMethods.cpp +++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp @@ -757,20 +757,6 @@ void Parser::ParseLexedAttribute(LateParsedAttribute &LA, ParsedAttributes Attrs(AttrFactory); - // If a fatal error has occurred, semantic analysis is no longer reliable - // (name lookup may silently fail and wrap references in RecoveryExpr, - // producing value-dependent attribute conditions on non-template decls - // that later confuse the attribute consumers). Skip the parse and drain - // the cached tokens instead. The diagnostics emitted up to this point - // already justify failing the compilation. - if (Actions.getDiagnostics().hasFatalErrorOccurred()) { - while (Tok.isNot(tok::eof)) - ConsumeAnyToken(); - if (Tok.is(tok::eof) && Tok.getEofData() == AttrEnd.getEofData()) - ConsumeAnyToken(); - return; - } - if (LA.Decls.size() > 0) { Decl *D = LA.Decls[0]; NamedDecl *ND = dyn_cast<NamedDecl>(D); diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index e11bbd7085798..d6db0cb19ddac 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -7801,7 +7801,10 @@ bool Sema::diagnoseArgDependentDiagnoseIfAttrs(const FunctionDecl *Function, // It's sane to use the same Args for any redecl of this function, since // EvaluateWithSubstitution only cares about the position of each // argument in the arg list, not the ParmVarDecl* it maps to. - if (!DIA->getCond()->EvaluateWithSubstitution( + // FIXME: This doesn't consider value-dependent cases, because doing so + // is very difficult. Ideally, we should handle them more gracefully. + if (DIA->getCond()->isValueDependent() || + !DIA->getCond()->EvaluateWithSubstitution( Result, Context, cast<FunctionDecl>(DIA->getParent()), Args, ThisArg)) return false; return Result.isInt() && Result.getInt().getBoolValue(); >From bc7ec8c397143a4b6b16442114703878ff321c14 Mon Sep 17 00:00:00 2001 From: Soowon Jeong <[email protected]> Date: Fri, 15 May 2026 10:31:40 +0900 Subject: [PATCH 3/5] clang-format --- clang/lib/Sema/SemaOverload.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index d6db0cb19ddac..bce725e8c658c 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -7805,7 +7805,8 @@ bool Sema::diagnoseArgDependentDiagnoseIfAttrs(const FunctionDecl *Function, // is very difficult. Ideally, we should handle them more gracefully. if (DIA->getCond()->isValueDependent() || !DIA->getCond()->EvaluateWithSubstitution( - Result, Context, cast<FunctionDecl>(DIA->getParent()), Args, ThisArg)) + Result, Context, cast<FunctionDecl>(DIA->getParent()), Args, + ThisArg)) return false; return Result.isInt() && Result.getInt().getBoolValue(); }); >From 68fdd17920e366c768b4ff0d29fc267b21fdc786 Mon Sep 17 00:00:00 2001 From: Soowon Jeong <[email protected]> Date: Tue, 19 May 2026 12:26:28 +0900 Subject: [PATCH 4/5] Simplify regression test Co-Authored-By: Claude Opus 4.6 <[email protected]> --- .../test/SemaCXX/diagnose-if-fatal-error.cpp | 49 +++++-------------- 1 file changed, 11 insertions(+), 38 deletions(-) diff --git a/clang/test/SemaCXX/diagnose-if-fatal-error.cpp b/clang/test/SemaCXX/diagnose-if-fatal-error.cpp index 57dea815e9ac7..6ce806b62571e 100644 --- a/clang/test/SemaCXX/diagnose-if-fatal-error.cpp +++ b/clang/test/SemaCXX/diagnose-if-fatal-error.cpp @@ -1,46 +1,19 @@ -// RUN: %clang_cc1 -fsyntax-only -std=c++17 -ferror-limit 19 -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c++17 -ferror-limit 1 -verify %s +// Regression test for #197625: diagnose_if condition becomes value-dependent +// via RecoveryExpr after fatal error, hitting an assertion in EvaluateWithSubstitution. -// After -ferror-limit is exceeded the parser enters a fatal-error state. -// Late-parsed attribute conditions must not be attached to the declaration -// in that state: name lookup of the condition's sub-expressions may fail -// silently and wrap the result in a RecoveryExpr, producing a -// value-dependent condition on a non-template function that later confuses -// the diagnose_if consumer (Expr::EvaluateWithSubstitution asserts on -// value-dependent expressions). -// We test that we do not crash in such cases (#197625). - -#define _diagnose_if(...) __attribute__((diagnose_if(__VA_ARGS__))) - -using size_t = decltype(sizeof(0)); -namespace std { -template <typename T> -struct initializer_list { - const T *p; - size_t s; - constexpr size_t size() const { return s; } +template <typename T> struct S { + T val; + constexpr T get() const { return val; } }; -} // namespace std -template <typename T> -struct E { - void f(int i) _diagnose_if(i, "bad i", "error"); // expected-note 19 {{from 'diagnose_if' attribute on 'f'}} -}; +void bad(int i) __attribute__((diagnose_if(i, "bad", "error"))); // expected-note {{from 'diagnose_if'}} void blast() { - E<int> e; - e.f(1); e.f(1); e.f(1); e.f(1); e.f(1); // expected-error 5 {{bad i}} - e.f(1); e.f(1); e.f(1); e.f(1); e.f(1); // expected-error 5 {{bad i}} - e.f(1); e.f(1); e.f(1); e.f(1); e.f(1); // expected-error 5 {{bad i}} - e.f(1); e.f(1); e.f(1); e.f(1); e.f(1); // expected-error 4 {{bad i}} expected-error@* {{too many errors emitted}} + bad(1); // expected-error {{bad}} + bad(1); // expected-error@* {{too many errors}} } struct Foo { - Foo(std::initializer_list<int> l) - _diagnose_if(l.size() == 1, "first", "warning") - _diagnose_if(l.size() == 2, "second", "error") {} + Foo(S<int> s) __attribute__((diagnose_if(s.get() == 1, "x", "warning"))) {} }; - -void run() { - Foo{std::initializer_list<int>{}}; - Foo{std::initializer_list<int>{1}}; - Foo{std::initializer_list<int>{1, 2}}; -} +void run() { Foo{S<int>{1}}; } >From 58c860a87e4149ffb7e3dcef5aa82b6f0160cb26 Mon Sep 17 00:00:00 2001 From: Soowon Jeong <[email protected]> Date: Tue, 19 May 2026 12:50:56 +0900 Subject: [PATCH 5/5] Add template instantiation test for diagnose_if guard Co-Authored-By: Claude Opus 4.6 <[email protected]> --- clang/test/SemaCXX/diagnose-if-fatal-error.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/clang/test/SemaCXX/diagnose-if-fatal-error.cpp b/clang/test/SemaCXX/diagnose-if-fatal-error.cpp index 6ce806b62571e..54b4175923be5 100644 --- a/clang/test/SemaCXX/diagnose-if-fatal-error.cpp +++ b/clang/test/SemaCXX/diagnose-if-fatal-error.cpp @@ -7,9 +7,16 @@ template <typename T> struct S { constexpr T get() const { return val; } }; -void bad(int i) __attribute__((diagnose_if(i, "bad", "error"))); // expected-note {{from 'diagnose_if'}} +// The isValueDependent() guard must not suppress diagnose_if after instantiation. +struct Bar { + template <typename T> + Bar(S<T> s) __attribute__((diagnose_if(s.get() == 1, "one", "error"))) {} // expected-note {{from 'diagnose_if'}} +}; +void instantiated() { Bar{S<int>{1}}; } // expected-error {{one}} + +// After fatal error, the guard must skip evaluation instead of crashing. +void bad(int i) __attribute__((diagnose_if(i, "bad", "error"))); void blast() { - bad(1); // expected-error {{bad}} bad(1); // expected-error@* {{too many errors}} } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
