[clang] [Clang] FunctionEffects: Make a separate diagnostic group for redeclarations/overrides where effects are implicit. (PR #148690)
Doug Wyatt via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 15 08:21:33 PDT 2025
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 <dwyatt at apple.com>
Date: Mon, 14 Jul 2025 10:44:56 -0700
Subject: [PATCH 1/5] [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 <dwyatt at apple.com>
Date: Mon, 14 Jul 2025 11:22:12 -0700
Subject: [PATCH 2/5] 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 <doug at sonosphere.com>
Date: Tue, 15 Jul 2025 08:07:13 -0700
Subject: [PATCH 3/5] Update clang/lib/Sema/SemaDeclCXX.cpp
Co-authored-by: Sirraide <aeternalmail at 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 <dwyatt at apple.com>
Date: Tue, 15 Jul 2025 08:08:34 -0700
Subject: [PATCH 4/5] 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 <dwyatt at apple.com>
Date: Tue, 15 Jul 2025 08:17:57 -0700
Subject: [PATCH 5/5] 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)
More information about the cfe-commits
mailing list