This is the mail archive of the 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: [patch] plugin directory / common location for plugins

On 06.04.2010 03:45, Le-Chun Wu wrote:
Sorry for being late in the game. Here are my comments and thoughts:

While some of us don't really feel the need to create a new GCC policy for a
default plugin location, I actually agree with Basile and Matthias that
having such a default location surely will make it easier for the plugin
writers to ship their plugins and for plugin users to use them. So I would
support this change and approve the patch with the following comments:

--- a/src/gcc/testsuite/gcc.dg/plugin/plugin.exp (revision
+++ b/src/gcc/testsuite/gcc.dg/plugin/plugin.exp (working
@@ -51,6 +51,7 @@
     { ggcplug.c ggcplug-test-1.c } \
      { one_time_plugin.c one_time-test-1.c } \
      { start_unit_plugin.c start_unit-test-1.c } \
+    { plugdir_plugin.c plugdir-test-1.c } \

It's not clear to me what the purpose of this test is. It's apparently not testing the short plugin name (and default location) support as the plugin is still being called in the standard way. It seems to test the function "plugin_directory_name", but you don't appear to check for the correctness of the returned name. I think better test cases should be created/added.

removed, and added four new patches checking the passing of the options and the expansion in cc1.

+++ b/src/gcc/gcc-plugin.h (working
@@ -141,4 +141,10 @@

extern int unregister_callback (const char *plugin_name, int event);

+/* Retrieve the plugin directory name, as returned by the
+   -fprint-file-name=plugin argument to the gcc program, which is the
+   -iplugindir program argument to cc1.  */
+extern const char* plugin_directory_name (void);

I would change the name of this function to something like "default_plugin_dir_name" to make it clearer what it does.


+++ b/src/gcc/common.opt (working
@@ -1535,6 +1535,11 @@
  Common JoinedOrMissing Negative(gcoff)
  Generate debug information in extended XCOFF format

+Common Joined Separate Var(plugindir_string)
+-iplugindir<dir>  Set<dir>  to be the plugin directory

Add an entry in doc/invoke.texi? Especially you want to mention the this flag is not meant for the end users.

I did set the Undocumented property instead (alternatively attached is another patch to document the option).

@@ -818,3 +844,12 @@
    return event_last;
+/* Retrieve the plugin directory.

s/Retrieve the plugin directory/Retrieve the default plugin directory


Given that this patch is actually a new feature (and in my opinion not
critical), I am not sure it's suitable for 4.5. (I will leave it to release
manager and others.)

Lets move it to the trunk first. I still would like to see this for 4.5, if possible. Same reason for the -fld=gold|ld.bfd to avoid handling with path names which differ on systems/installations.

Please find attached the updated patch.


Attachment: plugindir.diff
Description: Text document

Attachment: plugindir-doc.diff
Description: Text document

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