diff mbox

[testsuite] fix ggcplug.c test-case

Message ID CAAgBjMmqAX-5BpUbH9fzWiUU2DO3_hTKSeOrgJA_MerDR8HOKQ@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Jan. 11, 2015, 5:49 a.m. UTC
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.
OK to commit ?

Thank you,
Prathamesh
2015-01-11  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

testsuite/
	* gcc.dg/plugin/ggcplug.c: Include gcc-plugin.h before other headers.

Comments

Prathamesh Kulkarni Jan. 12, 2015, 9:09 a.m. UTC | #1
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.
Prathamesh Kulkarni Jan. 12, 2015, 10:20 a.m. UTC | #2
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)
Prathamesh Kulkarni Jan. 12, 2015, 10:50 a.m. UTC | #3
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.
diff mbox

Patch

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"