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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to