Patch: reporting the full include graph for a C++ compilation unit

Hello happy people,

I created a small patch to Clang to allow the compiler to report the full inclusion graph (i.e. a list of including file - included file) pairs. Note that this is quite similar to the functionality of -H, with the difference that this one reports also inclusions that are skipped because it would have no effect due to include guards and that the output is written to a file instead of stderr.

The patch is in the attachment. What does it take to get this submitted to Clang?

Lukács

first.patch (6.71 KB)

Index: include/clang/Driver/CC1Options.td

Hello happy people,

I created a small patch to Clang to allow the compiler to report the full inclusion graph (i.e. a list of including file - included file) pairs. Note that this is quite similar to the functionality of -H, with the difference that this one reports also inclusions that are skipped because it would have no effect due to include guards and that the output is written to a file instead of stderr.

The patch is in the attachment. What does it take to get this submitted to Clang?

Index: include/clang/Driver/CC1Options.td

— include/clang/Driver/CC1Options.td (revision 131087)
+++ include/clang/Driver/CC1Options.td (working copy)
@@ -200,6 +200,8 @@
HelpText<“Filename (or -) to write header include output to”>;
def H : Flag<“-H”>,
HelpText<“Show header includes and nesting depth”>;
+def inclusion_graph_file : Separate<“-inclusion-graph-file”>,

  • HelpText<“Print the complete header inclusion graph to this file”>;
    def MQ : Separate<“-MQ”>, HelpText<“Specify target to quote for dependency”>;
    def MT : Separate<“-MT”>, HelpText<“Specify target for dependency”>;
    def MP : Flag<“-MP”>,

This is a -cc1-level option. Did you also mean to have it work through the driver?

Yes! I admit I am quite unfamiliar with how Clang works, so I assumed that it was enough to do it only there. I threaded it through Driver, too.

If a path has spaces in it, you won’t be able to tell where the first path ends and the second path begins. You’ll need to perform some escaping here.

Done. Note that there is small bug in DependencyFileCallback, whereupon backslashes in the file names are not escaped correctly (see DependencyFile:122). I guess this is to mimic GCC’s behavior, though.

Also, the FileManager doesn’t necessarily use full paths. Did you want the relative paths or full paths?

The same that are output by -dependency-file, so I copied the logic from there.

New version of patch attached.

Lukács

p2.patch (8.13 KB)

On second thought, I added line numbers to the output file, so that each output line is now in the form of:

Patch attached.

p3.patch (8.31 KB)

A couple of high-level questions:

  1. Do you need to output both a make-style dependency file and the inclusion graph? One seems to be a superset of the other.
  2. Why did you move away from presumed locations? Those are what appear in the line table, and those are what would be preserved by the preprocessor.
  3. Why can’t we share the code with the make-style dependency file output? You mention basing your escaping on theirs, but you’ve hand rolled your own code.

A couple of high-level questions:

  1. Do you need to output both a make-style dependency file and the inclusion graph? One seems to be a superset of the other.

I think we will only need the inclusion graph.

  1. Why did you move away from presumed locations? Those are what appear in the line table, and those are what would be preserved by the preprocessor.

Because this is how DependencyFile.cpp works. I assumed that that is the “standard” way to go.

  1. Why can’t we share the code with the make-style dependency file output? You mention basing your escaping on theirs, but you’ve hand rolled your own code.

First, there is not much code to share (it’s basically only PrintFilename() that is common), second, the quoting is a bit different: I quote backslashes which DependencyFile.cpp does not do (presumably because that’s how GCC works)