Skip to content

Conversation

@dougsonos
Copy link
Contributor

In adopting [[clang::nonblocking]] there's been some user confusion. Changes to address -Wfunction-effects warnings are often pure annotation, with no runtime effect. Changes to avoid -Wperf-constraint-implies-noexcept warnings are risky: adding noexcept creates a new potential for the program to crash. In retrospect, -Wperf-constraint-implies-noexcept shouldn't have been made part of -Wall.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 11, 2025
@dougsonos dougsonos requested a review from Sirraide November 11, 2025 17:08
@llvmbot
Copy link
Member

llvmbot commented Nov 11, 2025

@llvm/pr-subscribers-clang

Author: Doug Wyatt (dougsonos)

Changes

In adopting [[clang::nonblocking]] there's been some user confusion. Changes to address -Wfunction-effects warnings are often pure annotation, with no runtime effect. Changes to avoid -Wperf-constraint-implies-noexcept warnings are risky: adding noexcept creates a new potential for the program to crash. In retrospect, -Wperf-constraint-implies-noexcept shouldn't have been made part of -Wall.


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

2 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2-1)
  • (modified) clang/include/clang/Basic/DiagnosticGroups.td (+3-3)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 980dbf1ff2cf6..fcad4c0fb7133 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -350,6 +350,8 @@ Improvements to Clang's diagnostics
   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.
+- Fixed several false positives and false negatives in function effect (`nonblocking`) analysis. (#GH166078) (#GH166101) (#GH166110)
+- Remove ``-Wperf-constraint-implies-noexcept`` from ``-Wall``.
 - Implemented diagnostics when retrieving the tuple size for types where its specialization of `std::tuple_size`
   produces an invalid size (either negative or greater than the implementation limit). (#GH159563)
 - Fixed fix-it hint for fold expressions. Clang now correctly places the suggested right
@@ -482,7 +484,6 @@ Bug Fixes to Attribute Support
 - Fix a crash when the function name is empty in the `swift_name` attribute. (#GH157075)
 - Fixes crashes or missing diagnostics with the `device_kernel` attribute. (#GH161905)
 - Fix handling of parameter indexes when an attribute is applied to a C++23 explicit object member function.
-- Fixed several false positives and false negatives in function effect (`nonblocking`) analysis. (#GH166078) (#GH166101) (#GH166110)
 
 Bug Fixes to C++ Support
 ^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 1e0321de3f4b6..8ec9f47c51fda 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1312,9 +1312,9 @@ def Consumed       : DiagGroup<"consumed">;
 // Note that putting warnings in -Wall will not disable them by default. If a
 // warning should be active _only_ when -Wall is passed in, mark it as
 // DefaultIgnore in addition to putting it here.
-def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool,
-                            MisleadingIndentation, PackedNonPod,
-                            VLACxxExtension, PerfConstraintImpliesNoexcept]>;
+def All
+    : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool,
+                        MisleadingIndentation, PackedNonPod, VLACxxExtension]>;
 
 // Warnings that should be in clang-cl /w4.
 def : DiagGroup<"CL4", [All, Extra]>;

Copy link
Member

@Sirraide Sirraide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we generally prefer fewer default-off warnings, this is specialised enough to where I think it’s fine to have it disabled by default.

I haven’t looked too closely at the CI failures, but you might have to add -Wperf-constraint-implies-noexcept to a few RUN lines.

@dougsonos
Copy link
Contributor Author

I haven’t looked too closely at the CI failures, but you might have to add -Wperf-constraint-implies-noexcept to a few RUN lines.

Thanks. Clearly I forgot what other tests to run, beyond ../clang/tests/Sema/attr-nonblocking* ...

@Sirraide
Copy link
Member

I haven’t looked too closely at the CI failures, but you might have to add -Wperf-constraint-implies-noexcept to a few RUN lines.

Thanks. Clearly I forgot what other tests to run, beyond ../clang/tests/Sema/attr-nonblocking* ...

FYI, you can run the entire test suite w/ ninja check-clang (or make check-clang I guess if you’re using make as a generator).

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

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.
- Remove ``-Wperf-constraint-implies-noexcept`` from ``-Wall``. This warning is somewhat pedantic and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh use of "pedantic" here may cause folks to think -pedantic is where it should live. Maybe "nit-picky"?

@dougsonos dougsonos merged commit 09122fe into llvm:main Nov 13, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants