On 8/16/24 04:30, Jakub Jelinek wrote:
On Sat, Jul 20, 2024 at 02:42:23PM -0600, Sandra Loosemore wrote:
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -263,9 +263,24 @@ struct GTY(()) c_parser {
       otherwise NULL.  */
    vec<c_token, va_gc> *in_omp_attribute_pragma;
+ /* When in_omp_attribute_pragma is non-null, these fields save the values
+     of the tokens and tokens_avail fields, so that they can be restored
+     after parsing the attribute.  Note that parsing the body of a
+     metadirective uses its own save/restore mechanism as those can be
+     nested with or without the attribute pragmas in the body.  */
+    c_token * GTY((skip)) save_tokens;
+    unsigned int save_tokens_avail;

The indentation of the above 2 is wrong.
Plus if those members are for metadirective parsing, their names are too
generic.

They are not for metadirective parsing, they are to generalize the state-saving for OpenMP attribute-syntax directives that are converted to pragmas. It's related to this patch hunk:

@@ -6846,7 +6884,6 @@ c_parser_handle_statement_omp_attributes (c_parser *parser
, tree &attrs,
     return false;

   unsigned int tokens_avail = parser->tokens_avail;
-  gcc_assert (parser->tokens == &parser->tokens_buf[0]);

   tokens++;
   vec<c_token, va_gc> *toks = NULL;

and the multiple instances like this:

@@ -1307,8 +1322,10 @@ c_parser_skip_until_found (c_parser *parser,
          c_token *token = c_parser_peek_token (parser);
          if (token->type == CPP_EOF)
            {
-             parser->tokens = &parser->tokens_buf[0];
-             parser->tokens_avail = token->flags;
+             parser->tokens = parser->save_tokens;
+             parser->save_tokens = NULL;
+             parser->tokens_avail = parser->save_tokens_avail;
+             parser->save_tokens_avail = 0;
              parser->in_omp_attribute_pragma = NULL;
            }
        }

where there presently is a hard-wired assumption that omp attribute pragma parsing is the *only* thing that messes with what parser->tokens points to. Metadirectives need to maintain a separate token stream too, and attribute-syntax pragma directives can appear inside a metadirective, so fixing this somehow is necessary. Due to the control flow of parsing the attribute-syntax pragmas it's not possible to save the state it needs in local variables on the stack, so hanging it off the parser object is about the only choice.

But more importantly, for something parsed really rarely, wouldn't it be
better to just add a single pointer to a new structure that contains
all you need for metadirective parsing?

Do you want me to create separate new data structures for both the attribute-syntax pragma parsing, and the metadirective-specific bits? And maybe submit the attribute-syntax pragma parsing state changes as its own patch separate from the metadirective pieces?

-Sandra

Reply via email to