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
