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