| Message ID | CAAgBjMmqAX-5BpUbH9fzWiUU2DO3_hTKSeOrgJA_MerDR8HOKQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 12 January 2015 at 14:19, Richard Biener <rguenther@suse.de> wrote: > On Sun, 11 Jan 2015, Prathamesh Kulkarni wrote: > >> Hi, >> The test-case plugin/ggcplug.c was failing due to flattening of tree.h >> and tree-core.h. >> Test-case was incorrect because it included gcc-plugin.h after tree.h whereas >> gcc-plugin.h should be the first header to be included by plugins. > > No, it should be definitely included _after_ config.h, system.h > and coretypes.h. gcc-plugin.h already includes these files. Shall I remove config.h, system.h and coretypes.h from ggcplug.c instead ? > > Ok with moving it after coretypes.h. > > Thanks, > Richard.
On 12 January 2015 at 14:36, Richard Biener <rguenther@suse.de> wrote: > On Mon, 12 Jan 2015, Prathamesh Kulkarni wrote: > >> On 12 January 2015 at 14:19, Richard Biener <rguenther@suse.de> wrote: >> > On Sun, 11 Jan 2015, Prathamesh Kulkarni wrote: >> > >> >> Hi, >> >> The test-case plugin/ggcplug.c was failing due to flattening of tree.h >> >> and tree-core.h. >> >> Test-case was incorrect because it included gcc-plugin.h after tree.h whereas >> >> gcc-plugin.h should be the first header to be included by plugins. >> > >> > No, it should be definitely included _after_ config.h, system.h >> > and coretypes.h. >> gcc-plugin.h already includes these files. Shall I remove config.h, >> system.h and coretypes.h >> from ggcplug.c instead ? > > No, keep the patch simple for now - we are inconsitent in all the > testsuite plugins it seems and wasn't the idea that plugins _only_ > need to include gcc-plugin.h now? Thus I'd rather cleanup all > plugin testcases at once, with a separate patch. I thought gcc-plugin.h would contain include dependencies of all headers (to make plugins transparent to include restructuring) and if a plugin needs a particular header, it should explicitly include it. Or am I missing something ? > > Thanks, > Richard. > >> > >> > Ok with moving it after coretypes.h. Shall I commit the patch after this change since this is the only plugin test case that's failing ? Thanks, Prathamesh >> > >> > Thanks, >> > Richard. >> >> > > -- > Richard Biener <rguenther@suse.de> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild, > Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)
On 12 January 2015 at 15:49, Richard Biener <rguenther@suse.de> wrote: > On Mon, 12 Jan 2015, Prathamesh Kulkarni wrote: > >> On 12 January 2015 at 14:36, Richard Biener <rguenther@suse.de> wrote: >> > On Mon, 12 Jan 2015, Prathamesh Kulkarni wrote: >> > >> >> On 12 January 2015 at 14:19, Richard Biener <rguenther@suse.de> wrote: >> >> > On Sun, 11 Jan 2015, Prathamesh Kulkarni wrote: >> >> > >> >> >> Hi, >> >> >> The test-case plugin/ggcplug.c was failing due to flattening of tree.h >> >> >> and tree-core.h. >> >> >> Test-case was incorrect because it included gcc-plugin.h after tree.h whereas >> >> >> gcc-plugin.h should be the first header to be included by plugins. >> >> > >> >> > No, it should be definitely included _after_ config.h, system.h >> >> > and coretypes.h. >> >> gcc-plugin.h already includes these files. Shall I remove config.h, >> >> system.h and coretypes.h >> >> from ggcplug.c instead ? >> > >> > No, keep the patch simple for now - we are inconsitent in all the >> > testsuite plugins it seems and wasn't the idea that plugins _only_ >> > need to include gcc-plugin.h now? Thus I'd rather cleanup all >> > plugin testcases at once, with a separate patch. >> I thought gcc-plugin.h would contain include dependencies of all >> headers (to make plugins transparent >> to include restructuring) and if a plugin needs a particular header, >> it should explicitly include it. Or am I >> missing something ? > > No idea - I thought the idea was that plugins only ever need to > include gcc-plugin.h which will include everything (aka the "world") > so plugins are immune to things moving between headers (another > thing that happened a lot for GCC 5). > >> > >> > Thanks, >> > Richard. >> > >> >> > >> >> > Ok with moving it after coretypes.h. >> Shall I commit the patch after this change since this is the only >> plugin test case that's failing ? > > You should commit a patch moving the gcc-plugin.h include in ggcplug.c > to after the include of coretypes.h. Moved gcc-plugin.h include after coretypes.h and committed as r219458. Thanks, Prathamesh > > Thanks, > Richard.
Index: gcc/testsuite/gcc.dg/plugin/ggcplug.c =================================================================== --- gcc/testsuite/gcc.dg/plugin/ggcplug.c (revision 219429) +++ gcc/testsuite/gcc.dg/plugin/ggcplug.c (working copy) @@ -1,12 +1,12 @@ /* This plugin tests the GGC related plugin events. */ /* { dg-options "-O" } */ +#include "gcc-plugin.h" #include "config.h" #include "system.h" #include "coretypes.h" #include "tm.h" #include "tree.h" -#include "gcc-plugin.h" #include "toplev.h" #include "basic-block.h" #include "hash-table.h"