Re: ping: [PATCH] diagnostics: Fix behavior of permerror options after diagnostic pop [PR111918]
On Tue, 2024-03-19 at 09:03 -0400, Lewis Hyatt wrote: > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html Sorry about the delay. The patch looks good for trunk, assuming it's passed the usual bootstrap and regression testing. Thanks Dave > > Thanks! > > On Fri, Feb 16, 2024 at 7:02 PM Lewis Hyatt wrote: > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > > > On Thu, Jan 25, 2024 at 4:57 PM Lewis Hyatt > > wrote: > > > > > > May I please ask again about this one? It's just a couple lines, > > > and I > > > think it fixes an important gap in the logic for #pragma GCC > > > diagnostic. The PR was not reported by me so I think at least one > > > other person does care about it :). Thanks! > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > > > > > -Lewis > > > > > > On Mon, Jan 8, 2024 at 6:53 PM Lewis Hyatt > > > wrote: > > > > > > > > Can I please ping this one again? It's 3 lines or so to fix the > > > > PR. Thanks! > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > > > > > > > On Tue, Dec 19, 2023 at 6:20 PM Lewis Hyatt > > > > wrote: > > > > > > > > > > Hello- > > > > > > > > > > May I please ping this one? Thanks... > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > > > > > > > > > -Lewis > > > > > > > > > > On Wed, Nov 29, 2023 at 7:05 PM Lewis Hyatt > > > > > wrote: > > > > > > > > > > > > On Thu, Nov 09, 2023 at 04:16:10PM -0500, Lewis Hyatt > > > > > > wrote: > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111918 > > > > > > > > > > > > > > This patch fixes the behavior of `#pragma GCC diagnostic > > > > > > > pop' for permissive > > > > > > > error diagnostics such as -Wnarrowing (in C++11). Those > > > > > > > currently do not > > > > > > > return to the correct state after the last pop; they > > > > > > > become effectively > > > > > > > simple warnings instead. Bootstrap + regtest all > > > > > > > languages on x86-64, does > > > > > > > it look OK please? Thanks! > > > > > > > > > > > > Hello- > > > > > > > > > > > > May I please ping this bug fix? > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635871.html > > > > > > > > > > > > Please note, it requires a trivial rebase on top of recent > > > > > > changes to > > > > > > the class diagnostic_context public interface. I attached > > > > > > the rebased patch > > > > > > here as well. Thanks! > > > > > > > > > > > > -Lewis >
ping: [PATCH] diagnostics: Fix behavior of permerror options after diagnostic pop [PR111918]
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html Thanks! On Fri, Feb 16, 2024 at 7:02 PM Lewis Hyatt wrote: > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > On Thu, Jan 25, 2024 at 4:57 PM Lewis Hyatt wrote: > > > > May I please ask again about this one? It's just a couple lines, and I > > think it fixes an important gap in the logic for #pragma GCC > > diagnostic. The PR was not reported by me so I think at least one > > other person does care about it :). Thanks! > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > > > -Lewis > > > > On Mon, Jan 8, 2024 at 6:53 PM Lewis Hyatt wrote: > > > > > > Can I please ping this one again? It's 3 lines or so to fix the PR. > > > Thanks! > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > > > > > On Tue, Dec 19, 2023 at 6:20 PM Lewis Hyatt wrote: > > > > > > > > Hello- > > > > > > > > May I please ping this one? Thanks... > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > > > > > > > -Lewis > > > > > > > > On Wed, Nov 29, 2023 at 7:05 PM Lewis Hyatt wrote: > > > > > > > > > > On Thu, Nov 09, 2023 at 04:16:10PM -0500, Lewis Hyatt wrote: > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111918 > > > > > > > > > > > > This patch fixes the behavior of `#pragma GCC diagnostic pop' for > > > > > > permissive > > > > > > error diagnostics such as -Wnarrowing (in C++11). Those currently > > > > > > do not > > > > > > return to the correct state after the last pop; they become > > > > > > effectively > > > > > > simple warnings instead. Bootstrap + regtest all languages on > > > > > > x86-64, does > > > > > > it look OK please? Thanks! > > > > > > > > > > Hello- > > > > > > > > > > May I please ping this bug fix? > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635871.html > > > > > > > > > > Please note, it requires a trivial rebase on top of recent changes to > > > > > the class diagnostic_context public interface. I attached the rebased > > > > > patch > > > > > here as well. Thanks! > > > > > > > > > > -Lewis
ping: [PATCH] diagnostics: Fix behavior of permerror options after diagnostic pop [PR111918]
CCing some global reviewers as well, in case anyone has a minute to take a look please? Thanks! https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html On Thu, Jan 25, 2024 at 4:57 PM Lewis Hyatt wrote: > > May I please ask again about this one? It's just a couple lines, and I > think it fixes an important gap in the logic for #pragma GCC > diagnostic. The PR was not reported by me so I think at least one > other person does care about it :). Thanks! > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > -Lewis > > On Mon, Jan 8, 2024 at 6:53 PM Lewis Hyatt wrote: > > > > Can I please ping this one again? It's 3 lines or so to fix the PR. Thanks! > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > > > On Tue, Dec 19, 2023 at 6:20 PM Lewis Hyatt wrote: > > > > > > Hello- > > > > > > May I please ping this one? Thanks... > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > > > > > -Lewis > > > > > > On Wed, Nov 29, 2023 at 7:05 PM Lewis Hyatt wrote: > > > > > > > > On Thu, Nov 09, 2023 at 04:16:10PM -0500, Lewis Hyatt wrote: > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111918 > > > > > > > > > > This patch fixes the behavior of `#pragma GCC diagnostic pop' for > > > > > permissive > > > > > error diagnostics such as -Wnarrowing (in C++11). Those currently do > > > > > not > > > > > return to the correct state after the last pop; they become > > > > > effectively > > > > > simple warnings instead. Bootstrap + regtest all languages on x86-64, > > > > > does > > > > > it look OK please? Thanks! > > > > > > > > Hello- > > > > > > > > May I please ping this bug fix? > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635871.html > > > > > > > > Please note, it requires a trivial rebase on top of recent changes to > > > > the class diagnostic_context public interface. I attached the rebased > > > > patch > > > > here as well. Thanks! > > > > > > > > -Lewis
ping: [PATCH] diagnostics: Fix behavior of permerror options after diagnostic pop [PR111918]
May I please ask again about this one? It's just a couple lines, and I think it fixes an important gap in the logic for #pragma GCC diagnostic. The PR was not reported by me so I think at least one other person does care about it :). Thanks! https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html -Lewis On Mon, Jan 8, 2024 at 6:53 PM Lewis Hyatt wrote: > > Can I please ping this one again? It's 3 lines or so to fix the PR. Thanks! > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > On Tue, Dec 19, 2023 at 6:20 PM Lewis Hyatt wrote: > > > > Hello- > > > > May I please ping this one? Thanks... > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > > > -Lewis > > > > On Wed, Nov 29, 2023 at 7:05 PM Lewis Hyatt wrote: > > > > > > On Thu, Nov 09, 2023 at 04:16:10PM -0500, Lewis Hyatt wrote: > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111918 > > > > > > > > This patch fixes the behavior of `#pragma GCC diagnostic pop' for > > > > permissive > > > > error diagnostics such as -Wnarrowing (in C++11). Those currently do not > > > > return to the correct state after the last pop; they become effectively > > > > simple warnings instead. Bootstrap + regtest all languages on x86-64, > > > > does > > > > it look OK please? Thanks! > > > > > > Hello- > > > > > > May I please ping this bug fix? > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635871.html > > > > > > Please note, it requires a trivial rebase on top of recent changes to > > > the class diagnostic_context public interface. I attached the rebased > > > patch > > > here as well. Thanks! > > > > > > -Lewis
ping^3: [PATCH] diagnostics: Fix behavior of permerror options after diagnostic pop [PR111918]
Can I please ping this one again? It's 3 lines or so to fix the PR. Thanks! https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html On Tue, Dec 19, 2023 at 6:20 PM Lewis Hyatt wrote: > > Hello- > > May I please ping this one? Thanks... > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html > > -Lewis > > On Wed, Nov 29, 2023 at 7:05 PM Lewis Hyatt wrote: > > > > On Thu, Nov 09, 2023 at 04:16:10PM -0500, Lewis Hyatt wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111918 > > > > > > This patch fixes the behavior of `#pragma GCC diagnostic pop' for > > > permissive > > > error diagnostics such as -Wnarrowing (in C++11). Those currently do not > > > return to the correct state after the last pop; they become effectively > > > simple warnings instead. Bootstrap + regtest all languages on x86-64, does > > > it look OK please? Thanks! > > > > Hello- > > > > May I please ping this bug fix? > > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635871.html > > > > Please note, it requires a trivial rebase on top of recent changes to > > the class diagnostic_context public interface. I attached the rebased patch > > here as well. Thanks! > > > > -Lewis
ping^2: [PATCH] diagnostics: Fix behavior of permerror options after diagnostic pop [PR111918]
Hello- May I please ping this one? Thanks... https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638692.html -Lewis On Wed, Nov 29, 2023 at 7:05 PM Lewis Hyatt wrote: > > On Thu, Nov 09, 2023 at 04:16:10PM -0500, Lewis Hyatt wrote: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111918 > > > > This patch fixes the behavior of `#pragma GCC diagnostic pop' for permissive > > error diagnostics such as -Wnarrowing (in C++11). Those currently do not > > return to the correct state after the last pop; they become effectively > > simple warnings instead. Bootstrap + regtest all languages on x86-64, does > > it look OK please? Thanks! > > Hello- > > May I please ping this bug fix? > https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635871.html > > Please note, it requires a trivial rebase on top of recent changes to > the class diagnostic_context public interface. I attached the rebased patch > here as well. Thanks! > > -Lewis
ping: [PATCH] diagnostics: Fix behavior of permerror options after diagnostic pop [PR111918]
On Thu, Nov 09, 2023 at 04:16:10PM -0500, Lewis Hyatt wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111918
>
> This patch fixes the behavior of `#pragma GCC diagnostic pop' for permissive
> error diagnostics such as -Wnarrowing (in C++11). Those currently do not
> return to the correct state after the last pop; they become effectively
> simple warnings instead. Bootstrap + regtest all languages on x86-64, does
> it look OK please? Thanks!
Hello-
May I please ping this bug fix?
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635871.html
Please note, it requires a trivial rebase on top of recent changes to
the class diagnostic_context public interface. I attached the rebased patch
here as well. Thanks!
-Lewis
When a diagnostic pragma changes the classification of a given diagnostic,
the global options flags (such as warn_narrowing, etc.) may get changed too.
Specifically, if a warning was not enabled initially and was later enabled
by a pragma, then the corresponding global flag will change from false to
true when the pragma is processed. That change is permanent and is not
undone by a subsequent `#pragma GCC diagnostic pop'; the warning flag needs
to remain enabled since a diagnostic could be generated later on for a
source location prior to the pop.
So in order to support popping to the initial classification, given that the
global options flags no longer reflect that state, the diagnostic_context
object itself remembers the way things were before it changed anything. The
current implementation works fine for diagnostics that are always errors or
always warnings, but it doesn't do the right thing for diagnostics that
could be either, such as -Wnarrowing. The classification of that diagnostic
(or any permerror diagnostic) depends on the state of -fpermissive; for the
particular case of -Wnarrowing it also matters whether a compile-time or
run-time narrowing is being diagnosed.
The problem is that the current implementation insists on recording whether
an enabled diagnostic should be a DK_WARNING or a DK_ERROR, and then, after
popping to the initial state, it overrides it always to that type only. Fix
that up by adding a new internal diagnostic type DK_ANY. This just indicates
that the diagnostic is enabled without mandating exactly what type of
diagnostic it should be. Then the diagnostic can be emitted with whatever
type the frontend asks for.
Incidentally, while making this change, I noticed that classify_diagnostic()
spends some time computing a return value (the old classification kind) that
is not used anywhere. The computed value seems to have some problems, mainly
that it does not take into account `#pragma GCC diagnostic pop' at all, and
so the returned value doesn't seem like it could make sense in many
contexts. Given it would also not be desirable to leak the new internal-only
DK_ANY type to outside callers, I think it would make sense in a subsequent
cleanup patch to remove the return value altogether.
gcc/ChangeLog:
PR c++/111918
* diagnostic-core.h (enum diagnostic_t): Add DK_ANY special flag.
* diagnostic.cc (diagnostic_option_classifier::classify_diagnostic):
Make use of DK_ANY to indicate a diagnostic was initially enabled.
(diagnostic_context::diagnostic_enabled): Do not change the type of
a diagnostic if the saved classification is type DK_ANY.
gcc/testsuite/ChangeLog:
PR c++/111918
* g++.dg/cpp0x/Wnarrowing21a.C: New test.
* g++.dg/cpp0x/Wnarrowing21b.C: New test.
* g++.dg/cpp0x/Wnarrowing21c.C: New test.
* g++.dg/cpp0x/Wnarrowing21d.C: New test.
diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h
index 04eba3d140e..4926c48da96 100644
--- a/gcc/diagnostic-core.h
+++ b/gcc/diagnostic-core.h
@@ -33,7 +33,10 @@ typedef enum
DK_LAST_DIAGNOSTIC_KIND,
/* This is used for tagging pragma pops in the diagnostic
classification history chain. */
- DK_POP
+ DK_POP,
+ /* This is used internally to note that a diagnostic is enabled
+ without mandating any specific type. */
+ DK_ANY,
} diagnostic_t;
/* RAII-style class for grouping related diagnostics. */
diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index 4f66fa6acaa..fd40018a734 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -1136,8 +1136,7 @@ classify_diagnostic (const diagnostic_context *context,
if (old_kind == DK_UNSPECIFIED)
{
old_kind = !context->option_enabled_p (option_index)
- ? DK_IGNORED : (context->warning_as_error_requested_p ()
- ? DK_ERROR : DK_WARNING);
+ ? DK_IGNORED : DK_ANY;
m_classify_diagnostic[option_index] = old_kind;
}
@@ -1472,7 +1471,15 @@ diagnostic_context::diagnostic_enabled (diagnostic_info
*diagnostic)
option. */
if (diag_class == DK_UNSPECIFIED
&& !option_unspecified_p (diagnostic->option_index))
-diagnostic->kind =
[PATCH] diagnostics: Fix behavior of permerror options after diagnostic pop [PR111918]
Hello- https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111918 This patch fixes the behavior of `#pragma GCC diagnostic pop' for permissive error diagnostics such as -Wnarrowing (in C++11). Those currently do not return to the correct state after the last pop; they become effectively simple warnings instead. Bootstrap + regtest all languages on x86-64, does it look OK please? Thanks! -Lewis -- >8 -- When a diagnostic pragma changes the classification of a given diagnostic, the global options flags (such as warn_narrowing, etc.) may get changed too. Specifically, if a warning was not enabled initially and was later enabled by a pragma, then the corresponding global flag will change from false to true when the pragma is processed. That change is permanent and is not undone by a subsequent `#pragma GCC diagnostic pop'; the warning flag needs to remain enabled since a diagnostic could be generated later on for a source location prior to the pop. So in order to support popping to the initial classification, given that the global options flags no longer reflect that state, the diagnostic_context object itself remembers the way things were before it changed anything. The current implementation works fine for diagnostics that are always errors or always warnings, but it doesn't do the right thing for diagnostics that could be either, such as -Wnarrowing. The classification of that diagnostic (or any permerror diagnostic) depends on the state of -fpermissive; for the particular case of -Wnarrowing it also matters whether a compile-time or run-time narrowing is being diagnosed. The problem is that the current implementation insists on recording whether an enabled diagnostic should be a DK_WARNING or a DK_ERROR, and then, after popping to the initial state, it overrides it always to that type only. Fix that up by adding a new internal diagnostic type DK_ANY. This just indicates that the diagnostic is enabled without mandating exactly what type of diagnostic it should be. Then the diagnostic can be emitted with whatever type the frontend asks for. Incidentally, while making this change, I noticed that classify_diagnostic() spends some time computing a return value (the old classification kind) that is not used anywhere. The computed value seems to have some problems, mainly that it does not take into account `#pragma GCC diagnostic pop' at all, and so the returned value doesn't seem like it could make sense in many contexts. Given it would also not be desirable to leak the new internal-only DK_ANY type to outside callers, I think it would make sense in a subsequent cleanup patch to remove the return value altogether. gcc/ChangeLog: PR c++/111918 * diagnostic-core.h (enum diagnostic_t): Add DK_ANY special flag. * diagnostic.cc (diagnostic_option_classifier::classify_diagnostic): Make use of DK_ANY to indicate a diagnostic was initially enabled. (diagnostic_context::diagnostic_enabled): Do not change the type of a diagnostic if the saved classification is type DK_ANY. gcc/testsuite/ChangeLog: PR c++/111918 * g++.dg/cpp0x/Wnarrowing21a.C: New test. * g++.dg/cpp0x/Wnarrowing21b.C: New test. * g++.dg/cpp0x/Wnarrowing21c.C: New test. * g++.dg/cpp0x/Wnarrowing21d.C: New test. --- gcc/diagnostic-core.h | 5 - gcc/diagnostic.cc | 13 ++--- gcc/testsuite/g++.dg/cpp0x/Wnarrowing21a.C | 14 ++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing21b.C | 9 + gcc/testsuite/g++.dg/cpp0x/Wnarrowing21c.C | 9 + gcc/testsuite/g++.dg/cpp0x/Wnarrowing21d.C | 9 + 6 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wnarrowing21a.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wnarrowing21b.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wnarrowing21c.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wnarrowing21d.C diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h index 04eba3d140e..4926c48da96 100644 --- a/gcc/diagnostic-core.h +++ b/gcc/diagnostic-core.h @@ -33,7 +33,10 @@ typedef enum DK_LAST_DIAGNOSTIC_KIND, /* This is used for tagging pragma pops in the diagnostic classification history chain. */ - DK_POP + DK_POP, + /* This is used internally to note that a diagnostic is enabled + without mandating any specific type. */ + DK_ANY, } diagnostic_t; /* RAII-style class for grouping related diagnostics. */ diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc index addd6606eaa..99921a10b7b 100644 --- a/gcc/diagnostic.cc +++ b/gcc/diagnostic.cc @@ -1126,8 +1126,7 @@ classify_diagnostic (const diagnostic_context *context, old_kind = !context->m_option_enabled (option_index, context->m_lang_mask, context->m_option_state) - ? DK_IGNORED :
