Could I please ping this one? Thanks! https://gcc.gnu.org/pipermail/gcc-patches/2026-July/724528.html
-Lewis On Sun, Jul 19, 2026 at 10:26 PM Lewis Hyatt <[email protected]> wrote: > > Hello- > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124240 > > This is a small patch to fix up the issue with #pragma GCC diagnostic noted > in the PR. OK? > > bootstrap + regtest all languages on x86-64 and aarch64 (cfarm425). > > Thanks! > > -Lewis > > -- >8 -- > > There are two distinct properties of a diagnostic that determine whether or > not it will be emitted: > > 1) Enabled/Disabled > A diagnostic can be globally enabled or disabled. Most commonly, a > diagnostic is enabled by passing an argument -Wxyz or -Werror=xyz > and disabled by -Wno-xyz. A diagnostic can also be enabled by > `#pragma GCC diagnostic'. > > 2) Classification > A diagnostic can be classifed as an error or a warning, or it can be > classified as "ignored" to suppress it. A diagnostic can be > classified as an error or a warning by -Werror=xyz or -Wno-error=xyz > (the former also enables the diagnostic; the latter does not enable > or disable it, but classifies it as a warning so that, if it is later > enabled, it will be a warning and not an error.) The default > classification of all enabled diagnostics can be set to "error" with > -Werror or to "warning" with -Wno-error. > > A diagnostic will be suppressed if either it is not enabled, or if it is > enabled but classified as "ignored". While the visible outcome is the same > in either case, the states are not identical. (For example, the work done to > diagnose an issue is done iff the diagnostic is enabled, regardless if it > will later be suppressed.) > > The implementation of `#pragma GCC diagnostic' does not currently > distinguish between these two properties sufficiently to get all possible > cases correct, as shown in the PR. The intended logic implemented in > diagnostics/option-classifier.cc is: > > -When a diagnostic is enabled by a pragma, that diagnostic becomes > enabled permanently. A subsequent `#pragma GCC diagnostic pop' will not > disable it; rather, the option remains enabled but is classified as > "ignored" in the regions of the source where the pragma that enabled it > is not in effect. > > -When a pragma changes a diagnostic classification, or changes the state > from disabled to enabled, the original situation (from the command > line, prior to any pragmas) needs to be remembered for use in source > regions outside the scope of the pragmas. > > -option-classifier.cc does not clearly make a separate note of the two > different properties described above; it stores only the previous > classification, assuming that if a diagnostic has been classified > previously, then it must be enabled. If a diagnostic has not been > classified, then it stores the classification as either "ignored", if > the diagnostic is disabled, otherwise as "any", meaning the diagnostic > should be emitted as a warning or error as later requested and not > suppressed. > > The issue with the last point is that after a sequence such as: > > -Werror -Wno-error=xyz -Wno-xyz > > then "xyz" is classified as a warning, but it is also disabled. If it is > later enabled by `#pragma GCC diagnostic error' or `#pragma GCC diagnostic > warning', then the incorrect original classification will be remembered, and > a subsequent `#pragma GCC diagnostic pop' will leave the diagnostic enabled > as a warning when it should be enabled and ignored. > > It could be more clear to explicitly remember the two different properties > within the option classifier, rather than collapsing enabled/disabled and > classification to a single variable. However, this would require more > extensive changes (including for streaming the pragmas in c++ modules), and > it is not necessary to make the logic correct. Given that a pragma can > enable a diagnostic but never disable a diagnostic, it is sufficient just to > tweak the existing logic a bit; namely, if a diagnostic is not enabled when > `#pragma GCC diagnostic' is seen, then the old classification should be > stored as "ignored" so that the correct state will be set when the pragma is > not in effect. > > gcc/ChangeLog: > > PR diagnostics/124240 > * diagnostics/option-classifier.cc > (option_classifier::classify_diagnostic): Always record a disabled > diagnostic option as being ignored. > > gcc/testsuite/ChangeLog: > > PR diagnostics/124240 > * c-c++-common/pr124240-1.c: New test. > * c-c++-common/pr124240-2.c: New test. > * c-c++-common/pr124240-3.c: New test. > --- > gcc/diagnostics/option-classifier.cc | 17 +++++++++++++---- > gcc/testsuite/c-c++-common/pr124240-1.c | 7 +++++++ > gcc/testsuite/c-c++-common/pr124240-2.c | 7 +++++++ > gcc/testsuite/c-c++-common/pr124240-3.c | 7 +++++++ > 4 files changed, 34 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/pr124240-1.c > create mode 100644 gcc/testsuite/c-c++-common/pr124240-2.c > create mode 100644 gcc/testsuite/c-c++-common/pr124240-3.c > > diff --git a/gcc/diagnostics/option-classifier.cc > b/gcc/diagnostics/option-classifier.cc > index 60dd1dda49a..b9069a60dde 100644 > --- a/gcc/diagnostics/option-classifier.cc > +++ b/gcc/diagnostics/option-classifier.cc > @@ -135,10 +135,19 @@ option_classifier::classify_diagnostic (const context > *dc, > the pragmas were. */ > if (where != UNKNOWN_LOCATION) > { > - /* Record the command-line status, so we can reset it back on > kind::pop. */ > - if (base_kind == kind::unspecified) > - base_kind = (!dc->option_enabled_p (opt_id) > - ? kind::ignored : kind::any); > + /* Record the command-line status, so we can reset it back on > kind::pop. > + If an option was not initially enabled, it may become enabled after a > + pragma, at which point it will always remain enabled even after a > + subsequent pragma pop, so we need to classify the baseline state as > + being ignored, even if it was previously set (e.g. by -Werror=) to > + something else. It is safe to always mark a non-enabled option as > + ignored here, given that a pragma can never cause an enabled > diagnostic > + to become disabled--it would rather just become enabled-but-ignored > + after #pragma GCC diagnostic ignored. */ > + if (!dc->option_enabled_p (opt_id)) > + base_kind = kind::ignored; > + else if (base_kind == kind::unspecified) > + base_kind = kind::any; > > classification_change_t v > = { where, opt_id.m_idx, new_kind }; > diff --git a/gcc/testsuite/c-c++-common/pr124240-1.c > b/gcc/testsuite/c-c++-common/pr124240-1.c > new file mode 100644 > index 00000000000..dfd6eab3809 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/pr124240-1.c > @@ -0,0 +1,7 @@ > +/* PR diagnostics/124240 */ > +/* { dg-do compile } */ > +/* { dg-additional-options "-Wall -Werror -Wno-unused-variable" } */ > +#pragma GCC diagnostic push > +#pragma GCC diagnostic error "-Wunused-variable" > +#pragma GCC diagnostic pop > +static int i = 1; /* { dg-bogus "-Wunused" } */ > diff --git a/gcc/testsuite/c-c++-common/pr124240-2.c > b/gcc/testsuite/c-c++-common/pr124240-2.c > new file mode 100644 > index 00000000000..8cf03e38606 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/pr124240-2.c > @@ -0,0 +1,7 @@ > +/* PR diagnostics/124240 */ > +/* { dg-do compile } */ > +/* { dg-additional-options "-Wall -Werror -Wno-error=unused-variable > -Wno-unused-variable" } */ > +#pragma GCC diagnostic push > +#pragma GCC diagnostic error "-Wunused-variable" > +#pragma GCC diagnostic pop > +static int i = 1; /* { dg-bogus "-Wunused" } */ > diff --git a/gcc/testsuite/c-c++-common/pr124240-3.c > b/gcc/testsuite/c-c++-common/pr124240-3.c > new file mode 100644 > index 00000000000..eb73b88fc1e > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/pr124240-3.c > @@ -0,0 +1,7 @@ > +/* PR diagnostics/124240 */ > +/* { dg-do compile } */ > +/* { dg-additional-options "-Wall -Werror -Wno-unused-variable > -Wno-error=unused-variable" } */ > +#pragma GCC diagnostic push > +#pragma GCC diagnostic error "-Wunused-variable" > +#pragma GCC diagnostic pop > +static int i = 1; /* { dg-bogus "-Wunused" } */
