Ping: [PATCH v2] diagnostics: Honor #pragma GCC diagnostic in the preprocessor [PR53431]

Message ID 20220524212818.GA3654@ldh-imac.local
State Committed
Commit ff64a8c951995cb1214b4d2864a4bd553b60f249
Headers
Series Ping: [PATCH v2] diagnostics: Honor #pragma GCC diagnostic in the preprocessor [PR53431] |

Commit Message

Lewis Hyatt May 24, 2022, 9:28 p.m. UTC
  Hello-

Now that we're back in stage 1, I thought it might be a better time to
ask for feedback on this pair of patches that tries to resolve PR53431
please?

https://gcc.gnu.org/pipermail/gcc-patches/2021-December/587357.html
https://gcc.gnu.org/pipermail/gcc-patches/2021-December/587358.html

Part 1/2 is a trivial cleanup in the C++ parser that simplifies
adding the support for early pragma handling.

Part 2/2 adds the concept of early pragma handling and makes the C++
and preprocessor frontends use it.

The patches required some minor rebasing, so I have attached updated
versions here.

bootstrap + regtest all languages still looks good:

FAIL 103 103
PASS 541178 541213
UNSUPPORTED 15177 15177
UNTESTED 136 136
XFAIL 4140 4140
XPASS 17 17

Thanks! If this approach doesn't seem like the right one, I am happy
to try another way.

-Lewis


On Fri, Dec 24, 2021 at 04:23:08PM -0500, Lewis Hyatt wrote:
> Hello-
> 
> I would like please to follow up on this patch submitted for PR53431 here:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586191.html
> 
> However, it was suggested on the PR that part of it could be split into a
> separate simpler patch. I have now done that, and also made a few tweaks to
> the first version at the same time, so may I please request that you review
> this version 2 instead? This email contains the first smaller cleanup patch,
> and the next email contains the main part of it. Thanks very much.
> 
> bootstrap and regtest were performed on x86-64 Linux, all tests look the same
> before + after, plus the new passing testcases.
> 
> FAIL 112 112
> PASS 528007 528042
> UNSUPPORTED 14888 14888
> UNTESTED 132 132
> XFAIL 3238 3238
> XPASS 17 17
> 
> -Lewis

> From: Lewis Hyatt <lhyatt@gmail.com>
> Date: Thu, 23 Dec 2021 17:03:04 -0500
> Subject: [PATCH] c++: Minor cleanup in parser.c
> 
> The code to determine whether a given token starts a module directive is
> currently repeated in 4 places in parser.c. I am about to submit a patch
> that needs to add it in a 5th place, so since the code is not completely
> trivial (needing to check for 3 different token types), it seems worthwhile
> to factor this logic into its own function.
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.c (cp_token_is_module_directive): New function
> 	refactoring common code.
> 	(cp_parser_skip_to_closing_parenthesis_1): Use the new function.
> 	(cp_parser_skip_to_end_of_statement): Likewise.
> 	(cp_parser_skip_to_end_of_block_or_statement): Likewise.
> 	(cp_parser_declaration): Likewise.
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 33fb40a5b59..9b7446655be 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -629,6 +629,16 @@ cp_lexer_alloc (void)
>    return lexer;
>  }
>  
> +/* Return TRUE if token is the start of a module declaration that will be
> +   terminated by a CPP_PRAGMA_EOL token.  */
> +static inline bool
> +cp_token_is_module_directive (cp_token *token)
> +{
> +  return token->keyword == RID__EXPORT
> +    || token->keyword == RID__MODULE
> +    || token->keyword == RID__IMPORT;
> +}
> +
>  /* Create a new main C++ lexer, the lexer that gets tokens from the
>     preprocessor.  */
>  
> @@ -3805,9 +3815,7 @@ cp_parser_skip_to_closing_parenthesis_1 (cp_parser *parser,
>  	  break;
>  
>  	case CPP_KEYWORD:
> -	  if (token->keyword != RID__EXPORT
> -	      && token->keyword != RID__MODULE
> -	      && token->keyword != RID__IMPORT)
> +	  if (!cp_token_is_module_directive (token))
>  	    break;
>  	  /* FALLTHROUGH  */
>  
> @@ -3908,9 +3916,7 @@ cp_parser_skip_to_end_of_statement (cp_parser* parser)
>  	  break;
>  
>  	case CPP_KEYWORD:
> -	  if (token->keyword != RID__EXPORT
> -	      && token->keyword != RID__MODULE
> -	      && token->keyword != RID__IMPORT)
> +	  if (!cp_token_is_module_directive (token))
>  	    break;
>  	  /* FALLTHROUGH  */
>  
> @@ -3997,9 +4003,7 @@ cp_parser_skip_to_end_of_block_or_statement (cp_parser* parser)
>  	  break;
>  
>  	case CPP_KEYWORD:
> -	  if (token->keyword != RID__EXPORT
> -	      && token->keyword != RID__MODULE
> -	      && token->keyword != RID__IMPORT)
> +	  if (!cp_token_is_module_directive (token))
>  	    break;
>  	  /* FALLTHROUGH  */
>  
> @@ -14860,9 +14864,7 @@ cp_parser_declaration (cp_parser* parser, tree prefix_attrs)
>        else
>  	cp_parser_module_export (parser);
>      }
> -  else if (token1->keyword == RID__EXPORT
> -	   || token1->keyword == RID__IMPORT
> -	   || token1->keyword == RID__MODULE)
> +  else if (cp_token_is_module_directive (token1))
>      {
>        bool exporting = token1->keyword == RID__EXPORT;
>        cp_token *next = exporting ? token2 : token1;
c++: Minor cleanup in parser.cc

The code to determine whether a given token starts a module directive is
currently repeated in 4 places in parser.cc. I am about to submit a patch
that needs to add it in a 5th place, so since the code is not completely
trivial (needing to check for 3 different token types), it seems worthwhile
to factor this logic into its own function.

gcc/cp/ChangeLog:

	* parser.cc (cp_token_is_module_directive): New function
	refactoring common code.
	(cp_parser_skip_to_closing_parenthesis_1): Use the new function.
	(cp_parser_skip_to_end_of_statement): Likewise.
	(cp_parser_skip_to_end_of_block_or_statement): Likewise.
	(cp_parser_declaration): Likewise.
diagnostics: Honor #pragma GCC diagnostic in the preprocessor [PR53431]

As discussed on PR c++/53431, currently, "#pragma GCC diagnostic" does
not always take effect for diagnostics generated by libcpp. The reason
is that libcpp itself does not interpret this pragma and only sends it on
to the frontend, hence the pragma is only honored if the frontend
arranges for it. The C frontend does process the pragma immediately
(more or less) after seeing the token, so things work fine there. The PR
points out that it doesn't work for C++, because the C++ frontend
doesn't handle anything until it has read all the tokens from
libcpp. The underlying problem is not C++-specific, though, and for
instance, gcc -E has the same issue.

This commit fixes the PR by adding the concept of an early pragma handler that
can be registered by frontends, which gives them a chance to process
diagnostic pragmas from libcpp before it is too late for them to take
effect. The C++ and preprocess-only frontends are modified to use early
pragmas and correct the behavior.

gcc/c-family/ChangeLog:

	PR c++/53431
	* c-common.cc (c_option_is_from_cpp_diagnostics): New function.
	* c-common.h (c_option_is_from_cpp_diagnostics): Declare.
	(c_pp_output_diagnostic_pragma): Declare.
	* c-ppoutput.cc (init_pp_output): Refactor logic about skipping pragmas
	to...
	(should_output_pragmas): ...here.  New function.
	(class token_streamer): Added new member "skip_this_pragma".
	(token_streamer::stream): Support printing early pragmas.
	(do_line_change): Likewise.
	(c_pp_output_diagnostic_pragma): New function.
	* c-pragma.cc (struct pragma_diagnostic_data): New helper class.
	(pragma_diagnostic_lex_normal): New function. Moved logic for
	interpreting GCC diagnostic pragmas here.
	(pragma_diagnostic_lex_pp): New function for parsing diagnostic pragmas
	directly from libcpp.
	(handle_pragma_diagnostic): Refactor into helper function...
	(handle_pragma_diagnostic_impl): ...here.  New function.
	(handle_pragma_diagnostic_early): New function.
	(handle_pragma_diagnostic_early_pp): New function.
	(struct pragma_ns_name): Renamed to...
	(struct pragma_pp_data): ...this.  Add new "early_handler" member.
	(c_register_pragma_1): Support early pragmas in the preprocessor.
	(c_register_pragma_with_early_handler): New function.
	(c_register_pragma): Support the new early handlers in struct
	internal_pragma_handler.
	(c_register_pragma_with_data): Likewise.
	(c_register_pragma_with_expansion): Likewise.
	(c_register_pragma_with_expansion_and_data): Likewise.
	(c_invoke_early_pragma_handler): New function.
	(c_pp_invoke_early_pragma_handler): New function.
	(init_pragma): Add early pragma support for diagnostic pragmas.
	* c-pragma.h (struct internal_pragma_handler): Add new early handler
	members.
	(c_register_pragma_with_early_handler): Declare.
	(c_invoke_early_pragma_handler): Declare.
	(c_pp_invoke_early_pragma_handler): Declare.

gcc/cp/ChangeLog:

	PR c++/53431
	* parser.cc (cp_parser_pragma_kind): Move earlier in the file.
	(cp_lexer_handle_early_pragma): New function.
	(cp_lexer_new_main): Support parsing and handling early pragmas.
	(c_parse_file): Adapt to changes in cp_lexer_new_main.

gcc/testsuite/ChangeLog:

	PR c++/53431
	* c-c++-common/pragma-diag-11.c: New test.
	* c-c++-common/pragma-diag-12.c: New test.
	* c-c++-common/pragma-diag-13.c: New test.

diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index c9c9e720bc8..1b8e73f7bc5 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -6611,6 +6611,20 @@ c_option_controlling_cpp_diagnostic (enum cpp_warning_reason reason)
   return 0;
 }
 
+/* Return TRUE if the given option index corresponds to a diagnostic
+   issued by libcpp.  Linear search seems fine for now.  */
+bool
+c_option_is_from_cpp_diagnostics (int option_index)
+{
+  for (auto entry = cpp_reason_option_codes; entry->reason != CPP_W_NONE;
+       ++entry)
+    {
+      if (entry->option_code == option_index)
+	return true;
+    }
+  return false;
+}
+
 /* Callback from cpp_diagnostic for PFILE to print diagnostics from the
    preprocessor.  The diagnostic is of type LEVEL, with REASON set
    to the reason code if LEVEL is represents a warning, at location
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 47442c95a53..0de9b4737a6 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -911,6 +911,7 @@ extern tree fold_for_warn (tree);
 extern tree c_common_get_narrower (tree, int *);
 extern bool get_attribute_operand (tree, unsigned HOST_WIDE_INT *);
 extern void c_common_finalize_early_debug (void);
+extern bool c_option_is_from_cpp_diagnostics (int);
 
 /* Used by convert_and_check; in front ends.  */
 extern tree convert_init (tree, tree);
@@ -1191,6 +1192,7 @@ extern void preprocess_file (cpp_reader *);
 extern void pp_file_change (const line_map_ordinary *);
 extern void pp_dir_change (cpp_reader *, const char *);
 extern bool check_missing_format_attribute (tree, tree);
+extern void c_pp_output_diagnostic_pragma (const char *, const char *);
 
 /* In c-omp.cc  */
 typedef wide_int_bitmask omp_clause_mask;
diff --git a/gcc/c-family/c-ppoutput.cc b/gcc/c-family/c-ppoutput.cc
index 9de46a9655f..fc31a033801 100644
--- a/gcc/c-family/c-ppoutput.cc
+++ b/gcc/c-family/c-ppoutput.cc
@@ -110,6 +110,14 @@ preprocess_file (cpp_reader *pfile)
     putc ('\n', print.outf);
 }
 
+/* Don't emit #pragma or #ident directives if we are processing
+   assembly language; the assembler may choke on them.  */
+static bool
+should_output_pragmas ()
+{
+  return cpp_get_options (parse_in)->lang != CLK_ASM;
+}
+
 /* Set up the callbacks as appropriate.  */
 void
 init_pp_output (FILE *out_stream)
@@ -121,7 +129,7 @@ init_pp_output (FILE *out_stream)
       cb->line_change = cb_line_change;
       /* Don't emit #pragma or #ident directives if we are processing
 	 assembly language; the assembler may choke on them.  */
-      if (cpp_get_options (parse_in)->lang != CLK_ASM)
+      if (should_output_pragmas ())
 	{
 	  cb->ident      = cb_ident;
 	  cb->def_pragma = cb_def_pragma;
@@ -173,6 +181,7 @@ class token_streamer
   bool avoid_paste;
   bool do_line_adjustments;
   bool in_pragma;
+  bool skip_this_pragma;
   bool line_marker_emitted;
 
  public:
@@ -181,6 +190,7 @@ class token_streamer
     do_line_adjustments (cpp_get_options (pfile)->lang != CLK_ASM
 			 && !flag_no_line_commands),
     in_pragma (false),
+    skip_this_pragma (false),
     line_marker_emitted (false)
     {
     }
@@ -188,6 +198,7 @@ class token_streamer
   void begin_pragma () 
   {
     in_pragma = true;
+    skip_this_pragma = false;
   }
 
   void stream (cpp_reader *pfile, const cpp_token *tok, location_t);
@@ -235,7 +246,7 @@ token_streamer::stream (cpp_reader *pfile, const cpp_token *token,
 	  print.printed = true;
 	}
     }
-  else if (token->flags & PREV_WHITE)
+  else if (token->flags & PREV_WHITE && token->type != CPP_PRAGMA)
     {
       unsigned src_line = LOCATION_LINE (loc);
 
@@ -252,23 +263,32 @@ token_streamer::stream (cpp_reader *pfile, const cpp_token *token,
   print.prev = token;
   if (token->type == CPP_PRAGMA)
     {
-      const char *space;
-      const char *name;
-
-      line_marker_emitted = maybe_print_line (token->src_loc);
-      fputs ("#pragma ", print.outf);
-      c_pp_lookup_pragma (token->val.pragma, &space, &name);
-      if (space)
-	fprintf (print.outf, "%s %s", space, name);
-      else
-	fprintf (print.outf, "%s", name);
-      print.printed = true;
       in_pragma = true;
+      if (should_output_pragmas ())
+	{
+	  const char *space;
+	  const char *name;
+
+	  line_marker_emitted = maybe_print_line (token->src_loc);
+	  fputs ("#pragma ", print.outf);
+	  c_pp_lookup_pragma (token->val.pragma, &space, &name);
+	  if (space)
+	    fprintf (print.outf, "%s %s", space, name);
+	  else
+	    fprintf (print.outf, "%s", name);
+	  print.printed = true;
+	  skip_this_pragma = false;
+	}
+      else
+	skip_this_pragma = true;
+      if (token->val.pragma >= PRAGMA_FIRST_EXTERNAL)
+	c_pp_invoke_early_pragma_handler (token->val.pragma);
     }
   else if (token->type == CPP_PRAGMA_EOL)
     {
-      maybe_print_line (UNKNOWN_LOCATION);
-      in_pragma = false;
+      if (!skip_this_pragma)
+	maybe_print_line (UNKNOWN_LOCATION);
+      in_pragma = skip_this_pragma = false;
     }
   else
     {
@@ -287,9 +307,12 @@ token_streamer::stream (cpp_reader *pfile, const cpp_token *token,
 	  do_line_change (pfile, token, loc, false);
 	  print.prev_was_system_token = !!in_system_header_at (loc);
 	}
-      cpp_output_token (token, print.outf);
-      line_marker_emitted = false;
-      print.printed = true;
+      if (!(in_pragma && skip_this_pragma))
+	{
+	  cpp_output_token (token, print.outf);
+	  line_marker_emitted = false;
+	  print.printed = true;
+	}
     }
 
   /* CPP_COMMENT tokens and raw-string literal tokens can have
@@ -561,8 +584,12 @@ do_line_change (cpp_reader *pfile, const cpp_token *token,
      one space per column greater than 2, since scan_translation_unit
      will provide a space if PREV_WHITE.  Don't bother trying to
      reconstruct tabs; we can't get it right in general, and nothing
-     ought to care.  Some things do care; the fault lies with them.  */
-  if (!CPP_OPTION (pfile, traditional))
+     ought to care.  Some things do care; the fault lies with them.
+
+     Also do not output the spaces if this is a CPP_PRAGMA token.  In this
+     case, libcpp has provided the location of the first token after #pragma,
+     so we would start at the wrong column.  */
+  if (!CPP_OPTION (pfile, traditional) && token->type != CPP_PRAGMA)
     {
       int spaces = LOCATION_COLUMN (src_loc) - 2;
       print.printed = true;
@@ -782,6 +809,20 @@ cb_def_pragma (cpp_reader *pfile, location_t line)
   print.src_line++;
 }
 
+/* Similar, for the portion of a diagnostic pragma that was parsed
+   internally and so not seen by our token streamer.  */
+void
+c_pp_output_diagnostic_pragma (const char *kind, const char *option)
+{
+  if (!should_output_pragmas ())
+    return;
+  gcc_assert (print.printed);
+  if (kind)
+    fprintf (print.outf, " %s", kind);
+  if (option)
+    fprintf (print.outf, " \"%s\"", option);
+}
+
 /* Dump out the hash table.  */
 static int
 dump_macro (cpp_reader *pfile, cpp_hashnode *node, void *v ATTRIBUTE_UNUSED)
diff --git a/gcc/c-family/c-pragma.cc b/gcc/c-family/c-pragma.cc
index cc05b2580fa..ed969d66a37 100644
--- a/gcc/c-family/c-pragma.cc
+++ b/gcc/c-family/c-pragma.cc
@@ -764,139 +764,329 @@ handle_pragma_visibility (cpp_reader *)
     warning (OPT_Wpragmas, "junk at end of %<#pragma GCC visibility%>");
 }
 
+/* Helper routines for parsing #pragma GCC diagnostic.  */
+class pragma_diagnostic_data
+{
+  pragma_diagnostic_data (const pragma_diagnostic_data &) = delete;
+  pragma_diagnostic_data& operator= (const pragma_diagnostic_data &) = delete;
+
+public:
+  bool valid;
+  location_t loc_kind, loc_option;
+  enum pd_kind_t
+    {
+      PD_KIND_INVALID,
+      PD_KIND_PUSH,
+      PD_KIND_POP,
+      PD_KIND_IGNORED_ATTRIBUTES,
+      PD_KIND_DIAGNOSTIC,
+    } pd_kind;
+  diagnostic_t diagnostic_kind;
+  const char *kind_str;
+  const char *option_str;
+  bool own_option_str;
+
+  pragma_diagnostic_data () { clear (); }
+  void clear ()
+  {
+    valid = false;
+    loc_kind = loc_option = UNKNOWN_LOCATION;
+    pd_kind = PD_KIND_INVALID;
+    diagnostic_kind = DK_UNSPECIFIED;
+    kind_str = option_str = nullptr;
+    own_option_str = false;
+  }
+
+  ~pragma_diagnostic_data ()
+  {
+    if (own_option_str && option_str)
+      XDELETEVEC (const_cast<char *> (option_str));
+  }
+
+  void set_kind (const char *kind_string)
+  {
+    kind_str = kind_string;
+
+    pd_kind = PD_KIND_INVALID;
+    diagnostic_kind = DK_UNSPECIFIED;
+    if (strcmp (kind_str, "push") == 0)
+      pd_kind = PD_KIND_PUSH;
+    else if (strcmp (kind_str, "pop") == 0)
+      pd_kind = PD_KIND_POP;
+    else if (strcmp (kind_str, "ignored_attributes") == 0)
+      pd_kind = PD_KIND_IGNORED_ATTRIBUTES;
+    else if (strcmp (kind_str, "error") == 0)
+      {
+	pd_kind = PD_KIND_DIAGNOSTIC;
+	diagnostic_kind = DK_ERROR;
+      }
+    else if (strcmp (kind_str, "warning") == 0)
+      {
+	pd_kind = PD_KIND_DIAGNOSTIC;
+	diagnostic_kind = DK_WARNING;
+      }
+    else if (strcmp (kind_str, "ignored") == 0)
+      {
+	pd_kind = PD_KIND_DIAGNOSTIC;
+	diagnostic_kind = DK_IGNORED;
+      }
+  }
+
+  bool needs_option () const
+  {
+    return pd_kind == PD_KIND_IGNORED_ATTRIBUTES
+      || pd_kind == PD_KIND_DIAGNOSTIC;
+  }
+
+};
+
+/* When compiling normally, use pragma_lex() to obtain the needed tokens.  This
+   will call into either the C or C++ frontends as appropriate.  */
 static void
-handle_pragma_diagnostic(cpp_reader *)
+pragma_diagnostic_lex_normal (pragma_diagnostic_data *result)
 {
+  result->clear ();
   tree x;
-  location_t loc;
-  enum cpp_ttype token = pragma_lex (&x, &loc);
-  if (token != CPP_NAME)
+  auto ttype = pragma_lex (&x, &result->loc_kind);
+  if (ttype != CPP_NAME)
+    return;
+  result->set_kind (IDENTIFIER_POINTER (x));
+  if (result->pd_kind == pragma_diagnostic_data::PD_KIND_INVALID)
+    return;
+
+  if (result->needs_option ())
     {
-      warning_at (loc, OPT_Wpragmas,
-		  "missing %<error%>, %<warning%>, %<ignored%>, %<push%>, "
-		  "%<pop%>, or %<ignored_attributes%> after "
-		  "%<#pragma GCC diagnostic%>");
-      return;
+      ttype = pragma_lex (&x, &result->loc_option);
+      if (ttype != CPP_STRING)
+	return;
+      result->option_str = TREE_STRING_POINTER (x);
     }
 
-  diagnostic_t kind;
-  const char *kind_string = IDENTIFIER_POINTER (x);
-  if (strcmp (kind_string, "error") == 0)
-    kind = DK_ERROR;
-  else if (strcmp (kind_string, "warning") == 0)
-    kind = DK_WARNING;
-  else if (strcmp (kind_string, "ignored") == 0)
-    kind = DK_IGNORED;
-  else if (strcmp (kind_string, "push") == 0)
+  result->valid = true;
+}
+
+/* When preprocessing only, pragma_lex() is not available, so obtain the tokens
+   directly from libcpp.  */
+static void
+pragma_diagnostic_lex_pp (pragma_diagnostic_data *result)
+{
+  result->clear ();
+
+  auto tok = cpp_get_token_with_location (parse_in, &result->loc_kind);
+  if (!(tok->type == CPP_NAME || tok->type == CPP_KEYWORD))
+    return;
+  const unsigned char *const kind_u = cpp_token_as_text (parse_in, tok);
+  result->set_kind ((const char *)kind_u);
+  if (result->pd_kind == pragma_diagnostic_data::PD_KIND_INVALID)
+    return;
+
+  if (result->needs_option ())
     {
-      diagnostic_push_diagnostics (global_dc, input_location);
-      return;
+      tok = cpp_get_token_with_location (parse_in, &result->loc_option);
+      if (tok->type != CPP_STRING)
+	return;
+      cpp_string str;
+      if (!cpp_interpret_string_notranslate (parse_in, &tok->val.str, 1, &str,
+					     CPP_STRING)
+	  || !str.len)
+	return;
+      result->option_str = (const char *)str.text;
+      result->own_option_str = true;
     }
-  else if (strcmp (kind_string, "pop") == 0)
+
+  result->valid = true;
+}
+
+/* Handle #pragma GCC diagnostic.  Early mode is used by frontends (such as C++)
+   that do not process the deferred pragma while they are consuming tokens; they
+   can use early mode to make sure diagnostics affecting the preprocessor itself
+   are correctly modified by the #pragma.  */
+template<bool early, bool is_pp> static void
+handle_pragma_diagnostic_impl ()
+{
+  static const bool want_diagnostics = (is_pp || !early);
+
+  pragma_diagnostic_data data;
+  if (is_pp)
+    pragma_diagnostic_lex_pp (&data);
+  else
+    pragma_diagnostic_lex_normal (&data);
+
+  /* When in preprocess-only mode, we need to let c-ppoutput.c know to output
+     these tokens, since it will not see them itself.  */
+  if (is_pp)
+    c_pp_output_diagnostic_pragma (data.kind_str, data.option_str);
+
+  if (!data.kind_str)
     {
-      diagnostic_pop_diagnostics (global_dc, input_location);
+      if (want_diagnostics)
+	warning_at (data.loc_kind, OPT_Wpragmas,
+		    "missing %<error%>, %<warning%>, %<ignored%>, %<push%>, "
+		    "%<pop%>, or %<ignored_attributes%> after "
+		    "%<#pragma GCC diagnostic%>");
       return;
     }
-  else if (strcmp (kind_string, "ignored_attributes") == 0)
+
+  switch (data.pd_kind)
     {
-      token = pragma_lex (&x, &loc);
-      if (token != CPP_STRING)
-	{
-	  warning_at (loc, OPT_Wpragmas,
-		      "missing attribute name after %<#pragma GCC diagnostic "
-		      "ignored_attributes%>");
-	  return;
-	}
-      char *args = xstrdup (TREE_STRING_POINTER (x));
-      const size_t l = strlen (args);
-      if (l == 0)
-	{
-	  warning_at (loc, OPT_Wpragmas, "missing argument to %<#pragma GCC "
-		      "diagnostic ignored_attributes%>");
-	  free (args);
+
+    case pragma_diagnostic_data::PD_KIND_PUSH:
+      diagnostic_push_diagnostics (global_dc, input_location);
+      return;
+
+    case pragma_diagnostic_data::PD_KIND_POP:
+      diagnostic_pop_diagnostics (global_dc, input_location);
+      return;
+
+    case pragma_diagnostic_data::PD_KIND_IGNORED_ATTRIBUTES:
+      {
+	if (early)
 	  return;
-	}
-      else if (args[l - 1] == ',')
+	if (!data.option_str)
+	  {
+	    warning_at (data.loc_option, OPT_Wpragmas,
+		       "missing attribute name after %<#pragma GCC diagnostic "
+			"ignored_attributes%>");
+	    return;
+	  }
+	char *args = xstrdup (data.option_str);
+	const size_t l = strlen (args);
+	if (l == 0)
+	  {
+	    warning_at (data.loc_option, OPT_Wpragmas,
+			"missing argument to %<#pragma GCC "
+			"diagnostic ignored_attributes%>");
+	    free (args);
+	    return;
+	  }
+	else if (args[l - 1] == ',')
+	  {
+	    warning_at (data.loc_option, OPT_Wpragmas,
+			"trailing %<,%> in arguments for "
+			"%<#pragma GCC diagnostic ignored_attributes%>");
+	    free (args);
+	    return;
+	  }
+	auto_vec<char *> v;
+	for (char *p = strtok (args, ","); p; p = strtok (NULL, ","))
+	  v.safe_push (p);
+	handle_ignored_attributes_option (&v);
+	free (args);
+	return;
+      }
+
+    case pragma_diagnostic_data::PD_KIND_DIAGNOSTIC:
+      if (!data.option_str)
 	{
-	  warning_at (loc, OPT_Wpragmas, "trailing %<,%> in arguments for "
-		      "%<#pragma GCC diagnostic ignored_attributes%>");
-	  free (args);
+	  if (want_diagnostics)
+	    warning_at (data.loc_option, OPT_Wpragmas,
+			"missing option after %<#pragma GCC diagnostic%> kind");
 	  return;
 	}
-      auto_vec<char *> v;
-      for (char *p = strtok (args, ","); p; p = strtok (NULL, ","))
-	v.safe_push (p);
-      handle_ignored_attributes_option (&v);
-      free (args);
-      return;
-    }
-  else
-    {
-      warning_at (loc, OPT_Wpragmas,
-		  "expected %<error%>, %<warning%>, %<ignored%>, %<push%>, "
-		  "%<pop%>, %<ignored_attributes%> after "
-		  "%<#pragma GCC diagnostic%>");
-      return;
-    }
+      break;
 
-  token = pragma_lex (&x, &loc);
-  if (token != CPP_STRING)
-    {
-      warning_at (loc, OPT_Wpragmas,
-		  "missing option after %<#pragma GCC diagnostic%> kind");
+    default:
+      if (want_diagnostics)
+	warning_at (data.loc_kind, OPT_Wpragmas,
+		    "expected %<error%>, %<warning%>, %<ignored%>, %<push%>, "
+		    "%<pop%>, %<ignored_attributes%> after "
+		    "%<#pragma GCC diagnostic%>");
       return;
+
     }
 
-  const char *option_string = TREE_STRING_POINTER (x);
+  gcc_assert (data.pd_kind == pragma_diagnostic_data::PD_KIND_DIAGNOSTIC);
+  gcc_assert (data.valid);
+
   unsigned int lang_mask = c_common_option_lang_mask () | CL_COMMON;
   /* option_string + 1 to skip the initial '-' */
-  unsigned int option_index = find_opt (option_string + 1, lang_mask);
+  unsigned int option_index = find_opt (data.option_str + 1, lang_mask);
+
+  if (early && !c_option_is_from_cpp_diagnostics (option_index))
+    return;
+
+  const char *arg = NULL;
+  if (cl_options[option_index].flags & CL_JOINED)
+    arg = data.option_str + 1 + cl_options[option_index].opt_len;
+  if (early && arg)
+    {
+      /* This warning is needed because of PR71603 - popping the diagnostic
+	 state does not currently reset changes to option arguments, only
+	 changes to the option dispositions.  */
+      warning_at (data.loc_option, OPT_Wpragmas,
+		  "a diagnostic pragma attempting to modify a preprocessor"
+		  " option argument may not work as expected");
+    }
+
   if (option_index == OPT_SPECIAL_unknown)
     {
-      auto_diagnostic_group d;
-      if (warning_at (loc, OPT_Wpragmas,
-		      "unknown option after %<#pragma GCC diagnostic%> kind"))
+      if (want_diagnostics)
 	{
-	  option_proposer op;
-	  const char *hint = op.suggest_option (option_string + 1);
-	  if (hint)
-	    inform (loc, "did you mean %<-%s%>?", hint);
+	  auto_diagnostic_group d;
+	  if (warning_at (data.loc_option, OPT_Wpragmas,
+			"unknown option after %<#pragma GCC diagnostic%> kind"))
+	    {
+	      option_proposer op;
+	      const char *hint = op.suggest_option (data.option_str + 1);
+	      if (hint)
+		inform (data.loc_option, "did you mean %<-%s%>?", hint);
+	    }
 	}
       return;
     }
   else if (!(cl_options[option_index].flags & CL_WARNING))
     {
-      warning_at (loc, OPT_Wpragmas,
-		  "%qs is not an option that controls warnings", option_string);
+      if (want_diagnostics)
+	warning_at (data.loc_option, OPT_Wpragmas,
+		    "%qs is not an option that controls warnings",
+		    data.option_str);
       return;
     }
   else if (!(cl_options[option_index].flags & lang_mask))
     {
-      char *ok_langs = write_langs (cl_options[option_index].flags);
-      char *bad_lang = write_langs (c_common_option_lang_mask ());
-      warning_at (loc, OPT_Wpragmas,
-		  "option %qs is valid for %s but not for %s",
-		  option_string, ok_langs, bad_lang);
-      free (ok_langs);
-      free (bad_lang);
+      if (want_diagnostics)
+	{
+	  char *ok_langs = write_langs (cl_options[option_index].flags);
+	  char *bad_lang = write_langs (c_common_option_lang_mask ());
+	  warning_at (data.loc_option, OPT_Wpragmas,
+		      "option %qs is valid for %s but not for %s",
+		      data.option_str, ok_langs, bad_lang);
+	  free (ok_langs);
+	  free (bad_lang);
+	}
       return;
     }
 
   struct cl_option_handlers handlers;
   set_default_handlers (&handlers, NULL);
-  const char *arg = NULL;
-  if (cl_options[option_index].flags & CL_JOINED)
-    arg = option_string + 1 + cl_options[option_index].opt_len;
   /* FIXME: input_location isn't the best location here, but it is
      what we used to do here before and changing it breaks e.g.
      PR69543 and PR69558.  */
-  control_warning_option (option_index, (int) kind,
-			  arg, kind != DK_IGNORED,
+  control_warning_option (option_index, (int) data.diagnostic_kind,
+			  arg, data.diagnostic_kind != DK_IGNORED,
 			  input_location, lang_mask, &handlers,
 			  &global_options, &global_options_set,
 			  global_dc);
 }
 
+static void
+handle_pragma_diagnostic (cpp_reader *)
+{
+  handle_pragma_diagnostic_impl<false, false> ();
+}
+
+static void
+handle_pragma_diagnostic_early (cpp_reader *)
+{
+  handle_pragma_diagnostic_impl<true, false> ();
+}
+
+static void
+handle_pragma_diagnostic_early_pp (cpp_reader *)
+{
+  handle_pragma_diagnostic_impl<true, true> ();
+}
+
 /*  Parse #pragma GCC target (xxx) to set target specific options.  */
 static void
 handle_pragma_target(cpp_reader *)
@@ -1332,14 +1522,15 @@ handle_pragma_float_const_decimal64 (cpp_reader *)
 
 static vec<internal_pragma_handler> registered_pragmas;
 
-struct pragma_ns_name
+struct pragma_pp_data
 {
   const char *space;
   const char *name;
+  pragma_handler_1arg early_handler;
 };
 
 
-static vec<pragma_ns_name> registered_pp_pragmas;
+static vec<pragma_pp_data> registered_pp_pragmas;
 
 struct omp_pragma_def { const char *name; unsigned int id; };
 static const struct omp_pragma_def oacc_pragmas[] = {
@@ -1452,14 +1643,14 @@ c_register_pragma_1 (const char *space, const char *name,
 
   if (flag_preprocess_only)
     {
-      pragma_ns_name ns_name;
-
-      if (!allow_expansion)
+      if (!(allow_expansion || ihandler.early_handler.handler_1arg))
 	return;
 
-      ns_name.space = space;
-      ns_name.name = name;
-      registered_pp_pragmas.safe_push (ns_name);
+      pragma_pp_data pp_data;
+      pp_data.space = space;
+      pp_data.name = name;
+      pp_data.early_handler = ihandler.early_handler.handler_1arg;
+      registered_pp_pragmas.safe_push (pp_data);
       id = registered_pp_pragmas.length ();
       id += PRAGMA_FIRST_EXTERNAL - 1;
     }
@@ -1484,10 +1675,17 @@ c_register_pragma_1 (const char *space, const char *name,
 void
 c_register_pragma (const char *space, const char *name,
                    pragma_handler_1arg handler)
+{
+  c_register_pragma_with_early_handler (space, name, handler, nullptr);
+}
+void c_register_pragma_with_early_handler (const char *space, const char *name,
+					   pragma_handler_1arg handler,
+					   pragma_handler_1arg early_handler)
 {
   internal_pragma_handler ihandler;
 
   ihandler.handler.handler_1arg = handler;
+  ihandler.early_handler.handler_1arg = early_handler;
   ihandler.extra_data = false;
   ihandler.data = NULL;
   c_register_pragma_1 (space, name, ihandler, false);
@@ -1504,6 +1702,7 @@ c_register_pragma_with_data (const char *space, const char *name,
   internal_pragma_handler ihandler;
 
   ihandler.handler.handler_2arg = handler;
+  ihandler.early_handler.handler_2arg = nullptr;
   ihandler.extra_data = true;
   ihandler.data = data;
   c_register_pragma_1 (space, name, ihandler, false);
@@ -1523,6 +1722,7 @@ c_register_pragma_with_expansion (const char *space, const char *name,
   internal_pragma_handler ihandler;
 
   ihandler.handler.handler_1arg = handler;
+  ihandler.early_handler.handler_1arg = nullptr;
   ihandler.extra_data = false;
   ihandler.data = NULL;
   c_register_pragma_1 (space, name, ihandler, true);
@@ -1544,6 +1744,7 @@ c_register_pragma_with_expansion_and_data (const char *space, const char *name,
   internal_pragma_handler ihandler;
 
   ihandler.handler.handler_2arg = handler;
+  ihandler.early_handler.handler_2arg = nullptr;
   ihandler.extra_data = true;
   ihandler.data = data;
   c_register_pragma_1 (space, name, ihandler, true);
@@ -1570,6 +1771,38 @@ c_invoke_pragma_handler (unsigned int id)
     }
 }
 
+/* In contrast to the normal handler, the early handler is optional.  */
+void
+c_invoke_early_pragma_handler (unsigned int id)
+{
+  internal_pragma_handler *ihandler;
+  pragma_handler_1arg handler_1arg;
+  pragma_handler_2arg handler_2arg;
+
+  id -= PRAGMA_FIRST_EXTERNAL;
+  ihandler = &registered_pragmas[id];
+  if (ihandler->extra_data)
+    {
+      handler_2arg = ihandler->early_handler.handler_2arg;
+      if (handler_2arg)
+	handler_2arg (parse_in, ihandler->data);
+    }
+  else
+    {
+      handler_1arg = ihandler->early_handler.handler_1arg;
+      if (handler_1arg)
+	handler_1arg (parse_in);
+    }
+}
+
+void
+c_pp_invoke_early_pragma_handler (unsigned int id)
+{
+  const auto data = &registered_pp_pragmas[id - PRAGMA_FIRST_EXTERNAL];
+  if (data->early_handler)
+    data->early_handler (parse_in);
+}
+
 /* Set up front-end pragmas.  */
 void
 init_pragma (void)
@@ -1625,7 +1858,14 @@ init_pragma (void)
 
   c_register_pragma ("GCC", "visibility", handle_pragma_visibility);
 
-  c_register_pragma ("GCC", "diagnostic", handle_pragma_diagnostic);
+  if (flag_preprocess_only)
+    c_register_pragma_with_early_handler ("GCC", "diagnostic",
+					  nullptr,
+					  handle_pragma_diagnostic_early_pp);
+  else
+    c_register_pragma_with_early_handler ("GCC", "diagnostic",
+					  handle_pragma_diagnostic,
+					  handle_pragma_diagnostic_early);
   c_register_pragma ("GCC", "target", handle_pragma_target);
   c_register_pragma ("GCC", "optimize", handle_pragma_optimize);
   c_register_pragma ("GCC", "push_options", handle_pragma_push_options);
diff --git a/gcc/c-family/c-pragma.h b/gcc/c-family/c-pragma.h
index 54864c2ec41..ad04cc36a21 100644
--- a/gcc/c-family/c-pragma.h
+++ b/gcc/c-family/c-pragma.h
@@ -218,7 +218,7 @@ union gen_pragma_handler {
 };
 /* Internally used to keep the data of the handler.  */
 struct internal_pragma_handler {
-  union gen_pragma_handler handler;
+  union gen_pragma_handler handler, early_handler;
   /* Permits to know if handler is a pragma_handler_1arg (extra_data is false)
      or a pragma_handler_2arg (extra_data is true).  */
   bool extra_data;
@@ -241,6 +241,17 @@ extern void c_register_pragma_with_expansion_and_data (const char *space,
                                                        void *data);
 extern void c_invoke_pragma_handler (unsigned int);
 
+/* Early pragma handlers run in addition to the normal ones.  They can be used
+   by frontends such as C++ that may want to process some pragmas during lexing
+   before they start processing them.  */
+extern void
+c_register_pragma_with_early_handler (const char *space, const char *name,
+				      pragma_handler_1arg handler,
+				      pragma_handler_1arg early_handler);
+extern void c_invoke_early_pragma_handler (unsigned int);
+extern void c_pp_invoke_early_pragma_handler (unsigned int);
+
+
 extern void maybe_apply_pragma_weak (tree);
 extern void maybe_apply_pending_pragma_weaks (void);
 extern tree maybe_apply_renaming_pragma (tree, tree);
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 9559f84b113..8da43322ebb 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -639,8 +639,58 @@ cp_token_is_module_directive (cp_token *token)
     || token->keyword == RID__IMPORT;
 }
 
+/* Return TOKEN's pragma_kind if it is CPP_PRAGMA, otherwise
+   PRAGMA_NONE.  */
+
+static enum pragma_kind
+cp_parser_pragma_kind (cp_token *token)
+{
+  if (token->type != CPP_PRAGMA)
+    return PRAGMA_NONE;
+  /* We smuggled the cpp_token->u.pragma value in an INTEGER_CST.  */
+  return (enum pragma_kind) TREE_INT_CST_LOW (token->u.value);
+}
+
+/* Handle #pragma gcc diagnostic, which needs to be done during preprocessing
+   for the case of preprocessing-related diagnostics.  */
+static void
+cp_lexer_handle_early_pragma (cp_lexer *lexer)
+{
+  const auto first_token = lexer->buffer->address ();
+  const auto last_token = first_token + lexer->buffer->length () - 1;
+
+  /* Back up to the start of the pragma so pragma_lex () can parse it when
+     c-pragma lib asks it to.  */
+  auto begin = last_token;
+  gcc_assert (begin->type == CPP_PRAGMA_EOL);
+  while (begin->type != CPP_PRAGMA)
+    {
+      if (cp_token_is_module_directive (begin))
+	return;
+      gcc_assert (begin != first_token);
+      --begin;
+    }
+  gcc_assert (!lexer->next_token);
+  gcc_assert (!lexer->last_token);
+  lexer->next_token = begin;
+  lexer->last_token = last_token;
+
+  /* Dispatch it.  */
+  const unsigned int id
+    = cp_parser_pragma_kind (cp_lexer_consume_token (lexer));
+  if (id >= PRAGMA_FIRST_EXTERNAL)
+    c_invoke_early_pragma_handler (id);
+
+  /* Reset to normal state.  */
+  lexer->next_token = lexer->last_token = nullptr;
+}
+
+/* The parser.  */
+static cp_parser *cp_parser_new (cp_lexer *);
+static GTY (()) cp_parser *the_parser;
+
 /* Create a new main C++ lexer, the lexer that gets tokens from the
-   preprocessor.  */
+   preprocessor, and also create the main parser.  */
 
 static cp_lexer *
 cp_lexer_new_main (void)
@@ -662,6 +712,10 @@ cp_lexer_new_main (void)
   if (modules_p ())
     filter = module_token_cdtor (parse_in, filter);
 
+  /* Create the parser now, so we can use it to handle early pragmas.  */
+  gcc_assert (!the_parser);
+  the_parser = cp_parser_new (lexer);
+
   /* Get the remaining tokens from the preprocessor.  */
   while (tok->type != CPP_EOF)
     {
@@ -669,6 +723,11 @@ cp_lexer_new_main (void)
 	/* Process the previous token.  */
 	module_token_lang (tok->type, tok->keyword, tok->u.value,
 			   tok->location, filter);
+
+      /* Check for early pragmas that need to be handled now.  */
+      if (tok->type == CPP_PRAGMA_EOL)
+	cp_lexer_handle_early_pragma (lexer);
+
       tok = vec_safe_push (lexer->buffer, cp_token ());
       cp_lexer_get_preprocessor_token (C_LEX_STRING_NO_JOIN, tok);
     }
@@ -2127,11 +2186,6 @@ pop_unparsed_function_queues (cp_parser *parser)
 
 /* Prototypes.  */
 
-/* Constructors and destructors.  */
-
-static cp_parser *cp_parser_new
-  (cp_lexer *);
-
 /* Routines to parse various constructs.
 
    Those that return `tree' will return the error_mark_node (rather
@@ -2894,18 +2948,6 @@ cp_parser_is_keyword (cp_token* token, enum rid keyword)
   return token->keyword == keyword;
 }
 
-/* Return TOKEN's pragma_kind if it is CPP_PRAGMA, otherwise
-   PRAGMA_NONE.  */
-
-static enum pragma_kind
-cp_parser_pragma_kind (cp_token *token)
-{
-  if (token->type != CPP_PRAGMA)
-    return PRAGMA_NONE;
-  /* We smuggled the cpp_token->u.pragma value in an INTEGER_CST.  */
-  return (enum pragma_kind) TREE_INT_CST_LOW (token->u.value);
-}
-
 /* Helper function for cp_parser_error.
    Having peeked a token of kind TOK1_KIND that might signify
    a conflict marker, peek successor tokens to determine
@@ -47737,11 +47779,7 @@ cp_parser_transaction_cancel (cp_parser *parser)
   return stmt;
 }
 
-/* The parser.  */
-
-static GTY (()) cp_parser *the_parser;
 
-
 /* Special handling for the first token or line in the file.  The first
    thing in the file might be #pragma GCC pch_preprocess, which loads a
    PCH file, which is a GC collection point.  So we need to handle this
@@ -48236,9 +48274,7 @@ c_parse_file (void)
 
   /* cp_lexer_new_main is called before doing any GC allocation
      because tokenization might load a PCH file.  */
-  cp_lexer *lexer = cp_lexer_new_main ();
-
-  the_parser = cp_parser_new (lexer);
+  cp_lexer_new_main ();
 
   cp_parser_translation_unit (the_parser);
   class_decl_loc_t::diag_mismatched_tags ();
diff --git a/gcc/testsuite/c-c++-common/pragma-diag-11.c b/gcc/testsuite/c-c++-common/pragma-diag-11.c
new file mode 100644
index 00000000000..2eef5c418df
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pragma-diag-11.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-Wundef" } */
+#pragma GCC diagnostic ignored "-Wundef"
+#if FOO
+#endif
+#define P _Pragma ("GCC diagnostic push") _Pragma ("GCC diagnostic warning \"-Wundef\"")
+P
+#if FOO2 /* { dg-warning "is not defined" } */
+#endif
+#pragma GCC diagnostic pop
+#if FOO3
+#endif
+int i;
diff --git a/gcc/testsuite/c-c++-common/pragma-diag-12.c b/gcc/testsuite/c-c++-common/pragma-diag-12.c
new file mode 100644
index 00000000000..0043a4287a0
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pragma-diag-12.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-E -Wdate-time" } */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdate-time"
+const char *date = __DATE__;
+_Pragma ("GCC diagnostic pop");
+const char *date2 = __DATE__; /* { dg-warning "__DATE__" } */
+/* { dg-final { scan-assembler "#pragma GCC diagnostic push" } } */
+/* { dg-final { scan-assembler "#pragma GCC diagnostic ignored \"-Wdate-time\"" } } */
+/* { dg-final { scan-assembler "#pragma GCC diagnostic pop" } } */
diff --git a/gcc/testsuite/c-c++-common/pragma-diag-13.c b/gcc/testsuite/c-c++-common/pragma-diag-13.c
new file mode 100644
index 00000000000..d67b3655639
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pragma-diag-13.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+#pragma GCC diagnostic /* { dg-warning "missing" } */
+#pragma GCC diagnostic warn /* { dg-warning "24:expected" } */
+#pragma GCC diagnostic ignored "-Wfoo" /* { dg-warning "32:unknown" } */
  

Patch

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 868b8610d60..9559f84b113 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -629,6 +629,16 @@  cp_lexer_alloc (void)
   return lexer;
 }
 
+/* Return TRUE if token is the start of a module declaration that will be
+   terminated by a CPP_PRAGMA_EOL token.  */
+static inline bool
+cp_token_is_module_directive (cp_token *token)
+{
+  return token->keyword == RID__EXPORT
+    || token->keyword == RID__MODULE
+    || token->keyword == RID__IMPORT;
+}
+
 /* Create a new main C++ lexer, the lexer that gets tokens from the
    preprocessor.  */
 
@@ -3829,9 +3839,7 @@  cp_parser_skip_to_closing_parenthesis_1 (cp_parser *parser,
 	  break;
 
 	case CPP_KEYWORD:
-	  if (token->keyword != RID__EXPORT
-	      && token->keyword != RID__MODULE
-	      && token->keyword != RID__IMPORT)
+	  if (!cp_token_is_module_directive (token))
 	    break;
 	  /* FALLTHROUGH  */
 
@@ -3932,9 +3940,7 @@  cp_parser_skip_to_end_of_statement (cp_parser* parser)
 	  break;
 
 	case CPP_KEYWORD:
-	  if (token->keyword != RID__EXPORT
-	      && token->keyword != RID__MODULE
-	      && token->keyword != RID__IMPORT)
+	  if (!cp_token_is_module_directive (token))
 	    break;
 	  /* FALLTHROUGH  */
 
@@ -4021,9 +4027,7 @@  cp_parser_skip_to_end_of_block_or_statement (cp_parser* parser)
 	  break;
 
 	case CPP_KEYWORD:
-	  if (token->keyword != RID__EXPORT
-	      && token->keyword != RID__MODULE
-	      && token->keyword != RID__IMPORT)
+	  if (!cp_token_is_module_directive (token))
 	    break;
 	  /* FALLTHROUGH  */
 
@@ -14939,9 +14943,7 @@  cp_parser_declaration (cp_parser* parser, tree prefix_attrs)
       else
 	cp_parser_module_export (parser);
     }
-  else if (token1->keyword == RID__EXPORT
-	   || token1->keyword == RID__IMPORT
-	   || token1->keyword == RID__MODULE)
+  else if (cp_token_is_module_directive (token1))
     {
       bool exporting = token1->keyword == RID__EXPORT;
       cp_token *next = exporting ? token2 : token1;