Patchwork do not include diagnostic-core.h in toplev.h

login
register
mail settings
Submitter Manuel López-Ibáñez
Date July 8, 2010, 2:02 p.m.
Message ID <AANLkTinSHIzrg5m-BOvKmRe9IAVE5Sq_jxpr2pGzwTAM@mail.gmail.com>
Download mbox | patch
Permalink /patch/58250/
State New
Headers show

Comments

Manuel López-Ibáñez - July 8, 2010, 2:02 p.m.
On 8 July 2010 13:30, Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> wrote:
>
> On Thu, 2010-07-08 at 06:23 +0200, Manuel López-Ibáñez wrote:
>> On 7 July 2010 21:25, Diego Novillo <dnovillo@google.com> wrote:
>> > On Tue, Jul 6, 2010 at 05:49, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
>> >
>> >> 2010-07-06  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>> >>
>> >>        * toplev.h: Do not include diagnostic-core.h.
>> >>        Include diagnostic-core.h in every file that includes toplev.h.
>> >>        * c-tree.h: Do not include toplev.h.
>> >>
>> >> c-family/
>> >>        * c-common.h: Include diagnostic-core.h. Error if already
>> >>        included.
>> >>        * c-semantics.c: Do not define GCC_DIAG_STYLE here.
>> >> cp/
>> >>        * cp-tree.h: Do not include toplev.h.
>> >>
>> >> java/
>> >>        * Include diagnostic-core.h in every file that includes toplev.h.
>> >> ada/
>> >>        * Include diagnostic-core.h in every file that includes toplev.h.
>> >> fortran/
>> >>        * Include diagnostic-core.h in every file that includes toplev.h.
>> >> lto/
>> >>        * Include diagnostic-core.h in every file that includes toplev.h.
>> >
>> > OK, though the CL should mention the affected files.
>>
>> Done. Committed as revision 161943.
>
>
> Sorry - didn't notice this before but won't this break bootstrap on
> every platform that uses error or any function in diagnostic-core.h in
> the backend machine description.

Sorry I read your email after the commit. TBH, I didn't know about
this. The trivial fix is to add diagnostic-core.h to every gen*c file.
A better fix would be to check which gen*c files do actually need
this. If someone knows/wants to check the latter, feel free to take
this. Otherwise, the patch below should suffice (testing now).

2010-07-08  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	* genrecog.c: Include diagnostic-core.h before toplev.h.
	* genoutput.c: Likewise.
	* genextract.c: Likewise.
	* genautomata.c: Likewise.
	* genemit.c: Likewise.
	* genpeep.c: Likewise.
	* genattrtab.c: Likewise.
	* genconditions.c: Likewise.
	* genpreds.c: Likewise.
Mark Mitchell - July 8, 2010, 10:04 p.m.
Manuel López-Ibáñez wrote:

> 2010-07-08  Manuel López-Ibáñez  <manu@gcc.gnu.org>
> 
> 	* genrecog.c: Include diagnostic-core.h before toplev.h.
> 	* genoutput.c: Likewise.
> 	* genextract.c: Likewise.
> 	* genautomata.c: Likewise.
> 	* genemit.c: Likewise.
> 	* genpeep.c: Likewise.
> 	* genattrtab.c: Likewise.
> 	* genconditions.c: Likewise.
> 	* genpreds.c: Likewise.

Go ahead and put this in.  It may be overkill, but it's not *wrong* per
se to declare things we don't need.

Thanks,
Manuel López-Ibáñez - July 8, 2010, 11:36 p.m.
On 9 July 2010 00:04, Mark Mitchell <mark@codesourcery.com> wrote:
> Manuel López-Ibáñez wrote:
>
>> 2010-07-08  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>>
>>       * genrecog.c: Include diagnostic-core.h before toplev.h.
>>       * genoutput.c: Likewise.
>>       * genextract.c: Likewise.
>>       * genautomata.c: Likewise.
>>       * genemit.c: Likewise.
>>       * genpeep.c: Likewise.
>>       * genattrtab.c: Likewise.
>>       * genconditions.c: Likewise.
>>       * genpreds.c: Likewise.
>
> Go ahead and put this in.  It may be overkill, but it's not *wrong* per
> se to declare things we don't need.

Perhaps it would be more robust for the future to have a header for
each gen* program, so they only include 1 header. This header will
contain the actual #includes, so it will be far easier to check what
is included by what. Modifying the headers will not require to modify
and recompile the gen* programs (plus regenerate and recompile the
generated file) but just recompile the generated file. What you think?

Manuel.
Mark Mitchell - July 8, 2010, 11:40 p.m.
Manuel López-Ibáñez wrote:

> Perhaps it would be more robust for the future to have a header for
> each gen* program, so they only include 1 header. This header will
> contain the actual #includes, so it will be far easier to check what
> is included by what. Modifying the headers will not require to modify
> and recompile the gen* programs (plus regenerate and recompile the
> generated file) but just recompile the generated file. What you think?

I don't think that's going to be a big win; it just moves the #includes
around.  In any case, I'm not overly worried about this kind of
fine-grained detail; I think it's much more important to continue the
overall modularization of the compiler.

Patch

Index: genrecog.c
===================================================================
--- genrecog.c	(revision 161942)
+++ genrecog.c	(working copy)
@@ -2448,6 +2448,7 @@ 
 #include \"flags.h\"\n\
 #include \"hard-reg-set.h\"\n\
 #include \"resource.h\"\n\
+#include \"diagnostic-core.h\"\n\
 #include \"toplev.h\"\n\
 #include \"reload.h\"\n\
 #include \"regs.h\"\n\
Index: genoutput.c
===================================================================
--- genoutput.c	(revision 161942)
+++ genoutput.c	(working copy)
@@ -240,6 +240,7 @@ 
   printf ("#include \"conditions.h\"\n");
   printf ("#include \"insn-attr.h\"\n\n");
   printf ("#include \"recog.h\"\n\n");
+  printf ("#include \"diagnostic-core.h\"\n");
   printf ("#include \"toplev.h\"\n");
   printf ("#include \"output.h\"\n");
   printf ("#include \"target.h\"\n");
Index: genextract.c
===================================================================
--- genextract.c	(revision 161942)
+++ genextract.c	(working copy)
@@ -365,6 +365,7 @@ 
 #include \"rtl.h\"\n\
 #include \"insn-config.h\"\n\
 #include \"recog.h\"\n\
+#include \"diagnostic-core.h\"\n\
 #include \"toplev.h\"\n\
 \n\
 /* This variable is used as the \"location\" of any missing operand\n\
Index: genautomata.c
===================================================================
--- genautomata.c	(revision 161942)
+++ genautomata.c	(working copy)
@@ -9559,7 +9559,7 @@ 
 		"#include \"regs.h\"\n"
 		"#include \"output.h\"\n"
 		"#include \"insn-attr.h\"\n"
-		"#include \"toplev.h\"\n"
+                "#include \"diagnostic-core.h\"\n"
 		"#include \"flags.h\"\n"
 		"#include \"function.h\"\n"
 		"#include \"emit-rtl.h\"\n");
Index: genemit.c
===================================================================
--- genemit.c	(revision 161942)
+++ genemit.c	(working copy)
@@ -862,6 +862,7 @@ 
   printf ("#include \"recog.h\"\n");
   printf ("#include \"resource.h\"\n");
   printf ("#include \"reload.h\"\n");
+  printf ("#include \"diagnostic-core.h\"\n");
   printf ("#include \"toplev.h\"\n");
   printf ("#include \"regs.h\"\n");
   printf ("#include \"tm-constrs.h\"\n");
Index: genpeep.c
===================================================================
--- genpeep.c	(revision 161942)
+++ genpeep.c	(working copy)
@@ -376,6 +376,7 @@ 
   printf ("#include \"recog.h\"\n");
   printf ("#include \"except.h\"\n");
   printf ("#include \"function.h\"\n");
+  printf ("#include \"diagnostic-core.h\"\n");
   printf ("#include \"toplev.h\"\n");
   printf ("#include \"flags.h\"\n");
   printf ("#include \"tm-constrs.h\"\n\n");
Index: genattrtab.c
===================================================================
--- genattrtab.c	(revision 161942)
+++ genattrtab.c	(working copy)
@@ -4920,7 +4920,8 @@ 
   printf ("#include \"recog.h\"\n");
   printf ("#include \"regs.h\"\n");
   printf ("#include \"output.h\"\n");
-  printf ("#include \"toplev.h\"\n");
+  printf ("#include \"diagnostic-core.h\"\n"
+          "#include \"toplev.h\"\n");
   printf ("#include \"flags.h\"\n");
   printf ("#include \"function.h\"\n");
   printf ("\n");
Index: genconditions.c
===================================================================
--- genconditions.c	(revision 161942)
+++ genconditions.c	(working copy)
@@ -86,6 +86,7 @@ 
 #include \"flags.h\"\n\
 #include \"hard-reg-set.h\"\n\
 #include \"resource.h\"\n\
+#include \"diagnostic-core.h\"\n\
 #include \"toplev.h\"\n\
 #include \"reload.h\"\n\
 #include \"tm-constrs.h\"\n");
Index: genpreds.c
===================================================================
--- genpreds.c	(revision 161942)
+++ genpreds.c	(working copy)
@@ -1337,6 +1337,7 @@ 
 #include \"flags.h\"\n\
 #include \"hard-reg-set.h\"\n\
 #include \"resource.h\"\n\
+#include \"diagnostic-core.h\"\n\
 #include \"toplev.h\"\n\
 #include \"reload.h\"\n\
 #include \"regs.h\"\n\