There are 9 tools I used over the run of the project. They were
developed in various stages and iterations, but I tried to at least have
some common interface things, and I tried some cleaning up and
documentation.
I'll probably have to make multiple passes over this. A disclaimer
first, I have done enough Python programming to develop a dislike for
the language, but not enough to call myself an expert.
General comments first. Where applicable, I think we should apply the
same coding standards to Python as we do for C/C++. That means things
like function comments documenting parameters. They are absent for the
most part in this patch, and I won't point out individual instances.
Also, I think the documentation should follow our usual rules. There are
spelling and grammar problems. I will point out what I find (only the
first instance for recurring problems), but please proofread the whole
thing for the next submission. The Thunderbird spellchecker actually is
pointing out a lot of these. Capitalize starts of sentences, write full
sentences and terminate with punctuation.
No commenting on the quality of python code... :-) I was
learning python on the fly. Im sure some things are QUITE awful.,
Yeah, the general impression is of fairly ad-hoc code. Not sure how much
can be done about this.
+ trigger the help message. Help may specify additonal functionality to what is
"additional"
+ - For*all* tools, option format for specifying filenames must have no spaces
Space after "For", remove double space. This pattern occurs very often -
something your editor does maybe?
+ - Many of the tools are required to be run from the core gcc source directory
+ containing coretypes.h typically that is in gcc/gcc from a source checkout.
Odd whitespace, and probably lack of punctuation before "typically".
+ gcc-order-headers
+ -----------------
+ This will reorder any primary backend headers files into a canonical order
+ which will resolve any hidden dependencies they may have. Any unknown
+ headers will simply be occur after the recognized core files, and retain the
+ same relative ordering they had.
Grammar ("be occur").
This sounds like the intention is to move recognized core files (I
assume these are the ones in the "order" array in the tool) to the
start, and leaving everything alone? I was a bit confused about this at
first; I see for example "timevar.h" moving around without being present
in the list, but it looks like it gets added implicitly through being
included by df.h. (Incidentally, this looks like another case like the
obstack one - a file using timevars should include timevar.h IMO, even
if it also includes df.h).
+
+ Must be run in the core gcc source directory
"This tool must be run in the core gcc source directory." (Punctuation
and grammar).
+ Any files which are changed are output, and the original is saved with a
+ .bak extention.
+
+ ex.: gcc-order-headers tree-ssa.c c/c-decl.c
It might be more useful to produce a diff rather than modify files in-place.
+ if any header files are included within a conditional code block, the tool
+ will issue a message and not change the file. When this happens, you can
+ manually inspect the file, and if reorder it will be fine, rerun the command
"if reorder it will be fine"?
+ It does a 4 level deep find of all source files from the current directory
+ and look in each of those for a #include of the specified headers. So
expect
+ a little bit of slowness.
"looks"?
+
+ -i limits the search to only other header files.
+ -c limits the search to .c and .cc files.
+ -a shows only source files which include*all* specified headers.
Whitespace.
+ it is good practive to run 'gcc-order-headers' on a source file before
trying
"practice"
+ Any desired target builds should be built in one directory using a modified
+ config-list.mk file which doesnt delete the build directory when its done.
"doesn't", or more probably "does not" in documentation.
+ The tool will analyze a source file and attempt to remove each
non-conditional
+ header from last to first in the file.:
+ It will first attempt to build the native all-gcc target.
+ If that succeeds, it will attempt to build any target build .o files
+ If that suceeds, it will check to see if there are any conditional
"succeeds"
+ compilation dependencies between this header file and the source file
or
+ any header whihch have already been determined as non-removable.
"whihch"
+ If all these tests are passed, the header file is determined to be
removable
+ and is removed from the source file.
+ This continues until all headers have been checked.
One thing I've wondered about - have you tried checking for object file
differences?
As far as I can tell the dependency checking does not check for undefs.
Is that correct? I think that needs to be added.
+ At this point, the a bootstrap is attempted in the native build, and if that
"the a"
+ A small subset of targets has been determined to provide excellent coverage,
+ at least as of Aug 31/15 . A fullset of targets reduced all of the files
"fullset", and whitespace. Determined how?
+ making up libbackend.a. All of the features which requires target testing
+ were found to be triggered by one or more of these targets. They are
+ actually known to the tool, and when checkiong target, it will check those
"checkiong".
+ targets first, then the rest. It is mostly safe to do a reduction with just
+ these targets, at least until some new whacky target comes along.
+ building config-list.mk with :
+ LIST="aarch64-linux-gnu arm-netbsdelf avr-rtems c6x-elf epiphany-elf
hppa2.0-hpux10.1 i686-mingw32crt i686-pc-msdosdjgpp mipsel-elf powerpc-eabisimaltivec
rs6000-ibm-aix5.1.0 sh-superh-elf sparc64-elf spu-elf"
I think I get what you're trying to say, but the whole paragraph could
be rewritten for clarity.
+ reduce-headers.log : All the compilation failure output that tool tried.
"the tool"?
+ and why it thinks taht is the case
"taht".
+ $src.c.log : for each failed header removal, the compilation
+ messages as to why it failed.
+ $header.h.log: The same log is put into the relevent header log as well.
"relevant"
+ The tool will aggregate all these and generate a graph of the dependencies
+ exposed during compilation. red lines indicate dependecies that are
"depndecies"
+ presednt because a head file physically includes another header. Black lines
"presednt"
+ represent data dependencies causing compilation if the header isnt present.
"is not"
+ for x in sys.argv[1:]:
+ if x[0:2] == "-h":
+ usage = True
+ else:
+ src.append (x)
There are getopt and argparse libraries available for python. I seem to
recall fighting them at some point because they didn't quite do what I
expected from a C getopt, so it may not be worth it trying to use them.
+ if not usage and len(src) > 0:
+
+ incl = { }
Watch the extra blank lines.
+ if dup.get(d) == None:
I think we want to be consistent with our C style? I.e., extra space
before parentheses.
+ l.sort(key=lambda tup:tup[0], reverse=True)
And spaces around things like = operators.
+ # Don't put diagnostic*.h into the ordering list, its special since
"it is". Many instances, please grep for "its" and fix them all.
+ # 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.
+ # rtl.h gets tagged as a duplicate includer for all of coretypes, but thats
"that's"
+ # process diagnostic.h first.. it's special since GCC_DIAG_STYLE can be
+ # overridden by languages, but must be done so by a file included BEFORE it.
+ # so make sure it isn't seen as inclujded by one of those files by making it
"inclujded"
+ # Now crate the master ordering list
"create".
+ 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.
+ print " -s Show the cananoical order of known includes"
"canonical"
+ print "Multi-line comments after a #include can also cause failuer, they must be
turned"
"failuer"
+ ignore = [ "coretypes_h",
+ "machmode_h",
+ "signop_h",
+ "wide_int_h",
+ "double_int_h",
+ "real_h",
+ "fixed_value_h",
+ "hash_table_h",
+ "statistics_h",
+ "ggc_h",
+ "vec_h",
+ "hashtab_h",
+ "inchash_h",
+ "mem_stats_traits_h",
+ "hash_map_traits_h",
+ "mem_stats_h",
+ "hash_map_h",
+ "hash_set_h",
+ "input_h",
+ "line_map_h",
+ "is_a_h",
+ "system_h",
+ "config_h" ]
Is the random indentation indicating some kind of nesting? If not,
please fix.
+ 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).
+ if dohelp:
+ print "Generates a graph of the include web for specified files."
+ print "Usage: [-finput_file] [-h] [-ooutput] [file1 ... [filen]]"
+ print " -finput_file : Input file is file containing a list of files"
+ print " -ooutput : Specifies output to output.dot and output.png"
+ print " defaults to graph.dot and graph.png"
+ print " -nnum : specifies the # of edges beyond which sfdp is invoked.
def=0"
+ print " -a : Aggregate all .c files to 1 file. Shows only include
web."
+ print " -at : Aggregate, but don't include terminal.h to .c links. "
+ print " -h : help"
The formatting of the help output seems somewhat random. Also "is a file"?
+ if len(inc) > 0:
+ # inc2 = re.findall (ur"defined *\((.+?)\)", inc[0])
+ inc2 = re.findall (ur"defined\s*\((.+?)\)", inc[0])
Intentionally commented out?
+
+ def process_ii (filen):
+ return process_include_info (filen, False, False)
+
+ def process_ii_macro (filen):
+ return process_include_info (filen, True, False)
+
+ def process_ii_src (filen):
+ return process_include_info (filen, False, True)
+
+ def process_ii_macro_src (filen):
+ return process_include_info (filen, True, True)
+
+ def ii_base (iinfo):
+ return iinfo[0]
+
+ def ii_path (iinfo):
+ return iinfo[1]
+
+ def ii_include_list (iinfo):
+ return iinfo[2]
+
+ def ii_include_list_cond (iinfo):
+ return iinfo[3]
+
+ def ii_include_list_non_cond (iinfo):
+ l = ii_include_list (iinfo)
+ for n in ii_include_list_cond (iinfo):
+ l.remove (n)
+ return l
+
+ def ii_macro_consume (iinfo):
+ return iinfo[4]
+
+ def ii_macro_define (iinfo):
+ return iinfo[5]
+
+ def ii_src (iinfo):
+ return iinfo[6]
+
+ def ii_src_line (iinfo):
+ return iinfo[7]
That's a lot of little functions with pretty much no clue for the reader
what's going on. It looks like maybe there's an array where a struct
should have been used?
+ # extract data for include file name_h and enter it into the dictionary.
+ # this doesnt change once read in. use_requies is True if you want to
"does not", "use_requies"
+ # find FIND in src, and replace it with the list of includes in REPLACE
+ # remove any duplicates of find or replace, and if some of hte replace
"hte"
+ # includes occur earlier in the inlude chain, leave them.
"inlude"
+ # 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?
+ print " -a : Show only files which*all* listed files are included"
Whitespace around *all*. Seems to happen quite often.
+ # given a header name, normalize it. ie cp/cp-tree.h could be in gcc, while
Formatting, capitalization.
+ # the same header could be referenecd from within the cp subdirectory as
"referenced"
+ # Adds a header file and it's sub includes to the global dictionary if they
This time, "its".
+ # aren't already there. SPecify s_path since different build directories may
"SPecify"
+ if usage:
+ print "Attempts to remove extraneous include files from source files. "
+ print " "
+ print "Should be run from the main gcc source directory, and works on a
target"
+ print "directory, as we attempt to make the 'all' target."
+ print " "
+ print "By default, gcc-reorder-includes is run on each file before
attempting"
+ print "to remove includes. this removes duplicates and puts some headers in
a"
+ print "canonical ordering"
+ print " "
+ print "The build directory should be ready to compile via make. Time is saved
"
Space at the end of the line (two cases in this block).
+ print " "
+ print " show in a hierarchical visual format how many times each header
file"
+ print " is included ina source file. Should be run from the source
directory"
"ina".
Bernd
