[PATCH] D25153: preprocessor supports `-dI` flag

Steve O'Brien via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 4 10:42:53 PDT 2016


elsteveogrande added a comment.

Thanks again @rsmith!  Updates will be coming; I have some other fixes as well.



> rsmith wrote in PrintPreprocessedOutput.cpp:329
> Is this really necessary? It'll be very ugly on Windows.

Yeah, there will be tons of double-backslashes because of this.  I think to sidestep this -- plus the derp moment above about `\/*` still breaking comments regardless -- I could perhaps simplify all this escaping.

Side note, to explain the motivation of this diff and why I'm jumping through hoops and spending so much time to make this properly-escaped; skip to last paragraph if it's not of interest :).

The ulterior motive is to allow our build system (http://buckbuild.com) to determine how include paths were resolved during preprocessing of some file.  So, for example, when checking an existing precompiled header to see if it's compatible with some other subsequent build (which we'd like to use the PCH in), and assuming other flags like `-g -O2 -fPIC` and so on match, we need to know whether given lists of include paths (that from the PCH build, and the current build) are "compatible".  Like, would a `#include <thread>` resolve to the same libc++ version of thread.

Anyway: so I was trying hard to make this "magic comment" machine-readable but it's getting pretty hairy, with escapes and backslashes and having to somehow escape `*/` and so on.  (Escaping forward slashes will make this ugly on non-Windows :) ).  Since this is a purpose-built and non-generic, directory name escaper, I could try:

- fix the `*/` problem, and `\\`, somehow.  Change `*/` to `*\/` but don't bother escaping slashes all the time (only on comment-busting slashes).
- just drop asterisks and "weird stuff" from filenames, solving those, but causing other problems (correctness, mangling up pathnames).

> PrintPreprocessedOutput.cpp:376-378
>        // FIXME: Preseve whether this was a
>        // #include/#include_next/#include_macros/#import.
>        OS << "#include "

Will fix this site as mentioned.

> rsmith wrote in PrintPreprocessedOutput.cpp:396
> Please do this in the preceding case too.

Will do.  Actually I found `PP.getSpelling(token)` in this same file, worked fine :)

https://reviews.llvm.org/D25153





More information about the cfe-commits mailing list