Re: [Gcc4cli-devel] Source location patch #2

[Gabriele S. <gab...@gm...>]

2009/12/14 Thierry LAFAGE <thi...@in...>:
> Hi Gabriele,
>
> Thanks for your remarks. What kind of coding guidelines do you think of?
> I can fix them before ciing.
>
> Thierry Lafage.

Some lines are longer than 80 characters, those should be split on two
lines (there are many places were this is done so you can follow the
style used in those lines). I also noticed a couple of places were you
have left a multi-line statement without putting brackets around it,
for example:

	if (TREE_CODE (node) == REALPART_EXPR)
          stmt = cil_build_stmt_arg (CIL_LDFLDA,
                                     cil_get_builtin_complex_real_fld (type));

Should be:

	if (TREE_CODE (node) == REALPART_EXPR)
	  {
	    stmt = cil_build_stmt_arg (CIL_LDFLDA,
                                       cil_get_builtin_complex_real_fld (type));
	  }

BTW the same line has 8 spaces in front of the new statement, in GCC
you should use 2, 4 or 6 spaces when indenting and insert a tabulator
every 8 spaces. Finally a space is always left between the name of a
function and the arguments, e.g.:

cil_set_locus(stmt, sloc);

Should become:

cil_set_locus (stmt, sloc);

I know it's annoying, GCC coding guidelines are very quirky but it's
better to stick to them because it simplifies merges with the
mainline. You will get used to them eventually ;-)5

Just a final remark which you might find interesting for your future
work. In our gimple-to-CIL phase we turn GIMPLE nodes into TREE nodes
for simplicity because our original work was done in a time when this
was not necessary because GIMPLE statements were 'special' tree nodes,
now they are specific structures on the other hand. However even if
GIMPLE nodes have their own structures the functionality used for
turning them into trees was left into GCC because nobody wanted to
rewrite the pass (expand) that turned GIMPLE code into RTL code (the
lower-level GCC IR). I haven't been able to follow closely the latest
GCC development but it is entirely possible that in version 4.5 that
pass will have been rewritten and GIMPLE-to-tree functionality
removed. We will have to rewrite the gimple2cil to accomodate for this
in the future and the source location information will also have to
come from GIMPLE so I suggest that you investigate (starting from
gimple_location() and friends) if your work can be adapted to it. I
believe it should be possible and fairly easy; in fact when we will
have to deal with only GIMPLE nodes the whole pass should be simpler
(and we will need a much better stack-promotion pass!). Ciao!

 Gabriele