Re: [PATCH V2 1/1] Unify registered_pp_pragmas and registered_pragmas

[Jason Merrill]

On 11/6/24 8:08 AM, Paul Iannetta wrote:

On Mon, Nov 04, 2024 at 01:36:56PM -0500, Jason Merrill wrote:

On 11/3/24 12:26 PM, Paul Iannetta wrote:

On Fri, Nov 01, 2024 at 11:45:07AM -0400, Jason Merrill wrote:

On 10/31/24 6:43 AM, Paul Iannetta wrote:

gcc/c-family/ChangeLog:

        * c-pragma.cc (struct pragma_pp_data): Use (struct 
internal_pragma_handler);
        (c_register_pragma_1): Always register name and space for all pragmas.

If we're using the _pp data structure for all pragmas now, I'd think we
should rename it to remove _pp?

Thank you for the feedback.  Do you think we should also keep the
"registered_pragma" variable rather than the "registered_pp_pragma"
variable then?

I think the single remaining variable should be called "registered_pragmas",
yes.

Below the second version of the patch.  I rename every instance of
pragma_pp_data to pragma_data, and unified registered_pragma and
registered_pp_pragma under register_pragma.

OK, for trunk?

OK.

Thanks,
Paul

----------------------------------------------------------------------
[PATCH] Unify registered_pp_pragmas and registered_pragmas

Until now, the structures that keep pragma information were different
when in preprocessing only mode and in normal mode.  This change unifies
both so that the space and name of a pragma are always registered and
can be queried easily at a later time.

gcc/c-family/ChangeLog:

        * c-pragma.cc (struct pragma_data): Use (struct 
internal_pragma_handler);
        (c_register_pragma_1): Always register name and space for all pragmas.
        (c_invoke_pragma_handler): Adapt.
        (c_invoke_early_pragma_handler): Likewise.
        (c_pp_invoke_early_pragma_handler): Likewise.
---
  gcc/c-family/c-pragma.cc | 66 ++++++++++++++++++----------------------
  1 file changed, 29 insertions(+), 37 deletions(-)

diff --git a/gcc/c-family/c-pragma.cc b/gcc/c-family/c-pragma.cc
index de7c378965d..c78721824e3 100644
--- a/gcc/c-family/c-pragma.cc
+++ b/gcc/c-family/c-pragma.cc
@@ -1488,17 +1488,15 @@ handle_pragma_float_const_decimal64 (cpp_reader *)

/* A vector of registered pragma callbacks, which is never freed. */ -static vec<internal_pragma_handler> registered_pragmas; -struct pragma_pp_data

+struct pragma_data
  {
    const char *space;
    const char *name;
-  pragma_handler_1arg early_handler;
+  struct internal_pragma_handler ihandler;
  };

-

-static vec<pragma_pp_data> registered_pp_pragmas;
+static vec<pragma_data> registered_pragmas;

struct omp_pragma_def { const char *name; unsigned int id; };

  static const struct omp_pragma_def oacc_pragmas[] = {
@@ -1594,10 +1592,10 @@ c_pp_lookup_pragma (unsigned int id, const char 
**space, const char **name)
        }

if (id >= PRAGMA_FIRST_EXTERNAL

-      && (id < PRAGMA_FIRST_EXTERNAL + registered_pp_pragmas.length ()))
+      && (id < PRAGMA_FIRST_EXTERNAL + registered_pragmas.length ()))
      {
-      *space = registered_pp_pragmas[id - PRAGMA_FIRST_EXTERNAL].space;
-      *name = registered_pp_pragmas[id - PRAGMA_FIRST_EXTERNAL].name;
+      *space = registered_pragmas[id - PRAGMA_FIRST_EXTERNAL].space;
+      *name = registered_pragmas[id - PRAGMA_FIRST_EXTERNAL].name;
        return;
      }

@@ -1613,31 +1611,24 @@ c_register_pragma_1 (const char *space, const char *name,

  {
    unsigned id;

- if (flag_preprocess_only)

-    {
-      if (cpp_get_options (parse_in)->directives_only
-         || !(allow_expansion || ihandler.early_handler.handler_1arg))
-       return;
+  pragma_data data;
+  data.space = space;
+  data.name = 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;
-    }
-  else
-    {
-      registered_pragmas.safe_push (ihandler);
-      id = registered_pragmas.length ();
-      id += PRAGMA_FIRST_EXTERNAL - 1;
-
-      /* The C front end allocates 8 bits in c_token.  The C++ front end
-        keeps the pragma kind in the form of INTEGER_CST, so no small
-        limit applies.  At present this is sufficient.  */
-      gcc_assert (id < 256);
-    }
+  if (flag_preprocess_only
+      && (cpp_get_options (parse_in)->directives_only
+       || !(allow_expansion || ihandler.early_handler.handler_1arg)))
+    return;
+
+  data.ihandler = ihandler;
+  registered_pragmas.safe_push (data);
+  id = registered_pragmas.length ();
+  id += PRAGMA_FIRST_EXTERNAL - 1;
+
+  /* The C front end allocates 8 bits in c_token.  The C++ front end
+     keeps the pragma kind in the form of INTEGER_CST, so no small
+     limit applies.  At present this is sufficient.  */
+  gcc_assert (id < 256);

cpp_register_deferred_pragma (parse_in, space, name, id,

                                allow_expansion, false);
@@ -1731,7 +1722,7 @@ c_invoke_pragma_handler (unsigned int id)
    pragma_handler_2arg handler_2arg;

id -= PRAGMA_FIRST_EXTERNAL;

-  ihandler = &registered_pragmas[id];
+  ihandler = &registered_pragmas[id].ihandler;
    if (ihandler->extra_data)
      {
        handler_2arg = ihandler->handler.handler_2arg;
@@ -1753,7 +1744,7 @@ c_invoke_early_pragma_handler (unsigned int id)
    pragma_handler_2arg handler_2arg;

id -= PRAGMA_FIRST_EXTERNAL;

-  ihandler = &registered_pragmas[id];
+  ihandler = &registered_pragmas[id].ihandler;
    if (ihandler->extra_data)
      {
        handler_2arg = ihandler->early_handler.handler_2arg;
@@ -1771,10 +1762,11 @@ c_invoke_early_pragma_handler (unsigned int id)
  void
  c_pp_invoke_early_pragma_handler (unsigned int id)
  {
-  const auto data = &registered_pp_pragmas[id - PRAGMA_FIRST_EXTERNAL];
-  if (data->early_handler)
+  const auto data = &registered_pragmas[id - PRAGMA_FIRST_EXTERNAL];
+  pragma_handler_1arg handler = data->ihandler.early_handler.handler_1arg;
+  if (handler)
      {
-      data->early_handler (parse_in);
+      handler (parse_in);
        pragma_lex_discard_to_eol ();
      }
  }