gengtype indentation issues

[Basile Starynkevitch <basile at starynkevitch dot net>]

Hello All,

References
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01739.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01740.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01742.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01743.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01743.html
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01744.html

Overall context:

gengtype (files gcc/gengtype*.[chl] of the trunk rev. 164550 to be
specific) is a generator program used internally within GCC to handle
GTY-ed types and generate GGC marking routines & PCH persisting
routines, in relation with the Ggc garbage collector -usually called
between GCC passes- and the precompiler header infrastructures). See
http://gcc.gnu.org/onlinedocs/gccint/Type-Information.html for more.
Gengtype generates files named gt-*.h and gtype*.[ch] in GCC build
directory by reading many source files in GCC source directory (and
some of these XXX.c source files have a #include "gt-XXX.h" line near
the end).

In my not so humble opinion, gengtype is not very well written (its
code is much worse than the average GCC pass code), perhaps because it
has been committed long time ago (2002?) without an extensive and deep
peer review.  It is only a C code generator.  It is used internally
when building GCC, and may be used or needed when building GCC plugins
(the few plugins using GTY).  The code generated by gengtype is used
within GCC (the actual program installed by GCC users).  So if
gengtype don't change its generating code behavior -that is don't
change the generated C files it is writing, it cannot possibly alter
the behavior of GCC (because gengtype's own code does not appear
inside GCC programs like gcc or cc1 or cc1plus etc; the only &
important effect of gengtype on GCC programs is by the C code
generated by gengtype. So w.r.t. GCC, gengtype is a metaprogramming
utility.).

Currently (and in GCC 4.5) to use gengtype for building any GCC plugin
having its own GTY-ed types, you need both the entire GCC build tree
and the entire GCC source tree.  This is unfortunate, but it is
documented (end of http://gcc.gnu.org/onlinedocs/gccint/Plugins.html
page) as "Plugins needing to use gengtype require a GCC build
directory for the same version of GCC that they will be linked
against.". This is against the packaging practices of most Linux
distributions and is hurting many good software engineering rules
Diego Novillo told once (IIRC) that it is a dirty kludge

With Jeremie Salvucci, I worked hard on improving gengtype. We did
clean up minimally & superficially gengtype source code (which, even
with our patches, remain dirty, but a bit less dirty than before), and
most importantly, we painfully coded a persistence mechanism.  qWith
our gengtype patches, gengtype generates the same code as before (so
cannot alter GCC behevior) and writes its entire state in a textual
file gtype.state.  For plugins using GTY, our patched gengtype can
read that gtype.state file, parse the plugin source code, and generate
the gt-*.h

Notice that gengtype is not very well written, and it is *not* (in its
current form from trunk rev. 164550) indented according to GNU & GCC
standards.  Much more on this below!


However, the patches we (Jeremie & Basile) have sent still have
indentation issues.  And I am not able, even after having tried more
than once, to cope with them without constrctive and practical help
(e.g. a tool, a precise procedure to make patches [including .emacs
configuration etc...]), and I did try several times to work on these
indentation issues without success, since most of the comments I've
got so far on the patches I have sent are on indentation.  Dealing
with the non-indentation issues of my patches seem feasible to me.
However my time budget on gengtype patches is very negative already (I
will already have hard time to justify w.r.t. the funding agencies for
my GCC work having spent too much time on gengtype).  I really did
spent much too much time on gengtype, and I begin to think that it is
not worth much more efforts. After all, these gengtype efforts are
only useful for plugin developers, and there are few of them, plugins
are disliked by most of the community.  In particular, this gengtype
effort is only profitable for plugins using GTY.  And there are very
few such plugins (perhaps only MELT and possibly DraggonEgg).  For
MELT, I can and do already deal without the gengtype feature
introduced by my patches: I just keep in the
contrib/gt-melt-runtime-plugin-4.5.h file of the MELT branch the
gengtype generated file corresponding to gcc/melt-runtime.[ch] so that
in practice MELT can be built as a plugin without gengtype.  So I can
continue to develop MELT without strictly needing an improved gengtype
(even if that would be very convenient).  In other words, my gengtype
work is to help the community, and I don't need it much myself (I can
cope without) within MELT.

Regarding the indentation aspects of our work: Jeremie did in the past
had a faulty .emacs (unsuitable for GCC formatting conventions), and I
did not notice it immediately (this is my = Basile's = fault, not
Jeremie's.  I am the one to blame for not noticing that
immediately.). But then I did recreate twice our patches, working on
indentation issues. I failed to achieve conformance with current
gengtype source code, because gengtype indentation is faulty
(currently, and without any patches).


About current's gengtype mis-indentation.

The current gengtype source code is badly indented, and this explains
why we have so hard time to produce patches with minimal indentation
issues. 

To demonstrate that gengtype is currently badly indented, I did the
following. I looked at all the typedef-s inside gengtype.[ch]; there
are few of them.  Then, I made the following indentgengtype script.
This is on Debian/Testing/AMD64
     #! /bin/sh -x
     indent -gnu \
      -T lang_bitmap \
      -T outf_p \
      -T process_field_fn \
      -T func_name_fn \
      -T pair_p \
      -T type_p \
      -T const_type_p \
      -T options_p 
         $*

The indent is GNU ident 2.2.11.

I then run indentgengtype on pristine (that is GCC trunk 164550)
gcc/gengtype.c & gcc/gengtype.h -there is no hidden .indent.pro file
involved; this produced a huge diff. And it did indeed found a lot of
indenting issues within the current gcc/gengtype.c, for example

@@ -1844,23 +1850,24 @@ close_output_files (void)
   for (of = output_files; of; of = of->next)
     {
 
-      if (!is_file_equal(of))
-      {
-        FILE *newfile = fopen (of->name, "w");
-        if (newfile == NULL)
-          fatal ("opening output file %s: %s", of->name, xstrerror (errno));
-        if (fwrite (of->buf, 1, of->bufused, newfile) != of->bufused)
-          fatal ("writing output file %s: %s", of->name, xstrerror (errno));
-        if (fclose (newfile) != 0)
-          fatal ("closing output file %s: %s", of->name, xstrerror (errno));
-      }
-      free(of->buf);
+      if (!is_file_equal (of))
+       {
+         FILE *newfile = fopen (of->name, "w");
+         if (newfile == NULL)
+           fatal ("opening output file %s: %s", of->name, xstrerror (errno));
+         if (fwrite (of->buf, 1, of->bufused, newfile) != of->bufused)
+           fatal ("writing output file %s: %s", of->name, xstrerror (errno));
+         if (fclose (newfile) != 0)
+           fatal ("closing output file %s: %s", of->name, xstrerror (errno));
+       }
+      free (of->buf);
       of->buf = NULL;
       of->bufused = of->buflength = 0;
     }
 }

And indeed, this example shows that the original gengtype.c is badly
indented (according to
http://www.gnu.org/prep/standards/standards.html#Formatting the brace
should not be at the same colon than the if). There are lot of other
such indenting issues. (The patch obtained by indenting the two
gengtype.[ch] files is 1931 lines long).

If one uses GNU emacs (version 23.2.1 from Debian/Testing) M-x
mark-whole-buffer and then M-x indent-region, one gets also a big
patch of 527 lines at least. So really gengtype source code is wrongly
indented.  For reference, my ~/.emacs is quite small, it is exactly

  ;; Basile's .emacs
  (global-set-key [f6] 'goto-line)
  (global-set-key [f10] 'next-error)
  (global-set-key [f12] 'recompile)
  (line-number-mode 1)
  (mouse-wheel-mode 1)
  (server-start)

  ;;; http://gcc.gnu.org/wiki/FormattingCodeForGCC
  ;; This line is not actually needed, it is on by default.
  (setq-default indent-tabs-mode t)

  (add-hook 'c-mode-hook 'linux-c-mode)
  (defun linux-c-mode()
  ;; set gnu style.
   (c-set-style "gnu")
  ;; TAB offset set to 2
   (setq c-basic-offset 2)
   )

  (custom-set-variables
   ;; custom-set-variables was added by Custom.
   ;; If you edit it by hand, you could mess it up, so be careful.
   ;; Your init file should contain only one such instance.
   ;; If there is more than one, they won't work right.
   '(column-number-mode t)
   '(ps-default-bg "mistyrose")
   '(show-paren-mode t)
   '(transient-mark-mode t)
   '(uniquify-buffer-name-style (quote forward) nil (uniquify)))
  (custom-set-faces
   ;; custom-set-faces was added by Custom.
   ;; If you edit it by hand, you could mess it up, so be careful.
   ;; Your init file should contain only one such instance.
   ;; If there is more than one, they won't work right.
   )


This explains why Jeremie & me Basile have so hard time solving
indentation issues. We are not realistically able to indent our code
manually line by line (disabling auto-indentation, and typing exactly
the number of spaces on each line; this is an Herculean effort
requiring too much time, and it will not help future gengtype hackers,
which is one of our goals) to match as closely as possible current
gengtype faulty indentation.  When we edit code inside a function, we
are asking emacs to indent the entire function, precisely to help us
indent code following GNU standards.  But current gengtype code does
not follow indentation rules required by GNU & GCC standards.

So, I don't see how we can get out of this mess in reasonable time,
and I am asking for your help.  I was thinking of (by order of
decreasing preference):

  1. [my strong preference] indenting cleanly [probably with GNU
  indent, and then some minor tweaks] the gengtype.c & gengtype.h code
  and submitting a first patch containing only indentation changes!
  But before doing that easy change, I want to be sure that such
  indentation patch to gengtype.[ch] could be accepted (it is against
  GCC habits, but IMHO gengtype is in so poor shape that it deserves a
  special exception; also notice that indentation changes in gengtype
  cannot change the behavior of GCC; even any change of gengtype which
  don't alter the generated C files cannot change GCC behavior).

  2. Getting much more constructive and detailed feedback on our patch
  series, e.g. not only "indentation is wrong" in
  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01744.html but the
  exact patch which should be minimally accepted. While this has the
  advantage of minimizing patch length, it has the big disadvantage of
  perpetuating indentation errors already present in gengtype.c and
  that will not help future gengtype hackers.

  3. Giving exact and concrete and precise editing advices, as precise
  as put this and this lines [please be extremely specific; don't
  forget that I have a French AZERTY keyboard, not US QWERTY ones] in
  your .emacs near line 10, and type select with your mouse the
  function to be indented, and type M-x foo followed by M-x bar.  I
  need hints at that low level of precision!  Helpful remarks like
  "Indentation!"  is *not* enough.

  4. Since I am attending the GCC Summit, I could perhaps fly one or
  two days earlier to Ottowa and meet any GCC global reviewer able to
  help me indenting code according to his preference, and able to OK
  it.  But if some global reviewer (Diego Novillo, Ian Taylor, someone
  else?)  is able to give me his precious time (and that includes
  teaching me face to face, on the same physical keyboard, how to
  indent gengtype.c code according to his preference), please tell me
  much in advance (i.e. before Tuesday september 28th 2010 if
  possible)


Without such concrete indentation help on gengtype, I am not able to
push our gengtype efforts [the patch improving it for plugins], and I
believe they will be lost.

With help on indentation, I am able to work on the few non-indentation
issues remaining in our patches.

Thanks for reading.

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 mines, sont seulement les miennes} ***