On 7/31/22 11:05 AM, Lewis Hyatt wrote:
On Sat, Jul 30, 2022 at 7:06 PM Tom Honermann via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
On 7/27/22 7:09 PM, Joseph Myers wrote:
On Sun, 24 Jul 2022, Tom Honermann via Gcc-patches wrote:
Gcc's '#pragma GCC diagnostic' directives are processed in "early mode"
(see handle_pragma_diagnostic_early) for the C++ frontend and, as such,
require that the target diagnostic option be enabled for the preprocessor
(see c_option_is_from_cpp_diagnostics). This change modifies the
-Wc++20-compat option definition to register it as a preprocessor option
so that its associated diagnostics can be suppressed. The changes also
There are lots of C++ warning options, all of which should support pragma
suppression regardless of whether they are relevant to the preprocessor or
not. Do they all need this kind of handling, or is it only -Wc++20-compat
that has some kind of problem?
I had only checked -Wc++20-compat when working on the patch.
I did some spot checking now and confirmed that suppression works as
expected for C++ for at least the following warnings:
-Wuninitialized
-Warray-compare
-Wbool-compare
-Wtautological-compare
-Wterminate
I don't know the diagnostic framework well. As best I can tell, this
issue is specific to the -Wc++20-compat option and when the particular
diagnostic is issued (e.g., during lexing as opposed to during parsing).
The following call chains appear to be relevant.
cp_lexer_new_main -> cp_lexer_handle_early_pragma ->
c_invoke_early_pragma_handler
cp_parser_* -> cp_parser_pragma -> c_invoke_pragma_handler
(where * might be "declaration", "toplevel_declaration",
"class_head", "objc_interstitial_code", ...)
The -Wc++20-compat enabled warning regarding new keywords in C++20 is
issued from cp_lexer_get_preprocessor_token.
Tom.
I have been working on improving the handling of "#pragma GCC
diagnostic" lately. The behavior for C++ changed since r13-1544
(https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e46f4d7430c5210465791603735ab219ef263c51).
I have some more comments about the patch's approach on the PR
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431#c44).
"#pragma GCC diagnostic" formerly did not work in C++ at all, for
diagnostics generated by libcpp, because C++ obtains all the tokens
from libcpp first (including deferred pragmas), and then processes
them afterward, too late to take effect for diagnostics that libcpp
has already emitted. r13-1544 fixed this up by adding an early pragma
handler, which runs as soon as a deferred pragma token is seen and
handles diagnostic pragmas if they pertain to libcpp-controlled
diagnostics. Non-libcpp diagnostics still need to be handled later,
during parsing, or else they get processed too early and it leads to
other problems. Basically, now each diagnostic pragma is handled as
close in time as possible to the time the associated diagnostics might
be generated.
The early pragma handler determines that an option comes from libcpp,
and so should be subject to early processing, if it was marked as such
in the options definition file. Tom's patch points out that
-Wc++20-compat needs to be handled early, and so marking it as a
libcpp diagnostic in c-family/c.opt arranges for that to work as
intended. Now one potential objection here is that -Wc++20-compat
warnings are not technically generated by libcpp. They are generated
by the C++ frontend immediately after lexing an identifier token from
libcpp (cp_lexer_get_preprocessor_token()). But the distinction
between these two steps is rather blurry and it seems logical to me,
to denote this as a libcpp-related option. Also, the same is already
done for -Wc++11-compat. Otherwise, we would need to add some new
option property to indicate which ones need to be handled for pragmas
at lexing time rather than parsing time.
At the moment I don't see any other diagnostics issued from
cp_lexer_get_preprocessor_token() that would need similar adjustments.
Assuming the approach is OK, it might be nice to add a comment to that
function, indicating that any diagnostics emitted there should be
annotated as libcpp options in the .opt file?
Thank you for those details; I wasn't aware of that history.
If I'm interpreting your response correctly, it sounds like you agree
with the direction of the patch.
If you like, I can add a comment as you suggested and re-post the patch.
Perhaps:
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 4f67441eeb1..c3584446827 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -924,7 +924,10 @@cp_lexer_saving_tokens (const cp_lexer* lexer)
/* Store the next token from the preprocessor in *TOKEN. Return true
if we reach EOF. If LEXER is NULL, assume we are handling an
initial #pragma pch_preprocess, and thus want the lexer to return
- processed strings. */
+ processed strings.
+
+ Diagnostics issued from this function must have their controlling
option (if
+ any) in c.opt annotated as a libcpp option via the CppReason
property. */
static void
cp_lexer_get_preprocessor_token (unsigned flags, cp_token *token)
Tom.
-Lewis
