[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

[Mike Rice via Phabricator via cfe-commits]

mikerice added inline comments.


================
Comment at: include/clang/Driver/CC1Options.td:604
+  HelpText<"When creating a pch stop at this file.  When using a pch start "
+           "after this file.">;
 def fno_pch_timestamp : Flag<["-"], "fno-pch-timestamp">,
----------------
hans wrote:
> The "through header" terminology was new to me, and I didn't see it when browsing the MSDN articles about precompiled headers. The HelpText here probably isn't the right place, but it would be good if the term could be documented somewhere to make it clearer exactly what the behaviour is. It's not really obvious to me what "stop at this file" and "start after this file"  means. I can guess, but it would be nice if it were more explicit :-)
You definitely have to look hard at the MSDN docs to find mention of through headers.  If you look at the documentation for /Yc and /Yu you can see some vague references.  I think it may have been more prominent many years ago. 

The MSDN page says "For /Yc, filename specifies the point at which precompilation stops; the compiler precompiles all code though(sic) filename..." https://msdn.microsoft.com/en-us/library/z0atkd6c.aspx

I'll look for a place to document this better in a comment at least.




================
Comment at: lib/Lex/PPDirectives.cpp:1887
+      SkippingUntilPCHThroughHeader = false;
+    return;
+  }
----------------
hans wrote:
> Returning here seemed surprising to me. Isn't just setting the flag and then carrying on as usual what we want?
When skipping we always return since we don't want to include any headers encountered because they are in the PCH.  When the 'through header' is seen we also return because it too is in the PCH.  In that case we also set the flag to stop the skipping.  

So with /Yu"hdr2.h":

#include "hdr1.h"  <- in PCH
#include "hdr2.h"  <-  in PCH
#include "hdr3.h"  <- next include processed normally.

That's because /Yc"hdr2.h" contains all the headers "through" hdr2.h (i.e. hdr1.h and hdr2.h).



https://reviews.llvm.org/D46652