Patchwork c-pragma: adding a data field to pragma_handler

login
register
mail settings
Submitter Pierre
Date June 1, 2011, 4:54 p.m.
Message ID <4DE66ECE.5030102@laposte.net>
Download mbox | patch
Permalink /patch/98220/
State New
Headers show

Comments

Pierre - June 1, 2011, 4:54 p.m.
This patch is about the pragmas.

In c-family/c-pragma.h, we declare a pragma_handler which is a function 
accepting cpp_reader as parameter.

I have changed this handler in order to accept a second parameter which 
is a void *, allowing to give extra datas to the handler. I think this 
data field might be of general use: we can have condition or data at 
register time that we want to express in the handler. I guess this is a 
common way to pass data to an handler function.

I would like your opinion on this patch! Thanks!

Pierre Vittet

Changelog

2011-06-01  Pierre Vittet  <piervit@pvittet.com>

* c-pragma.h (pragma_handler,internal_pragma_handler, c_register_pragma,
	c_register_pragma_with_expansion): create internal_pragma_handler, add a
	new void * data parameter.
	* c-pragma.c (handle_pragma_pack, handle_pragma_weak,
	handle_pragma_redefine_extname, handle_pragma_visibility,
	handle_pragma_diagnostic, handle_pragma_target, handle_pragma_optimize,
	handle_pragma_push_options, handle_pragma_pop_options,
	handle_pragma_reset_options, handle_pragma_message,
	handle_pragma_float_const_decimal64, registered_pragmas,
	c_register_pragma_1, c_register_pragma, c_register_pragma_with_expansion,
	init_pragma): add support of the void * data field.
Basile Starynkevitch - June 1, 2011, 5:53 p.m.
On Wed, 01 Jun 2011 18:54:38 +0200
Pierre <p.vittet@laposte.net> wrote:

> This patch is about the pragmas.
> 
> In c-family/c-pragma.h, we declare a pragma_handler which is a function 
> accepting cpp_reader as parameter.
> 
> I have changed this handler in order to accept a second parameter which 
> is a void *, allowing to give extra datas to the handler. I think this 
> data field might be of general use: we can have condition or data at 
> register time that we want to express in the handler. I guess this is a 
> common way to pass data to an handler function.

I find this patch interesting and useful (& not only for MELT).

A general coding rule in C seems to be that every time function can be
variably called thru indirect pointers (which can have several
different functions as value), they better take an extra data argument.
This is the case, in particular, inside Glib & GTK, inside the Linux
kernel, and on several other occurrences in GCC.

A use case of such pragma handlers with data for pragmas would be a
plugin which permit some messages to other channels than stdout/stderr
(e.g. other files, or a pipe, or the D-Bus, or a widget, or a web
service...). Then the same routine would handle 
   #pragma GCCPLUGIN message_to_file "foo"
and 
   #pragma GCCPLUGIN message_to_pipe "bar"
and the data pointer would be different (an fopen-ed or popen-ed
FILE*, or even an std::ostream& if the plugin is coded in C++). 


I am not authorized to ok the patch (I believe the changelog had some
typos), but I hope someone will review & ok it.

Regards
Tom Tromey - June 2, 2011, 5:51 p.m.
>>>>> "Pierre" == Pierre  <p.vittet@laposte.net> writes:

Pierre> I have changed this handler in order to accept a second parameter
Pierre> which is a void *, allowing to give extra datas to the handler. I
Pierre> think this data field might be of general use: we can have condition
Pierre> or data at register time that we want to express in the handler. I
Pierre> guess this is a common way to pass data to an handler function.

I can't approve or reject this patch, but the idea seems reasonable
enough to me.

Pierre> I would like your opinion on this patch! Thanks!

It has a number of formatting issues.

Pierre> +typedef void (*pragma_handler)(struct cpp_reader *, void * );

No space after the final "*".

Pierre> +/* Internally use to keep the data of the handler.  */
Pierre> +struct internal_pragma_handler_d{

Space before the "{".

Pierre> +  pragma_handler handler;
Pierre> +  void * data; 

No space.  Lots of instances of this.

Pierre>  /* A vector of registered pragma callbacks.  */
Pierre> +/*This is never freed as we need it during the whole execution */

Coalesce the two comments.  The comment formatting is wrong, see GNU
standards.

Pierre>        ns_name.space = space;
Pierre>        ns_name.name = name;
Pierre> +      
Pierre>        VEC_safe_push (pragma_ns_name, heap, registered_pp_pragmas, &ns_name);

Gratuitous newline addition.

Pierre> +      ihandler->handler = handler;
Pierre> +      ihandler->data = data;

I didn't see anything that initialized ihandler.

Pierre> +      VEC_safe_push (internal_pragma_handler, heap, registered_pragmas,
Pierre> +                                                    &ihandler);

I think you wanted just `internal_pragma_handler ihandler', no "*", for
the definition.

Pierre> +c_register_pragma (const char *space, const char *name, pragma_handler handler, 
Pierre> +                   void * data)

There are lots of calls to this that you did not update.
Do a recursive grep to see.

One way to avoid a massive change is to add a new "overload" that passes
in the data to c_register_pragma_1; and then change the "legacy"
functions to pass NULL.

I don't know if that approach is ok (it is typical in gdb...), so if
not, you have to update all callers.

Tom

Patch

Index: gcc/c-family/c-pragma.h
===================================================================
--- gcc/c-family/c-pragma.h	(revision 174521)
+++ gcc/c-family/c-pragma.h	(working copy)
@@ -84,10 +84,19 @@  extern bool pop_visibility (int);
 extern void init_pragma (void);
 
 /* Front-end wrappers for pragma registration.  */
-typedef void (*pragma_handler)(struct cpp_reader *);
-extern void c_register_pragma (const char *, const char *, pragma_handler);
-extern void c_register_pragma_with_expansion (const char *, const char *,
-					      pragma_handler);
+/* The void * allows to pass extra data to the handler.  */
+typedef void (*pragma_handler)(struct cpp_reader *, void * );
+/* Internally use to keep the data of the handler.  */
+struct internal_pragma_handler_d{
+  pragma_handler handler;
+  void * data; 
+};
+typedef struct internal_pragma_handler_d internal_pragma_handler;
+
+extern void c_register_pragma (const char * space, const char * name,
+                               pragma_handler handler, void * data);
+extern void c_register_pragma_with_expansion (const char * space, 
+                      const char * name,  pragma_handler handler , void * data);
 extern void c_invoke_pragma_handler (unsigned int);
 
 extern void maybe_apply_pragma_weak (tree);
Index: gcc/c-family/c-pragma.c
===================================================================
--- gcc/c-family/c-pragma.c	(revision 174521)
+++ gcc/c-family/c-pragma.c	(working copy)
@@ -53,7 +53,7 @@  typedef struct GTY(()) align_stack {
 
 static GTY(()) struct align_stack * alignment_stack;
 
-static void handle_pragma_pack (cpp_reader *);
+static void handle_pragma_pack (cpp_reader *, void * data);
 
 /* If we have a "global" #pragma pack(<n>) in effect when the first
    #pragma pack(push,<n>) is encountered, this stores the value of
@@ -133,7 +133,7 @@  pop_alignment (tree id)
    #pragma pack (pop)
    #pragma pack (pop, ID) */
 static void
-handle_pragma_pack (cpp_reader * ARG_UNUSED (dummy))
+handle_pragma_pack (cpp_reader * ARG_UNUSED (dummy), void * ARG_UNUSED (data))
 {
   tree x, id = 0;
   int align = -1;
@@ -247,7 +247,7 @@  DEF_VEC_ALLOC_O(pending_weak,gc);
 static GTY(()) VEC(pending_weak,gc) *pending_weaks;
 
 static void apply_pragma_weak (tree, tree);
-static void handle_pragma_weak (cpp_reader *);
+static void handle_pragma_weak (cpp_reader *, void * data);
 
 static void
 apply_pragma_weak (tree decl, tree value)
@@ -334,7 +334,7 @@  maybe_apply_pending_pragma_weaks (void)
 
 /* #pragma weak name [= value] */
 static void
-handle_pragma_weak (cpp_reader * ARG_UNUSED (dummy))
+handle_pragma_weak (cpp_reader * ARG_UNUSED (dummy), void * ARG_UNUSED (data))
 {
   tree name, value, x, decl;
   enum cpp_ttype t;
@@ -411,11 +411,12 @@  DEF_VEC_ALLOC_O(pending_redefinition,gc);
 
 static GTY(()) VEC(pending_redefinition,gc) *pending_redefine_extname;
 
-static void handle_pragma_redefine_extname (cpp_reader *);
+static void handle_pragma_redefine_extname (cpp_reader *, void * data);
 
 /* #pragma redefine_extname oldname newname */
 static void
-handle_pragma_redefine_extname (cpp_reader * ARG_UNUSED (dummy))
+handle_pragma_redefine_extname (cpp_reader * ARG_UNUSED (dummy), 
+                                void * ARG_UNUSED (data))
 {
   tree oldname, newname, decl, x;
   enum cpp_ttype t;
@@ -481,7 +482,8 @@  static GTY(()) tree pragma_extern_prefix;
 
 /* #pragma extern_prefix "prefix" */
 static void
-handle_pragma_extern_prefix (cpp_reader * ARG_UNUSED (dummy))
+handle_pragma_extern_prefix (cpp_reader * ARG_UNUSED (dummy), 
+                             void * ARG_UNUSED (data))
 {
   tree prefix, x;
   enum cpp_ttype t;
@@ -594,7 +596,7 @@  maybe_apply_renaming_pragma (tree decl, tree asmna
 }
 
 
-static void handle_pragma_visibility (cpp_reader *);
+static void handle_pragma_visibility (cpp_reader *, void * data);
 
 static VEC (int, heap) *visstack;
 
@@ -644,7 +646,8 @@  pop_visibility (int kind)
    specified on the command line.  */
 
 static void
-handle_pragma_visibility (cpp_reader *dummy ATTRIBUTE_UNUSED)
+handle_pragma_visibility (cpp_reader *dummy ATTRIBUTE_UNUSED, 
+                          void * ARG_UNUSED (data))
 {
   /* Form is #pragma GCC visibility push(hidden)|pop */
   tree x;
@@ -687,7 +690,8 @@  static void
 }
 
 static void
-handle_pragma_diagnostic(cpp_reader *ARG_UNUSED(dummy))
+handle_pragma_diagnostic(cpp_reader *ARG_UNUSED(dummy), 
+                         void * ARG_UNUSED (data))
 {
   const char *kind_string, *option_string;
   unsigned int option_index;
@@ -738,7 +742,7 @@  static void
 
 /*  Parse #pragma GCC target (xxx) to set target specific options.  */
 static void
-handle_pragma_target(cpp_reader *ARG_UNUSED(dummy))
+handle_pragma_target(cpp_reader *ARG_UNUSED(dummy), void * ARG_UNUSED (data))
 {
   enum cpp_ttype token;
   tree x;
@@ -806,7 +810,7 @@  static void
 
 /* Handle #pragma GCC optimize to set optimization options.  */
 static void
-handle_pragma_optimize (cpp_reader *ARG_UNUSED(dummy))
+handle_pragma_optimize (cpp_reader *ARG_UNUSED(dummy), void * ARG_UNUSED (data))
 {
   enum cpp_ttype token;
   tree x;
@@ -893,7 +897,8 @@  static GTY(()) struct opt_stack * options_stack;
    options.  */
 
 static void
-handle_pragma_push_options (cpp_reader *ARG_UNUSED(dummy))
+handle_pragma_push_options (cpp_reader *ARG_UNUSED(dummy), 
+                            void * ARG_UNUSED (data))
 {
   enum cpp_ttype token;
   tree x = 0;
@@ -923,7 +928,8 @@  static void
    optimization options from a previous push_options.  */
 
 static void
-handle_pragma_pop_options (cpp_reader *ARG_UNUSED(dummy))
+handle_pragma_pop_options (cpp_reader *ARG_UNUSED(dummy), 
+                           void * ARG_UNUSED (data))
 {
   enum cpp_ttype token;
   tree x = 0;
@@ -971,7 +977,8 @@  static void
    optimization options to the original options used on the command line.  */
 
 static void
-handle_pragma_reset_options (cpp_reader *ARG_UNUSED(dummy))
+handle_pragma_reset_options (cpp_reader *ARG_UNUSED(dummy), 
+                             void * ARG_UNUSED (data))
 {
   enum cpp_ttype token;
   tree x = 0;
@@ -1007,7 +1014,7 @@  static void
 /* Print a plain user-specified message.  */
 
 static void
-handle_pragma_message (cpp_reader *ARG_UNUSED(dummy))
+handle_pragma_message (cpp_reader *ARG_UNUSED(dummy), void * ARG_UNUSED (data))
 {
   enum cpp_ttype token;
   tree x, message = 0;
@@ -1110,7 +1117,8 @@  handle_stdc_pragma (const char *pname)
    #pragma STDC FLOAT_CONST_DECIMAL64 DEFAULT */
 
 static void
-handle_pragma_float_const_decimal64 (cpp_reader *ARG_UNUSED (dummy))
+handle_pragma_float_const_decimal64 (cpp_reader *ARG_UNUSED (dummy), 
+                                     void * ARG_UNUSED (data))
 {
   if (c_dialect_cxx ())
     {
@@ -1148,12 +1156,12 @@  static void
 }
 
 /* A vector of registered pragma callbacks.  */
+/*This is never freed as we need it during the whole execution */
+DEF_VEC_O (internal_pragma_handler);
+DEF_VEC_ALLOC_O (internal_pragma_handler, heap);
 
-DEF_VEC_O (pragma_handler);
-DEF_VEC_ALLOC_O (pragma_handler, heap);
+static VEC(internal_pragma_handler, heap) *registered_pragmas;
 
-static VEC(pragma_handler, heap) *registered_pragmas;
-
 typedef struct
 {
   const char *space;
@@ -1216,9 +1224,10 @@  c_pp_lookup_pragma (unsigned int id, const char **
 
 static void
 c_register_pragma_1 (const char *space, const char *name,
-		     pragma_handler handler, bool allow_expansion)
+		     pragma_handler handler, bool allow_expansion, void * data)
 {
   unsigned id;
+  internal_pragma_handler * ihandler;
 
   if (flag_preprocess_only)
     {
@@ -1229,14 +1238,18 @@  c_register_pragma_1 (const char *space, const char
 
       ns_name.space = space;
       ns_name.name = name;
+      
       VEC_safe_push (pragma_ns_name, heap, registered_pp_pragmas, &ns_name);
       id = VEC_length (pragma_ns_name, registered_pp_pragmas);
       id += PRAGMA_FIRST_EXTERNAL - 1;
     }
   else
     {
-      VEC_safe_push (pragma_handler, heap, registered_pragmas, &handler);
-      id = VEC_length (pragma_handler, registered_pragmas);
+      ihandler->handler = handler;
+      ihandler->data = data;
+      VEC_safe_push (internal_pragma_handler, heap, registered_pragmas,
+                                                    &ihandler);
+      id = VEC_length (internal_pragma_handler, registered_pragmas);
       id += PRAGMA_FIRST_EXTERNAL - 1;
 
       /* The C++ front end allocates 6 bits in cp_token; the C front end
@@ -1249,27 +1262,28 @@  c_register_pragma_1 (const char *space, const char
 }
 
 void
-c_register_pragma (const char *space, const char *name, pragma_handler handler)
+c_register_pragma (const char *space, const char *name, pragma_handler handler, 
+                   void * data)
 {
-  c_register_pragma_1 (space, name, handler, false);
+  c_register_pragma_1 (space, name, handler, false, data);
 }
 
 void
 c_register_pragma_with_expansion (const char *space, const char *name,
-				  pragma_handler handler)
+				  pragma_handler handler, void * data)
 {
-  c_register_pragma_1 (space, name, handler, true);
+  c_register_pragma_1 (space, name, handler, true, data);
 }
 
 void
 c_invoke_pragma_handler (unsigned int id)
 {
-  pragma_handler handler;
+  internal_pragma_handler * ihandler;
 
   id -= PRAGMA_FIRST_EXTERNAL;
-  handler = *VEC_index (pragma_handler, registered_pragmas, id);
-
-  handler (parse_in);
+  ihandler = VEC_index (internal_pragma_handler, registered_pragmas, id);
+  pragma_handler handler = ihandler->handler;
+  handler (parse_in, ihandler->data);
 }
 
 /* Set up front-end pragmas.  */
@@ -1291,27 +1305,28 @@  init_pragma (void)
 				  PRAGMA_GCC_PCH_PREPROCESS, false, false);
 
 #ifdef HANDLE_PRAGMA_PACK_WITH_EXPANSION
-  c_register_pragma_with_expansion (0, "pack", handle_pragma_pack);
+  c_register_pragma_with_expansion (0, "pack", handle_pragma_pack, NULL);
 #else
-  c_register_pragma (0, "pack", handle_pragma_pack);
+  c_register_pragma (0, "pack", handle_pragma_pack, NULL);
 #endif
-  c_register_pragma (0, "weak", handle_pragma_weak);
-  c_register_pragma ("GCC", "visibility", handle_pragma_visibility);
+  c_register_pragma (0, "weak", handle_pragma_weak, NULL);
+  c_register_pragma ("GCC", "visibility", handle_pragma_visibility, NULL);
 
-  c_register_pragma ("GCC", "diagnostic", handle_pragma_diagnostic);
-  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);
-  c_register_pragma ("GCC", "pop_options", handle_pragma_pop_options);
-  c_register_pragma ("GCC", "reset_options", handle_pragma_reset_options);
+  c_register_pragma ("GCC", "diagnostic", handle_pragma_diagnostic, NULL);
+  c_register_pragma ("GCC", "target", handle_pragma_target, NULL);
+  c_register_pragma ("GCC", "optimize", handle_pragma_optimize, NULL);
+  c_register_pragma ("GCC", "push_options", handle_pragma_push_options, NULL);
+  c_register_pragma ("GCC", "pop_options", handle_pragma_pop_options, NULL);
+  c_register_pragma ("GCC", "reset_options", handle_pragma_reset_options, NULL);
 
   c_register_pragma ("STDC", "FLOAT_CONST_DECIMAL64",
-		     handle_pragma_float_const_decimal64);
+		     handle_pragma_float_const_decimal64, NULL);
 
-  c_register_pragma_with_expansion (0, "redefine_extname", handle_pragma_redefine_extname);
-  c_register_pragma (0, "extern_prefix", handle_pragma_extern_prefix);
+  c_register_pragma_with_expansion (0, "redefine_extname",
+                                    handle_pragma_redefine_extname, NULL);
+  c_register_pragma (0, "extern_prefix", handle_pragma_extern_prefix, NULL);
 
-  c_register_pragma_with_expansion (0, "message", handle_pragma_message);
+  c_register_pragma_with_expansion (0, "message", handle_pragma_message, NULL);
 
 #ifdef REGISTER_TARGET_PRAGMAS
   REGISTER_TARGET_PRAGMAS ();