diff mbox

[objc++] RFH: PR 48167 gengtype failure (issue4291054)

Message ID 1300471187.399111124@192.168.4.58
State New
Headers show

Commit Message

Nicola Pero March 18, 2011, 5:59 p.m. UTC 
> The Obj-C++ FE is kind of weird as it shares files from cp/ and objc/,
> so I'm missing some other connection I need to make to fix this.
>
> Any ideas?

As far as I can see, the problem is that header files (such as cp/parser.h)
generate GC stuff that gets put into gtype-{lang}.h, where {lang} is determined
by looking at the directory where the file is (eg, if it's in the cp/ directory,
then {lang} is cp) ... unless you override this with a custom hardcoded rule in
gengtype.c.

It seems that all files that are shared between ObjC or C++ and ObjC++ have custom
hardcoded rules in gengtype.c.  The cp/parser.h file you added didn't, and without
it, every time it is processed, the results are stored in gtype-cp.h even if the
file is processed for ObjC++. :-(

This all looks ugly.  Anyhow, until someone refactors everything, the following
patch fixes ObjC++ bootstrap in trunk by processing cp/parser.h in the same way
as the other C++ headers are processed. :-)

Ok to commit ?

Thanks
Index: gengtype.c
===================================================================
--- gengtype.c	(revision 171138)
+++ gengtype.c	(working copy)
@@ -1761,6 +1761,12 @@
    matters, so change with extreme care!  */
 
 struct file_rule_st files_rules[] = {
+  /* The general rule assumes that files in subdirectories belong to a
+     special front-end, and files not in subdirectories are shared.
+     The following rules deal with exceptions - files that are in
+     subdirectories and yet are shared, and files that are top-level,
+     but are not shared.  */
+
   /* the c-family/ source directory is special.  */
   { DIR_PREFIX_REGEX "c-family/([[:alnum:]_-]*)\\.c$",
     REG_EXTENDED, NULL_REGEX,
@@ -1792,7 +1798,12 @@
     REG_EXTENDED, NULL_REGEX,
     "gt-cp-name-lookup.h", "cp/name-lookup.c", NULL_FRULACT },
 
-  /* objc/objc-act.h fives gt-objc-objc-act.h for objc/objc-act.c !  */
+  /* cp/parser.h gives gt-cp-parser.h for cp/parser.c !  */
+  { DIR_PREFIX_REGEX "cp/parser\\.h$",
+    REG_EXTENDED, NULL_REGEX,
+    "gt-cp-parser.h", "cp/parser.c", NULL_FRULACT },
+
+  /* objc/objc-act.h gives gt-objc-objc-act.h for objc/objc-act.c !  */
   { DIR_PREFIX_REGEX "objc/objc-act\\.h$",
     REG_EXTENDED, NULL_REGEX,
     "gt-objc-objc-act.h", "objc/objc-act.c", NULL_FRULACT },
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 171138)
+++ ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2011-03-18  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+	* gengtype.c (files_rules): Added rule for cp/parser.h.
+
 2011-03-18  Chung-Lin Tang  <cltang@codesourcery.com>
 
 	* combine.c (try_combine): Do simplification only call of
Index: objcp/Make-lang.in
===================================================================
--- objcp/Make-lang.in	(revision 171138)
+++ objcp/Make-lang.in	(working copy)
@@ -45,7 +45,7 @@
 .PHONY: obj-c++
 
 START_HDRS = $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) $(CXX_TREE_H) \
-  langhooks.h c-family/c-objc.h objc/objc-act.h
+  $(CXX_PARSER_H) $(CXX_PRETTY_PRINT_H) langhooks.h c-family/c-objc.h objc/objc-act.h
 
 # Use maximal warnings for this front end.  Also, make ObjC and C++
 # headers accessible.
@@ -78,7 +78,7 @@
 
 objcp/objcp-lang.o : objcp/objcp-lang.c $(START_HDRS) \
   $(C_COMMON_H) $(LANGHOOKS_DEF_H) cp/cp-objcp-common.h \
-  $(TARGET_H) gtype-objcp.h 
+  $(TARGET_H) gtype-objcp.h
 
 objcp/objcp-decl.o : objcp/objcp-decl.c \
    $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) $(CXX_TREE_H) \
Index: objcp/ChangeLog
===================================================================
--- objcp/ChangeLog	(revision 171138)
+++ objcp/ChangeLog	(working copy)
@@ -1,3 +1,11 @@
+2011-03-17  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+	* Make-lang.in (START_HDRS): Added CXX_PARSER_H and
+	CXX_PRETTY_PRINT_H.
+	* config-lang.in (gtfiles): Added cp/parser.h and reorganized list
+	so that it is more obvious that it is identical to the C++ one
+	with the addition of some files at the end.
+	
 2011-03-06  Joseph Myers  <joseph@codesourcery.com>
 
 	* lang-specs.h: Match -save-temps* instead of -save-temps.
Index: objcp/config-lang.in
===================================================================
--- objcp/config-lang.in	(revision 171138)
+++ objcp/config-lang.in	(working copy)
@@ -37,5 +37,14 @@
 lang_requires="objc c++"
 subdir_requires="objc cp"
 
-gtfiles="\$(srcdir)/objc/objc-act.h \$(srcdir)/objc/objc-act.c \$(srcdir)/objc/objc-runtime-shared-support.c \$(srcdir)/objc/objc-gnu-runtime-abi-01.c \$(srcdir)/objc/objc-next-runtime-abi-01.c \$(srcdir)/objc/objc-next-runtime-abi-02.c \$(srcdir)/cp/call.c \$(srcdir)/cp/class.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/mangle.c \$(srcdir)/cp/method.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-objcp-common.c \$(srcdir)/cp/parser.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/rtti.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-lex.c \$(srcdir)/c-family/c-cppbuiltin.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c "
+# When you add to this gtfiles list a header which comes from a
+# directory belonging to another language (ie, C++ or ObjC), you need
+# to also edit gengtype.c adding a special rule for the header to
+# avoid having the GC stuff from that header being added to gtype-cp.h
+# or gtype-objc.h.
 
+# This list is separated in two parts: the first one is identical to
+# the C++ one, the second one contains our ObjC++ additions.
+gtfiles="\$(srcdir)/cp/rtti.c \$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.h \$(srcdir)/cp/parser.c \$(srcdir)/cp/method.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-lex.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c \$(srcdir)/cp/class.c \$(srcdir)/cp/cp-objcp-common.c \
+\$(srcdir)/objc/objc-act.h \$(srcdir)/objc/objc-act.c \$(srcdir)/objc/objc-runtime-shared-support.c \$(srcdir)/objc/objc-gnu-runtime-abi-01.c \$(srcdir)/objc/objc-next-runtime-abi-01.c \$(srcdir)/objc/objc-next-runtime-abi-02.c \$(srcdir)/c-family/c-cppbuiltin.c"
+

Comments

Basile Starynkevitch March 18, 2011, 6:40 p.m. UTC | #1 
On Fri, 18 Mar 2011 18:59:47 +0100 (CET)
"Nicola Pero" <nicola.pero@meta-innovation.com> wrote:

> 
> > The Obj-C++ FE is kind of weird as it shares files from cp/ and objc/,
> > so I'm missing some other connection I need to make to fix this.
> 
> This all looks ugly.  Anyhow, until someone refactors everything, the following
> patch fixes ObjC++ bootstrap in trunk by processing cp/parser.h in the same way
> as the other C++ headers are processed. :-)
> 
> Ok to commit ?


I don't have authority to Ok, but I find your patch nice (and I believe
that it illustrates the fact that adding some regexp rules to gengtype
was a good idea).

Regards!
Diego Novillo March 18, 2011, 7:50 p.m. UTC | #2 
On Fri, Mar 18, 2011 at 13:59, Nicola Pero
<nicola.pero@meta-innovation.com> wrote:
>
>> The Obj-C++ FE is kind of weird as it shares files from cp/ and objc/,
>> so I'm missing some other connection I need to make to fix this.
>>
>> Any ideas?
>
> As far as I can see, the problem is that header files (such as cp/parser.h)
> generate GC stuff that gets put into gtype-{lang}.h, where {lang} is determined
> by looking at the directory where the file is (eg, if it's in the cp/ directory,
> then {lang} is cp) ... unless you override this with a custom hardcoded rule in
> gengtype.c.

Oh!  That's what I was missing.  Thanks for the fix.

>
> It seems that all files that are shared between ObjC or C++ and ObjC++ have custom
> hardcoded rules in gengtype.c.  The cp/parser.h file you added didn't, and without
> it, every time it is processed, the results are stored in gtype-cp.h even if the
> file is processed for ObjC++. :-(

Ugh.

> This all looks ugly.  Anyhow, until someone refactors everything, the following
> patch fixes ObjC++ bootstrap in trunk by processing cp/parser.h in the same way
> as the other C++ headers are processed. :-)
>
> Ok to commit ?
>
> Thanks
>
> Index: gcc/gengtype.c
> ===================================================================
> --- gcc/gengtype.c      (revision 171155)
> +++ gcc/gengtype.c      (working copy)
> @@ -1761,6 +1761,12 @@
>    matters, so change with extreme care!  */
>
>  struct file_rule_st files_rules[] = {
> +  /* The general rule assumes that files in subdirectories belong to a
> +     particular front-end, and files not in subdirectories are shared.
> +     The following rules deal with exceptions - files that are in
> +     subdirectories and yet are shared, and files that are top-level,
> +     but are not shared.  */
> +
>   /* the c-family/ source directory is special.  */
>   { DIR_PREFIX_REGEX "c-family/([[:alnum:]_-]*)\\.c$",
>     REG_EXTENDED, NULL_REGEX,
> @@ -1792,7 +1798,12 @@
>     REG_EXTENDED, NULL_REGEX,
>     "gt-cp-name-lookup.h", "cp/name-lookup.c", NULL_FRULACT },
>
> -  /* objc/objc-act.h fives gt-objc-objc-act.h for objc/objc-act.c !  */
> +  /* cp/parser.h gives gt-cp-parser.h for cp/parser.c !  */
> +  { DIR_PREFIX_REGEX "cp/parser\\.h$",
> +    REG_EXTENDED, NULL_REGEX,
> +    "gt-cp-parser.h", "cp/parser.c", NULL_FRULACT },

But cp/parser.c also gets its own gt-cp-parser.h.  Won't they
conflict?  Doesn't this mean that cp/parser.h should be renamed to
something else?  (say cp/cp-parser.h).

Other than that, it looks fine.


Diego.
Nicola Pero March 18, 2011, 8:17 p.m. UTC | #3 
>> +  /* cp/parser.h gives gt-cp-parser.h for cp/parser.c !  */
>> +  { DIR_PREFIX_REGEX "cp/parser\\.h$",
>> +    REG_EXTENDED, NULL_REGEX,
>> +    "gt-cp-parser.h", "cp/parser.c", NULL_FRULACT },
>
> But cp/parser.c also gets its own gt-cp-parser.h.  Won't they
> conflict?  Doesn't this mean that cp/parser.h should be renamed to
> something else?  (say cp/cp-parser.h).

My understanding is that, with that rule, GC generated code from
cp-parser.h will end up into gt-cp-parser.h, together with the code
generated from cp-parser.c.  gengtype basically runs once for all
files, so has no problem putting GC generated code from multiple
input files into the same output file.

In practical terms, cp/decl.c and cp/decl.h (or cp/name-lookup.c
and cp/name-lookup.h) are in a similar situation, and work fine
with similar gengtype.c rules. :-)

Thanks
Diego Novillo March 18, 2011, 9:14 p.m. UTC | #4 
On Fri, Mar 18, 2011 at 16:17, Nicola Pero
<nicola.pero@meta-innovation.com> wrote:
>
>>> +  /* cp/parser.h gives gt-cp-parser.h for cp/parser.c !  */
>>> +  { DIR_PREFIX_REGEX "cp/parser\\.h$",
>>> +    REG_EXTENDED, NULL_REGEX,
>>> +    "gt-cp-parser.h", "cp/parser.c", NULL_FRULACT },
>>
>> But cp/parser.c also gets its own gt-cp-parser.h.  Won't they
>> conflict?  Doesn't this mean that cp/parser.h should be renamed to
>> something else?  (say cp/cp-parser.h).
>
> My understanding is that, with that rule, GC generated code from
> cp-parser.h will end up into gt-cp-parser.h, together with the code
> generated from cp-parser.c.  gengtype basically runs once for all
> files, so has no problem putting GC generated code from multiple
> input files into the same output file.

OK, thanks.

I'm OK with the patch, but please wait for a specific approval from
Laurynas.  I'm not sufficiently familiar with this code to give final
approval.


Diego.
Mike Stump March 19, 2011, 4:04 a.m. UTC | #5 
On Mar 18, 2011, at 10:59 AM, Nicola Pero wrote:
> This all looks ugly.  Anyhow, until someone refactors everything, the following
> patch fixes ObjC++ bootstrap in trunk by processing cp/parser.h in the same way
> as the other C++ headers are processed. :-)
> 
> Ok to commit ?

Ok for the Objective parts.  Thanks for the work.
Laurynas Biveinis March 21, 2011, 8:19 a.m. UTC | #6 
2011/3/18 Nicola Pero <nicola.pero@meta-innovation.com>:
> Index: gcc/ChangeLog
> ===================================================================
> --- gcc/ChangeLog       (revision 171155)
> +++ gcc/ChangeLog       (working copy)
> @@ -1,3 +1,7 @@
> +2011-03-18  Nicola Pero  <nicola.pero@meta-innovation.com>
> +
> +       * gengtype.c (files_rules): Added rule for cp/parser.h.
> +

The gengtype parts are OK. Thanks and sorry for the delay.
diff mbox

Patch

Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c      (revision 171155)
+++ gcc/gengtype.c      (working copy)
@@ -1761,6 +1761,12 @@ 
    matters, so change with extreme care!  */
 
 struct file_rule_st files_rules[] = {
+  /* The general rule assumes that files in subdirectories belong to a
+     particular front-end, and files not in subdirectories are shared.
+     The following rules deal with exceptions - files that are in
+     subdirectories and yet are shared, and files that are top-level,
+     but are not shared.  */
+
   /* the c-family/ source directory is special.  */
   { DIR_PREFIX_REGEX "c-family/([[:alnum:]_-]*)\\.c$",
     REG_EXTENDED, NULL_REGEX,
@@ -1792,7 +1798,12 @@ 
     REG_EXTENDED, NULL_REGEX,
     "gt-cp-name-lookup.h", "cp/name-lookup.c", NULL_FRULACT },
 
-  /* objc/objc-act.h fives gt-objc-objc-act.h for objc/objc-act.c !  */
+  /* cp/parser.h gives gt-cp-parser.h for cp/parser.c !  */
+  { DIR_PREFIX_REGEX "cp/parser\\.h$",
+    REG_EXTENDED, NULL_REGEX,
+    "gt-cp-parser.h", "cp/parser.c", NULL_FRULACT },
+
+  /* objc/objc-act.h gives gt-objc-objc-act.h for objc/objc-act.c !  */
   { DIR_PREFIX_REGEX "objc/objc-act\\.h$",
     REG_EXTENDED, NULL_REGEX,
     "gt-objc-objc-act.h", "objc/objc-act.c", NULL_FRULACT },
Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog       (revision 171155)
+++ gcc/ChangeLog       (working copy)
@@ -1,3 +1,7 @@ 
+2011-03-18  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * gengtype.c (files_rules): Added rule for cp/parser.h.
+
 2011-03-18  Maxim Kuvyrkov  <maxim@codesourcery.com>
 
        PR rtl-optimization/48170
Index: gcc/objcp/Make-lang.in
===================================================================
--- gcc/objcp/Make-lang.in      (revision 171155)
+++ gcc/objcp/Make-lang.in      (working copy)
@@ -45,7 +45,7 @@ 
 .PHONY: obj-c++
 
 START_HDRS = $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) $(CXX_TREE_H) \
-  langhooks.h c-family/c-objc.h objc/objc-act.h
+  $(CXX_PARSER_H) $(CXX_PRETTY_PRINT_H) langhooks.h c-family/c-objc.h objc/objc-act.h
 
 # Use maximal warnings for this front end.  Also, make ObjC and C++
 # headers accessible.
@@ -78,7 +78,7 @@ 
 
 objcp/objcp-lang.o : objcp/objcp-lang.c $(START_HDRS) \
   $(C_COMMON_H) $(LANGHOOKS_DEF_H) cp/cp-objcp-common.h \
-  $(TARGET_H) gtype-objcp.h 
+  $(TARGET_H) gtype-objcp.h
 
 objcp/objcp-decl.o : objcp/objcp-decl.c \
    $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) $(CXX_TREE_H) \
Index: gcc/objcp/ChangeLog
===================================================================
--- gcc/objcp/ChangeLog (revision 171155)
+++ gcc/objcp/ChangeLog (working copy)
@@ -1,3 +1,11 @@ 
+2011-03-17  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * Make-lang.in (START_HDRS): Added CXX_PARSER_H and
+       CXX_PRETTY_PRINT_H.
+       * config-lang.in (gtfiles): Added cp/parser.h and reorganized list
+       so that it is more obvious that it is identical to the C++ one
+       with the addition of some files at the end.
+       
 2011-03-06  Joseph Myers  <joseph@codesourcery.com>
 
        * lang-specs.h: Match -save-temps* instead of -save-temps.
Index: gcc/objcp/config-lang.in
===================================================================
--- gcc/objcp/config-lang.in    (revision 171155)
+++ gcc/objcp/config-lang.in    (working copy)
@@ -37,5 +37,14 @@ 
 lang_requires="objc c++"
 subdir_requires="objc cp"
 
-gtfiles="\$(srcdir)/objc/objc-act.h \$(srcdir)/objc/objc-act.c \$(srcdir)/objc/objc-runtime-shared-support.c \$(srcdir)/objc/objc-gnu-runtime-abi-01.c \$(srcdir)/objc/objc-next-runtime-abi-01.c \$(srcdir)/objc/objc-next-runtime-abi-02.c \$(srcdir)/cp/call.c \$(srcdir)/cp/class.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/mangle.c \$(srcdir)/cp/method.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-objcp-common.c \$(srcdir)/cp/parser.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/rtti.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-lex.c \$(srcdir)/c-family/c-cppbuiltin.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c "
+# When you add to this gtfiles list a header which comes from a
+# directory belonging to another language (ie, C++ or ObjC), you need
+# to also edit gengtype.c adding a special rule for the header to
+# avoid having the GC stuff from that header being added to gtype-cp.h
+# or gtype-objc.h.
 
+# This list is separated in two parts: the first one is identical to
+# the C++ one, the second one contains our ObjC++ additions.
+gtfiles="\$(srcdir)/cp/rtti.c \$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.h \$(srcdir)/cp/parser.c \$(srcdir)/cp/method.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-lex.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c \$(srcdir)/cp/class.c \$(srcdir)/cp/cp-objcp-common.c \
+\$(srcdir)/objc/objc-act.h \$(srcdir)/objc/objc-act.c \$(srcdir)/objc/objc-runtime-shared-support.c \$(srcdir)/objc/objc-gnu-runtime-abi-01.c \$(srcdir)/objc/objc-next-runtime-abi-01.c \$(srcdir)/objc/objc-next-runtime-abi-02.c \$(srcdir)/c-family/c-cppbuiltin.c"
+