Re: [patch 4/3] Header file reduction - Tools for contrib

[Andrew MacLeod <amacleod at redhat dot com>]

I went through and addressed the comments..  Just for info, a few replies:

> > +     # various front ends have to set GCC_DIAG_STYLE before 
> > including it
> > +     # for each file, we'll tailor where it belongs by looking at 
> > the dup
> > +     # list and seeing which file is included, and position it 
> > appropriately.
>
> From that comment it's not entirely clear how they are handled. Please 
> expand documentation of this mechanism.

I modified the comments in a couple of places to hopefully make it clearer.
> > + for i in order:
> > +   create_master_list (os.path.basename (i), False)
>
> I found myself wanting to pass True. The tool could use a "-v" flag.
I changed the existing -s flag to -v, and simply passed the value 
here...  Now you see the final list, as well as the list of where each 
one came from.
> > +   for line in logfile:
> > +     if len (line) > 21 and line[:21] in depstring:
> > +       if newinc:
> > +         incfrom = list()
> > +     newinc = False
>
> It looks like you are mixing tab and space indentation. For a language 
> like Python, that is absolutely scary. Please fix throughout (I think 
> only spaces is probably best).

vi is doing that automatically for me.. I will expandtabs everything.
> > + # compensate for this stupid warning that should be an error for
> > + # inlined templates
> > + def get_make_rc (rc, output):
> > +   rc = rc % 1280
> > +   if rc == 0:
> > +     # This is not considered a fatal error for a build!  /me rolls 
> > eyes
> > +     h = re.findall ("warning: inline function.*used but never 
> > defined", output)
> > +     if len(h) != 0:
> > +       rc = 1
> > +   return rc;
>
> What's this about?
I've updated the comment to be clearer.  Apparently its only a warning 
to use a template inline function with no definition. I suspect this is 
some oddball C++ thing :-).   Maybe it can be resolved at link time 
somehow?   Anyway,  what I found is that the return code from this is 0 
since its just a warning. SO the tool would remove the header file, and 
when I later try to link and build and object, it becomes a fatal link 
error with the function used but undefined.

It shows up when checking target builds since I only try to build the .o 
file there rather than build and link.   So the tool checks the output 
from the compilation, and if it sees this error, decided to be 
conservative and report it as a build error, and thus it will leave the 
header file in the source.

> > +   print " -a  : Show only files which*all*  listed files are included"
>
> Whitespace around *all*. Seems to happen quite often.

Yeah, that is very odd.   In the code here, there is a space in front of 
every single one of those. I simply changed all these to 'all' instead 
of '*all*'.

I'm also going to add a few more comments to functions in 
gcc-order-headers and reduce-headers, as well as utils.py

Andrew