https://github.com/dougsonos updated https://github.com/llvm/llvm-project/pull/148690
>From ea41b2d036ec9f857be107408b3a2b557d377304 Mon Sep 17 00:00:00 2001 From: Doug Wyatt <dwy...@apple.com> Date: Mon, 14 Jul 2025 10:44:56 -0700 Subject: [PATCH 1/6] [Clang] FunctionEffects: Make a separate diagnostic group for redeclarations/overrides where effects are implicit. --- clang/include/clang/Basic/DiagnosticGroups.td | 1 + clang/include/clang/Basic/DiagnosticSemaKinds.td | 15 ++++++++++----- clang/lib/Sema/SemaDeclCXX.cpp | 10 +++++++++- clang/test/Sema/attr-nonblocking-sema.cpp | 14 +++++++------- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 9a7a308600763..b59439d26a71a 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1291,6 +1291,7 @@ def ThreadSafetyBeta : DiagGroup<"thread-safety-beta">; // Warnings and notes related to the function effects system which underlies // the nonblocking and nonallocating attributes. def FunctionEffects : DiagGroup<"function-effects">; +def FunctionEffectRedeclarations : DiagGroup<"function-effect-redeclarations">; def PerfConstraintImpliesNoexcept : DiagGroup<"perf-constraint-implies-noexcept">; // Uniqueness Analysis warnings diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 577adc30ba2fa..2b1fa4330d325 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11529,17 +11529,22 @@ def note_in_evaluating_default_argument : Note< def warn_invalid_add_func_effects : Warning< "attribute '%0' should not be added via type conversion">, InGroup<FunctionEffects>, DefaultIgnore; -def warn_mismatched_func_effect_override : Warning< - "attribute '%0' on overriding function does not match base declaration">, - InGroup<FunctionEffects>, DefaultIgnore; -def warn_mismatched_func_effect_redeclaration : Warning< - "attribute '%0' on function does not match previous declaration">, +def warn_conflicting_func_effect_override : Warning< + "attribute '%0' on overriding function conflicts with base declaration">, InGroup<FunctionEffects>, DefaultIgnore; def warn_conflicting_func_effects : Warning< "effects conflict when merging declarations; kept '%0', discarded '%1'">, InGroup<FunctionEffects>, DefaultIgnore; def err_func_with_effects_no_prototype : Error< "'%0' function must have a prototype">; +// These are more pedantic: in redeclarations and virtual method overrides, +// the effect attribute(s) should be restated. +def warn_mismatched_func_effect_override : Warning< + "overriding function is missing '%0' attribute from base declaration">, + InGroup<FunctionEffectRedeclarations>, DefaultIgnore; +def warn_mismatched_func_effect_redeclaration : Warning< + "redeclaration is missing '%0' attribute from previous declaration">, + InGroup<FunctionEffectRedeclarations>, DefaultIgnore; } // end of sema category diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 5cc92ebb0171f..090a1d9d290bb 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -18686,7 +18686,7 @@ bool Sema::CheckOverridingFunctionAttributes(CXXMethodDecl *New, case FunctionEffectDiff::OverrideResult::NoAction: break; case FunctionEffectDiff::OverrideResult::Warn: - Diag(New->getLocation(), diag::warn_mismatched_func_effect_override) + Diag(New->getLocation(), diag::warn_conflicting_func_effect_override) << Diff.effectName(); Diag(Old->getLocation(), diag::note_overridden_virtual_function) << Old->getReturnTypeSourceRange(); @@ -18699,6 +18699,14 @@ bool Sema::CheckOverridingFunctionAttributes(CXXMethodDecl *New, QualType ModQT = Context.getFunctionType(NewFT->getReturnType(), NewFT->getParamTypes(), EPI); New->setType(ModQT); + if (Errs.empty()) { + // A warning here is somewhat pedantic, in a different warning group. + // Skip this if there was already a merge conflict, which is more serious. + Diag(New->getLocation(), diag::warn_mismatched_func_effect_override) + << Diff.effectName(); + Diag(Old->getLocation(), diag::note_overridden_virtual_function) + << Old->getReturnTypeSourceRange(); + } break; } } diff --git a/clang/test/Sema/attr-nonblocking-sema.cpp b/clang/test/Sema/attr-nonblocking-sema.cpp index f13cc783dfc33..b8a9071aae50c 100644 --- a/clang/test/Sema/attr-nonblocking-sema.cpp +++ b/clang/test/Sema/attr-nonblocking-sema.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -fsyntax-only -fblocks -fcxx-exceptions -verify -Wfunction-effects %s -// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -x c -std=c23 -Wfunction-effects %s +// RUN: %clang_cc1 -fsyntax-only -fblocks -fcxx-exceptions -verify -Wfunction-effects -Wfunction-effect-redeclarations %s +// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -x c -std=c23 -Wfunction-effects -Wfunction-effect-redeclarations %s #if !__has_attribute(nonblocking) #error "the 'nonblocking' attribute is not available" @@ -132,15 +132,15 @@ void type_conversions_2() #ifdef __cplusplus struct Base { virtual void f1(); - virtual void nonblocking() noexcept [[clang::nonblocking]]; - virtual void nonallocating() noexcept [[clang::nonallocating]]; + virtual void nonblocking() noexcept [[clang::nonblocking]]; // expected-note {{overridden virtual function is here}} + virtual void nonallocating() noexcept [[clang::nonallocating]]; // expected-note {{overridden virtual function is here}} virtual void f2() [[clang::nonallocating]]; // expected-note {{previous declaration is here}} }; struct Derived : public Base { void f1() [[clang::nonblocking]] override; - void nonblocking() noexcept override; - void nonallocating() noexcept override; + void nonblocking() noexcept override; // expected-warning {{overriding function is missing 'nonblocking' attribute from base declaration}} + void nonallocating() noexcept override; // expected-warning {{overriding function is missing 'nonallocating' attribute from base declaration}} void f2() [[clang::allocating]] override; // expected-warning {{effects conflict when merging declarations; kept 'allocating', discarded 'nonallocating'}} }; #endif // __cplusplus @@ -149,7 +149,7 @@ struct Derived : public Base { void f2(); void f2() [[clang::nonblocking]]; // expected-note {{previous declaration is here}} -void f2(); // expected-warning {{attribute 'nonblocking' on function does not match previous declaration}} +void f2(); // expected-warning {{redeclaration is missing 'nonblocking' attribute from previous declaration}} // Note: we verify that the attribute is actually seen during the constraints tests. void f3() [[clang::blocking]]; // expected-note {{previous declaration is here}} >From 2a71d8a0f426b6dd2275a2c4ef6f2e41b9bfd492 Mon Sep 17 00:00:00 2001 From: Doug Wyatt <dwy...@apple.com> Date: Mon, 14 Jul 2025 11:22:12 -0700 Subject: [PATCH 2/6] clang-format --- .../clang/Basic/DiagnosticSemaKinds.td | 24 ++++++++++++------- clang/lib/Sema/SemaDeclCXX.cpp | 5 ++-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 2b1fa4330d325..e37e9e0cb8031 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11529,9 +11529,11 @@ def note_in_evaluating_default_argument : Note< def warn_invalid_add_func_effects : Warning< "attribute '%0' should not be added via type conversion">, InGroup<FunctionEffects>, DefaultIgnore; -def warn_conflicting_func_effect_override : Warning< - "attribute '%0' on overriding function conflicts with base declaration">, - InGroup<FunctionEffects>, DefaultIgnore; +def warn_conflicting_func_effect_override + : Warning<"attribute '%0' on overriding function conflicts with base " + "declaration">, + InGroup<FunctionEffects>, + DefaultIgnore; def warn_conflicting_func_effects : Warning< "effects conflict when merging declarations; kept '%0', discarded '%1'">, InGroup<FunctionEffects>, DefaultIgnore; @@ -11539,12 +11541,16 @@ def err_func_with_effects_no_prototype : Error< "'%0' function must have a prototype">; // These are more pedantic: in redeclarations and virtual method overrides, // the effect attribute(s) should be restated. -def warn_mismatched_func_effect_override : Warning< - "overriding function is missing '%0' attribute from base declaration">, - InGroup<FunctionEffectRedeclarations>, DefaultIgnore; -def warn_mismatched_func_effect_redeclaration : Warning< - "redeclaration is missing '%0' attribute from previous declaration">, - InGroup<FunctionEffectRedeclarations>, DefaultIgnore; +def warn_mismatched_func_effect_override + : Warning<"overriding function is missing '%0' attribute from base " + "declaration">, + InGroup<FunctionEffectRedeclarations>, + DefaultIgnore; +def warn_mismatched_func_effect_redeclaration + : Warning< + "redeclaration is missing '%0' attribute from previous declaration">, + InGroup<FunctionEffectRedeclarations>, + DefaultIgnore; } // end of sema category diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 090a1d9d290bb..3f3ba7b32eca4 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -18700,8 +18700,9 @@ bool Sema::CheckOverridingFunctionAttributes(CXXMethodDecl *New, NewFT->getParamTypes(), EPI); New->setType(ModQT); if (Errs.empty()) { - // A warning here is somewhat pedantic, in a different warning group. - // Skip this if there was already a merge conflict, which is more serious. + // A warning here is somewhat pedantic, in a different warning + // group. Skip this if there was already a merge conflict, which is + // more serious. Diag(New->getLocation(), diag::warn_mismatched_func_effect_override) << Diff.effectName(); Diag(Old->getLocation(), diag::note_overridden_virtual_function) >From 4326b88f37a1c3864a662217b236e5993c760958 Mon Sep 17 00:00:00 2001 From: Doug Wyatt <d...@sonosphere.com> Date: Tue, 15 Jul 2025 08:07:13 -0700 Subject: [PATCH 3/6] Update clang/lib/Sema/SemaDeclCXX.cpp Co-authored-by: Sirraide <aeternalm...@gmail.com> --- clang/lib/Sema/SemaDeclCXX.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 3f3ba7b32eca4..4efd7fe9cd869 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -18700,9 +18700,8 @@ bool Sema::CheckOverridingFunctionAttributes(CXXMethodDecl *New, NewFT->getParamTypes(), EPI); New->setType(ModQT); if (Errs.empty()) { - // A warning here is somewhat pedantic, in a different warning - // group. Skip this if there was already a merge conflict, which is - // more serious. + // A warning here is somewhat pedantic. Skip this if there was already + // a merge conflict, which is more serious. Diag(New->getLocation(), diag::warn_mismatched_func_effect_override) << Diff.effectName(); Diag(Old->getLocation(), diag::note_overridden_virtual_function) >From 81acc832576a8cc66ee3f71be7b06a6c87c97a0d Mon Sep 17 00:00:00 2001 From: Doug Wyatt <dwy...@apple.com> Date: Tue, 15 Jul 2025 08:08:34 -0700 Subject: [PATCH 4/6] Add test for conflicting attribute on virtual override. --- clang/test/Sema/attr-nonblocking-sema.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/clang/test/Sema/attr-nonblocking-sema.cpp b/clang/test/Sema/attr-nonblocking-sema.cpp index b8a9071aae50c..c8fb40693eec0 100644 --- a/clang/test/Sema/attr-nonblocking-sema.cpp +++ b/clang/test/Sema/attr-nonblocking-sema.cpp @@ -127,7 +127,7 @@ void type_conversions_2() #endif // --- VIRTUAL METHODS --- -// Attributes propagate to overridden methods, so no diagnostics except for conflicts. +// Attributes propagate to overridden methods. // Check this in the syntax tests too. #ifdef __cplusplus struct Base { @@ -135,6 +135,7 @@ struct Base { virtual void nonblocking() noexcept [[clang::nonblocking]]; // expected-note {{overridden virtual function is here}} virtual void nonallocating() noexcept [[clang::nonallocating]]; // expected-note {{overridden virtual function is here}} virtual void f2() [[clang::nonallocating]]; // expected-note {{previous declaration is here}} + virtual void f3() [[clang::nonblocking]]; // expected-note {{overridden virtual function is here}} }; struct Derived : public Base { @@ -143,6 +144,11 @@ struct Derived : public Base { void nonallocating() noexcept override; // expected-warning {{overriding function is missing 'nonallocating' attribute from base declaration}} void f2() [[clang::allocating]] override; // expected-warning {{effects conflict when merging declarations; kept 'allocating', discarded 'nonallocating'}} }; + +template <bool B> +struct TDerived : public Base { + void f3() [[clang::nonblocking(B)]] override; // expected-warning {{attribute 'nonblocking' on overriding function conflicts with base declaration}} +}; #endif // __cplusplus // --- REDECLARATIONS --- >From f7c9c8fa19cfebc0901ab3e8f99b45c49753c154 Mon Sep 17 00:00:00 2001 From: Doug Wyatt <dwy...@apple.com> Date: Tue, 15 Jul 2025 08:17:57 -0700 Subject: [PATCH 5/6] Release note, clang-format. --- clang/docs/ReleaseNotes.rst | 6 ++++++ clang/lib/Sema/SemaDeclCXX.cpp | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 970825c98fec1..107de72d11c9d 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -707,6 +707,12 @@ Improvements to Clang's diagnostics - Improve the diagnostics for placement new expression when const-qualified object was passed as the storage argument. (#GH143708) +- Added a separate diagnostic group `-Wfunction-effect-redeclarations``, for the more pedantic + diagnostics for function effects (``[[clang::nonblocking]]`` and ``[[clang::nonallocating]]``). + Moved the warning for a missing (though implied) attribute on a redeclaration into this group. + Added a new warning in this group for the case where the attribute is missing/implicit on + an override of a virtual method. + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 4efd7fe9cd869..d72c827da677b 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -18700,8 +18700,8 @@ bool Sema::CheckOverridingFunctionAttributes(CXXMethodDecl *New, NewFT->getParamTypes(), EPI); New->setType(ModQT); if (Errs.empty()) { - // A warning here is somewhat pedantic. Skip this if there was already - // a merge conflict, which is more serious. + // A warning here is somewhat pedantic. Skip this if there was + // already a merge conflict, which is more serious. Diag(New->getLocation(), diag::warn_mismatched_func_effect_override) << Diff.effectName(); Diag(Old->getLocation(), diag::note_overridden_virtual_function) >From de36f93a93d588879961de655b6793f4cb4825a2 Mon Sep 17 00:00:00 2001 From: Doug Wyatt <d...@sonosphere.com> Date: Tue, 15 Jul 2025 08:25:51 -0700 Subject: [PATCH 6/6] Update clang/docs/ReleaseNotes.rst Fix typo Co-authored-by: Sirraide <aeternalm...@gmail.com> --- clang/docs/ReleaseNotes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 9a0e2488dab71..673ee4a5f7842 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -710,7 +710,7 @@ Improvements to Clang's diagnostics pointer, provided it can be proven that the pointer only points to ``[[noreturn]]`` functions. -- Added a separate diagnostic group `-Wfunction-effect-redeclarations``, for the more pedantic +- Added a separate diagnostic group ``-Wfunction-effect-redeclarations``, for the more pedantic diagnostics for function effects (``[[clang::nonblocking]]`` and ``[[clang::nonallocating]]``). Moved the warning for a missing (though implied) attribute on a redeclaration into this group. Added a new warning in this group for the case where the attribute is missing/implicit on _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits