On Fri, 2010-08-27 at 17:25 +0300, Laurynas Biveinis wrote: > 2010/8/27 <jeremie.salvu...@free.fr>: > > We recompiled GCC-trunk r162692 with the following modification : > > > > In function output_type_enum of gcc/gengtype.c, we replaced > > > > - if (s->kind == TYPE_PARAM_STRUCT && s->u.s.line.file != NULL) > > + if (s->kind == TYPE_PARAM_STRUCT && s->u.param_struct.line.file != NULL) > > > > And Gengtype works like before with c,c++, lto enabled. > > > > Do you think we have to submit a one line patch (if yes, could it be > > reviewed quickly)? We don't know why the old version works, and we think > > writing u.s.line.file is incorrect for TYPE_PARAM_STRUCT (even if it > > happens to work by luck), since the union u.param_struct member is the only > > valid for TYPE_PARAM_STRUCT. > > One-line patches are welcome, but in this instance could you please > find out how the old code worked before changing it (as you admit, you > don't understand it).
My impression is that s->u.s.line.file usually happens to have the same offset (at least on GNU/Linux/AMD64=x86_64) as s->u.param_struct.param[0] and that for every type concerned by output_type_enum its param[0] subfield happens to be non-null. This explains that it worked by accident. Is such an heuristic explanation enough to propose a patch? I am not sure to be able to provide a better one quickly (so if the explanation is not enough, I am not sure to want to propose a half-line patch). By the way, what is the good way to find out exactly what svn commit introduced the bogus line? What surprises me much more is that the s->u.s.line.file != NULL test has been accepted long time ago. From what we understand of gengtype, it could never have made any sense (because conceptually s->u.s does not exist for TYPE_PARAM_STRUCT!), even if it happens to work by pure luck. I am quite surprised (but I admit I only looked a few pages) that there does not seems to be any rules regarding use of union in C code inside GNU. My personal requirement is that a union is only usable if it is inside a structure and is discriminated by a field of this structure (the usual case of a union of sub-structures each starting with a discriminant logically fits that requirement) or by a simple pure fonction depending of such a field (in ML or Ocaml parlance, a union is a discriminated sum type; Also, rpcxdr from Sun twenty years ago had a similar requirement...). But I see no such rules within GCC, and I even saw several unions not used that way. My perhaps excessive opinion is that such union abuse always gives unmaintainable code (and I am in the minority which wants GCC code to be more easily maintainable & readable & hackable by new contributors, even at the expense of raw performance; I feel that competitors's free compilers like LLVM are much better in that aspect.). ### For the curious people, our current work on gengtype is available as http://starynkevitch.net/Basile/gengtype-r163582-27-august-2010.diff As usual, this is a temporary URL. Our patch is not yet ready for submission. I have to clean up the code, correct a bug or two, understand how exactly is the s->u.param_struct.line.file field set in present gengtype. I also have to split our work into several patches, and I am very afraid of not being able to make a sequence of small patches such that each change make still gengtype work for entire GCC! I have no idea if this is even doable (since gengtype is a code generator with *global* side effects on GCC code; it could happen that some partial change work for C but not C++ or Ada parts.). I will perhaps propose a few *related* patches on gengtype this week-end, if I am motivated enough to work on it. Cheers. -- 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} ***
