[objc++] RFH: PR 48167 gengtype failure

[Diego Novillo, 2011-03-18 15:52:49]

I think gengtype is failing me here, but I'm not sure.  In PR 48167 we
are failing to link cc1objplus because of undefined references:

cp/parser.o:(.rodata+0x2b18): undefined reference to `gt_ggc_mx_cp_parser'
cp/parser.o:(.rodata+0x2b20): undefined reference to `gt_pch_nx_cp_parser'
cp/tree.o: In function `gt_ggc_mx_lang_tree_node':
tree.c:(.text+0xd33e): undefined reference to `gt_ggc_mx_cp_token_cache'
cp/tree.o: In function `gt_pch_nx_lang_tree_node':
tree.c:(.text+0xea2e): undefined reference to `gt_pch_nx_cp_token_cache'
cp/tree.o: In function `gt_ggc_mx_lang_decl':
tree.c:(.text+0xd23b): undefined reference to `gt_ggc_mx_cp_token_cache'
cp/tree.o: In function `gt_pch_nx_lang_decl':
tree.c:(.text+0xe923): undefined reference to `gt_pch_nx_cp_token_cache'
collect2: ld returned 1 exit status
make: *** [cc1objplus] Error 1

The problem is that those GTY functions are generated in cp/cp-lang.o,
which is not linked to cc1objplus.

The usual way of getting these functions generated is to #include the
header file in <fe>/<fe>-lang.c and add it to gtfiles in
config-lang.in (which this patch does).  However, gengtype is not
generating the functions, so I'm not sure how to make it do that.

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?


Thanks.  Diego.

diff --git a/gcc/objcp/Make-lang.in b/gcc/objcp/Make-lang.in
index 5bbd27e..e2e0196 100644
--- a/gcc/objcp/Make-lang.in
+++ b/gcc/objcp/Make-lang.in
@@ -45,7 +45,7 @@ obj-c++: cc1objplus$(exeext)
 .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
+  langhooks.h c-family/c-objc.h objc/objc-act.h $(CXX_PARSER_H)
 
 # Use maximal warnings for this front end.  Also, make ObjC and C++
 # headers accessible.
diff --git a/gcc/objcp/config-lang.in b/gcc/objcp/config-lang.in
index 8c3d9c5..985da17 100644
--- a/gcc/objcp/config-lang.in
+++ b/gcc/objcp/config-lang.in
@@ -37,5 +37,5 @@ build_by_default="no"
 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 "
+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/parser.h
\$(srcdir)/cp/parser.c \$(srcdir)/cp/cp-objcp-common.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
\$(srcdir)/objcp/objcp-lang.c"
 
diff --git a/gcc/objcp/objcp-lang.c b/gcc/objcp/objcp-lang.c
index fe2be66..65857b5 100644
--- a/gcc/objcp/objcp-lang.c
+++ b/gcc/objcp/objcp-lang.c
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "langhooks-def.h"
 #include "target.h"
 #include "cp-objcp-common.h"
+#include "parser.h"
 
 enum c_language_kind c_language = clk_objcxx;
 static void objcxx_init_ts (void);

--
This patch is available for review at http://codereview.appspot.com/4291054

[Laurynas Biveinis, 2011-03-18 16:01:54]

I will be able to look into this on Monday, not earlier, sorry.

2011/3/18 Diego Novillo <dnovillo@google.com>:
> I think gengtype is failing me here, but I'm not sure.  In PR 48167 we
> are failing to link cc1objplus because of undefined references:
>
> cp/parser.o:(.rodata+0x2b18): undefined reference to `gt_ggc_mx_cp_parser'
> cp/parser.o:(.rodata+0x2b20): undefined reference to `gt_pch_nx_cp_parser'
> cp/tree.o: In function `gt_ggc_mx_lang_tree_node':
> tree.c:(.text+0xd33e): undefined reference to `gt_ggc_mx_cp_token_cache'
> cp/tree.o: In function `gt_pch_nx_lang_tree_node':
> tree.c:(.text+0xea2e): undefined reference to `gt_pch_nx_cp_token_cache'
> cp/tree.o: In function `gt_ggc_mx_lang_decl':
> tree.c:(.text+0xd23b): undefined reference to `gt_ggc_mx_cp_token_cache'
> cp/tree.o: In function `gt_pch_nx_lang_decl':
> tree.c:(.text+0xe923): undefined reference to `gt_pch_nx_cp_token_cache'
> collect2: ld returned 1 exit status
> make: *** [cc1objplus] Error 1
>
> The problem is that those GTY functions are generated in cp/cp-lang.o,
> which is not linked to cc1objplus.
>
> The usual way of getting these functions generated is to #include the
> header file in <fe>/<fe>-lang.c and add it to gtfiles in
> config-lang.in (which this patch does).  However, gengtype is not
> generating the functions, so I'm not sure how to make it do that.
>
> 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?
>
>
> Thanks.  Diego.
>
> diff --git a/gcc/objcp/Make-lang.in b/gcc/objcp/Make-lang.in
> index 5bbd27e..e2e0196 100644
> --- a/gcc/objcp/Make-lang.in
> +++ b/gcc/objcp/Make-lang.in
> @@ -45,7 +45,7 @@ obj-c++: cc1objplus$(exeext)
>  .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
> +  langhooks.h c-family/c-objc.h objc/objc-act.h $(CXX_PARSER_H)
>
>  # Use maximal warnings for this front end.  Also, make ObjC and C++
>  # headers accessible.
> diff --git a/gcc/objcp/config-lang.in b/gcc/objcp/config-lang.in
> index 8c3d9c5..985da17 100644
> --- a/gcc/objcp/config-lang.in
> +++ b/gcc/objcp/config-lang.in
> @@ -37,5 +37,5 @@ build_by_default="no"
>  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 "
> +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/parser.h
\$(srcdir)/cp/parser.c \$(srcdir)/cp/cp-objcp-common.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
\$(srcdir)/objcp/objcp-lang.c"
>
> diff --git a/gcc/objcp/objcp-lang.c b/gcc/objcp/objcp-lang.c
> index fe2be66..65857b5 100644
> --- a/gcc/objcp/objcp-lang.c
> +++ b/gcc/objcp/objcp-lang.c
> @@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "langhooks-def.h"
>  #include "target.h"
>  #include "cp-objcp-common.h"
> +#include "parser.h"
>
>  enum c_language_kind c_language = clk_objcxx;
>  static void objcxx_init_ts (void);
>
> --
> This patch is available for review at http://codereview.appspot.com/4291054
>



-- 
Laurynas

[basile_starynkevitch.net, 2011-03-18 18:40:21]

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!
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

[Diego Novillo, 2011-03-18 19:50:46]

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.

[Diego Novillo, 2011-03-18 21:14:20]

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.

[mikestump_comcast.net, 2011-03-19 04:04:23]

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, 2011-03-21 08:19:59]

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.

-- 
Laurynas