This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [plugins] Add user attribute(try #2)


Le-Chun Wu wrote:
Taras,

On Wed, Apr 1, 2009 at 9:39 AM, Taras Glek <tglek@mozilla.com> wrote:
Joseph S. Myers wrote:
On Wed, 1 Apr 2009, Richard Guenther wrote:


On Tue, Mar 31, 2009 at 8:17 PM, Taras Glek <tglek@mozilla.com> wrote:

Hi,
This patch adds a user("annotation") attribute to gcc for use with gcc
plugins that require code to be annotated.

Thanks,
Taras

Hm.  I wonder if it makes more sense to have a plugin hook for attribute
processing?

On the whole I agree that we should let plugins register new attributes
(whose table entries would get processed in init_attributes) rather than
adding one attribute all plugins have to go through.  These attributes would
then have all the flexibility of built-in attributes with regard to the
different ways decl_attributes can process them.  If the plugin isn't
loaded, they'd get a warning as for unknown attributes at present.

On the other hand, with one plugin-specific attribute it's immediately clear
to the user that the attribute is for some sort of an analysis is not
intended for gcc itself. Attributes are typically hidden behind macros, so
it doesn't matter what the attribute is named, so the end user wouldn't
really see any benefit.
I agree with Diego that it's nice to have GCC ignore user-attributes when no
plugin is loaded.

I can make a new patch for registering attributes, but that seems like
over-engineering to me.


Given that an unknown attribute will not cause a fatal syntax error, I think (and agree with Richard and Joseph) that allowing plugins to register new attributes is more flexible and useful than providing only a 'user' attribute. One shortcoming of the 'user' attribute I can think of is that there is no easy way for a plugin attribute to take arguments. Based on your current design, if a new plugin attribute 'new_attr' requires two arguments 'arg1' and 'arg2' (as shown below), the plugin will most likely need to parse the string to get the attribute name and the arguments,

__attribute__((user("new_attr(arg1, arg2)")))

while if we provide hooks to allow plugins to register new attributes
so that the above attribute will become

__attribute__((new_attr(arg1, arg2)))

then individual plugin doesn't have to parse the arguments and we have
all the power of the existing attribute processing code (for example,
checking the required number of arguments).

Anyway, providing attribute support for plugins is, in my opinion,
important and thanks a lot for working on it.
This patch lets plugins register attributes dynamically.

Taras
diff --git a/gcc/attribs.c b/gcc/attribs.c
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3.  
 #include "langhooks.h"
 #include "hashtab.h"
 #include "c-common.h"
+#include "plugin.h"
 
 static void init_attributes (void);
 
@@ -183,18 +184,25 @@ init_attributes (void)
   for (i = 0; i < ARRAY_SIZE (attribute_tables); i++)
     for (k = 0; attribute_tables[i][k].name != NULL; k++)
       {
+        register_attribute (&attribute_tables[i][k]);
+      }
+  invoke_plugin_callbacks (PLUGIN_ATTRIBUTES, NULL);
+  attributes_initialized = true;
+}
+
+void
+register_attribute (const struct attribute_spec *attr) 
+{
 	struct substring str;
 	const void **slot;
 
-	str.str = attribute_tables[i][k].name;
-	str.length = strlen (attribute_tables[i][k].name);
+	str.str = attr->name;
+	str.length = strlen (str.str);
 	slot = (const void **)htab_find_slot_with_hash (attribute_hash, &str,
 					 substring_hash (str.str, str.length),
 					 INSERT);
 	gcc_assert (!*slot);
-	*slot = &attribute_tables[i][k];
-      }
-  attributes_initialized = true;
+	*slot = attr;
 }
 
 /* Return the spec for the attribute named NAME.  */
diff --git a/gcc/gcc-plugin.h b/gcc/gcc-plugin.h
--- a/gcc/gcc-plugin.h
+++ b/gcc/gcc-plugin.h
@@ -29,6 +29,7 @@ enum plugin_event
   PLUGIN_CXX_CP_PRE_GENERICIZE, /* Allows to see low level AST in C++ FE.  */
   PLUGIN_FINISH,                /* Called before GCC exits.  */
   PLUGIN_INFO,                  /* Information about the plugin */
+  PLUGIN_ATTRIBUTES,            /* Called during attribute registration */
   PLUGIN_EVENT_LAST             /* Dummy event used for indexing callback
                                    array.  */
 };
diff --git a/gcc/plugin.c b/gcc/plugin.c
--- a/gcc/plugin.c
+++ b/gcc/plugin.c
@@ -491,6 +491,7 @@ register_callback (const char *plugin_na
       case PLUGIN_FINISH_TYPE:
       case PLUGIN_FINISH_UNIT:
       case PLUGIN_CXX_CP_PRE_GENERICIZE:
+      case PLUGIN_ATTRIBUTES:
       case PLUGIN_FINISH:
         {
           struct callback_info *new_callback;
@@ -532,6 +533,7 @@ invoke_plugin_callbacks (enum plugin_eve
       case PLUGIN_FINISH_TYPE:
       case PLUGIN_FINISH_UNIT:
       case PLUGIN_CXX_CP_PRE_GENERICIZE:
+      case PLUGIN_ATTRIBUTES:
       case PLUGIN_FINISH:
         {
           /* Iterate over every callback registered with this event and
diff --git a/gcc/plugin.h b/gcc/plugin.h
--- a/gcc/plugin.h
+++ b/gcc/plugin.h
@@ -22,6 +22,8 @@ along with GCC; see the file COPYING3.  
 
 #include "gcc-plugin.h"
 
+struct attribute_spec;
+
 extern void add_new_plugin (const char *);
 extern void parse_plugin_arg_opt (const char *);
 extern void invoke_plugin_callbacks (enum plugin_event, void *);
@@ -32,5 +34,6 @@ extern void print_plugins_versions (FILE
 extern void print_plugins_versions (FILE *file, const char *indent);
 extern void print_plugins_help (FILE *file, const char *indent);
 extern void finalize_plugins (void);
+extern void register_attribute (const struct attribute_spec *attr);
 
 #endif /* PLUGIN_H */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]