Ping: [PATCH] diagnostics: Fix incorrect state after #pragma GCC diagnostic pop

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" } */