[PATCH] D17695: clang-cl: Implement initial limited support for precompiled headers.

Hans Wennborg via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 1 15:08:03 PST 2016


hans added a comment.

lgtm2


================
Comment at: include/clang/Driver/Job.h:142
@@ +141,3 @@
+/// Like Command, but always pretends that the wrapped command succeeded.
+/// the primary command crashes.
+class ForceSuccessCommand : public Command {
----------------
The second sentence looks weird.

================
Comment at: lib/Driver/Driver.cpp:2369
@@ +2368,3 @@
+
+    // Add pch if needed: "If you do not specify an extension as part of the
+    // path name, an extension of .pch is assumed. "
----------------
Not a big deal, I just meant the comment would make it more obvious that it's about adding a pch *extension* to the filename if it says ".pch" instead of "pch". I know it becomes obvious later, but then my brain had already asked "what do you mean 'add pch'"?


================
Comment at: lib/Driver/Tools.cpp:438
@@ +437,3 @@
+          if (AI >= YcIndex)
+            continue;
+        } else {
----------------
You got it.

================
Comment at: lib/Frontend/CompilerInstance.cpp:746
@@ +745,3 @@
+      File = FileMgr.getFile(InputFile, /*OpenFile=*/true);
+    } else {
+      const FileEntry *FindFile = FileMgr.getFile(Opts.FindPchSource);
----------------
Maybe add a comment about this FindPchSource mode


http://reviews.llvm.org/D17695





More information about the cfe-commits mailing list