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

[Andrew MacLeod <amacleod at redhat dot com>]

On 11/03/2015 01:06 AM, Jeff Law wrote:
> On 10/14/2015 09:14 AM, Andrew MacLeod wrote:
> > Here's the latest version of the tools for a sub directory in contrib.
> > I've handled all the feedback, except I have not fully commented the
> > python code in the tools, nor followed any particular coding
> > convention...   Documentation has been handled, and I've added some
> > additional comments to the places which were noted as being unclear. Ive
> > also removed all tabs from the source files.
> >
> > Ive also updated show-headers slightly to be a little more
> > error-resistant and to put some emphasis on any header files specified
> > on the command as being of interest . (when there are 140 shown, it can
> > be hard to find the one you are looking for sometimes)
> >
> > Do we wish to impose anything in particular on the source for tools
> > going into this sub-directory of contrib? The other tools in contrib
> > don't seem to have much in the way of coding standards.     I also
> > wonder if anyone other than me will look at them much :-)
> I'm certainly interested in them.
>
> Do you have any sense of whether or not coverage of the tools has 
> improved over short time since we started squashing out conditional 
> compilation?  I was running the header file reordering bits on the 
> trunk and was a bit surprised of how many things they're still 
> changing.  But that would make sense if some files are now being 
> processed that weren't before because we've squashed out the 
> conditional compilation.

hmm. no, i dont have a feel for that.  Anl to be fair, I didn't run the 
tools on every file in trunk.  I limited it to the ones in backend.h, 
and took out even a few of those that were troublesome in some way or 
other at some point.   I wouldnt expect the conditional stuff to affect 
reordering much.  reducing...  we might start to see things like tm.h or 
target.h included less.

A further enhancement in line with that would be to teach the reducer 
about a couple of special files.. like the relationship between 
options.h, tm.h and target.h.   sometimes target.h was included when in 
fact options.h was the only thing actually needed..  During the 
flayttening process I manually handled this by flattening tm.h out of 
target.h and options.h into anything that included tm.h...  so every 
file had options.h, tm.h and target.h explicitly included, and then the 
reducer would just pick the "minimum".  of course, the reorder tool 
works against this by combining them again :-)

however, the tool could be taught when it see target.h for instance, if 
it can't be removed, it could try replacing it with options.h and if 
that fails, tm.h..    That sort of thing could automatically remove 
headers that arent needed because target macros have been turned into 
hooks or something.  I suppose that could even be generalized to trying 
to replace  each header that included other headers...  I wonder how 
safe that would be. hum.

> It certainly is true that the total result is smaller than any of the 
> backend, config/ or languages changes that you posted, and I'm running 
> it across the entire source tree, but I'm still surprised at how much 
> churn I'm seeing.
>
> If it weren't for the level of churn, I'd probably be suggesting we 
> just have this stuff run regularly (weekly, monthly, whatever) and 
> commit the result after a sanity looksie.  I've yet to see this tool 
> botch anything and if we're not unnecessarily churning the sources, 
> keeping us clean WRT canononical ordering and duplicate removal 
> automatically seems like a good place to be.
it can botch one of the go files.. go has a backend.h of it's own... 
which buggers things up quite nicely since it doesnt include a bunch of 
the headers gcc's backend.h does :-)

The reordering tool is likely safer to run across the board.. especially 
if we can determine the very small subset it shouldn't be run on.

Right now it triggers off the presence of system.h... if system.h is not 
present, it wont do anything to the file. I haven't tried running it 
against *.c to see if there are any other failures, perhaps thats not a 
bad idea.   That will also provide us with a list of files which have 
headers included within conditional compliation... there are a few of 
those :-P  and maybe they could be fixed.  by default it wou=nt do 
anything to those either.

Anyway, if we run it against everything and check it in, then in theory 
there isn't any reason you couldnt spot run it at some interval.. there 
shouldn't be much churn then.

> Maybe do another commit of the reordering output and evaluate again in 
> a month?
>
> I don't think we're quite there on the reducer and it obviously 
> requires more infrastructure in place to test.  But it'd be nice to 
> get to a similar state on that tool.
yeah, the reducer still needs some tweaks to be generally runnable I 
think.   IN particular, how to deal with externally supplied macros it 
cant really see.  Im still thinking about that one.

> Which reminds me, you ought to add a VMS target to your tests.  The 
> reducer botched vmsdbgout.c.

Thats one of the reasons vmsdbgout.c wasn't in the list of things I 
reduced :-)

#include "config.h"
#include "system.h"
#include "coretypes.h"

#ifdef VMS_DEBUGGING_INFO
#include "alias.h"
#include "tree.h"
#include "varasm.h"
#include "version.h"
#include "flags.h"
#include "rtl.h"
#include "output.h"
#include "vmsdbg.h"
#include "debug.h"
#include "langhooks.h"
#include "function.h"
#include "target.h"

You have to override it with -i in order to reorder them too.. There are 
headers included within  conditional compilation, and this is one of 
those that is simply not safe...  And yes, I guess adding a vms target 
would cover that for the reducer at least...

I almost suggested moving all the includes out of the conditional...  
which is not unreasonable... and would resolve the issue as well.   It 
would be good to remove all the conditionals in all the source files 
like that.. some are easier than others tho.

back to reordering...  the gen files are a bit of a pain too because of 
the rtl.h conditional inclusions.. which I never really found a good 
solution for...   maybe we should have a brtl.h which is used in concert 
with any source which uses bconfig.h.. brtl.h could verifies bconfig.h 
has been included and then includes those headers it needs, followed by 
rtl.h itself.. and the tool could confirm the right pairing of 
config.h/rtl.h  bconfig.h/brtl.h   is used.   hmm.

Andrew