Fix for bug 38508 - Don't do PCH processing when only generating preprocessor output

[mikerice]

Summary

This clang-cl driver change removes the PCH options when we are only generating preprocessed output. This is similar to the behavior of Y-.

[thakis]

The approach lgtm, thanks.

How does the gcc driver codepath handle this? Does it just not have to worry about this because it doesn't have something like warn_pp_macro_def_mismatch_with_pch?

test/Driver/cl-pch.cpp

368

Can we have an integration-test type test that creates a pch and uses it, and checks that there are no warnings? (Like in https://bugs.llvm.org/show_bug.cgi?id=38508#c1) Probably fairly easy to add to one of the test/PCH/pch-through* files.

[mikerice]

Added a -verify test to ensure no warnings on successful PCH use.

[mikerice]

In D50640#1201216, @thakis wrote:

How does the gcc driver codepath handle this?

Interestingly I'd say. So the gcc PCH model uses -include pch.h to use a PCH. In the driver, -include pch.h is handled by locating a matching pch file (say pch.h.gch) and if found adds -include_pch pch.h.gch instead of -include pch.h. This occurs even when only preprocessing (-E). When preprocessing the compiler then handles the -include_pch option by opening pch.h.gch, finding the original file that created it (pch.h) and doing what -include pch.h does. Seems like it would be better handled by the driver but maybe there is a reason for it.

When we only allowed PCH with a single /FI the model was the same as gcc and it worked similarly. Since the through header/MS model allows creation of a PCH from any tokens not just a single #include, we can't handle it this way anymore.

[thakis]

Thanks! Do you need someone to land this?

[mikerice]

In D50640#1203314, @thakis wrote:

Thanks! Do you need someone to land this?

Feel free to commit it if you want. Otherwise I'll have my colleague take care of it in the morning. Thanks!

lib/Driver/Driver.cpp

if (YcArg && YuArg && strcmp(YcArg->getValue(), YuArg->getValue()) != 0) {
     Args.eraseArg(options::OPT__SLASH_Yu);
     YcArg = YuArg = nullptr;
   }
   if (YcArg && Inputs.size() > 1) {
     Diag(clang::diag::warn_drv_yc_multiple_inputs_clang_cl);
     Args.eraseArg(options::OPT__SLASH_Yc);
     YcArg = nullptr;
   }
-  if (Args.hasArg(options::OPT__SLASH_Y_)) {
-    // /Y- disables all pch handling.  Rather than check for it everywhere,
-    // just remove clang-cl pch-related flags here.
+  if (FinalPhase == phases::Preprocess || Args.hasArg(options::OPT__SLASH_Y_)) {
+    // If only preprocessing or /Y- is used, all pch handling is disabled.
+    // Rather than check for it everywhere, just remove clang-cl pch-related
+    // flags here.
     Args.eraseArg(options::OPT__SLASH_Fp);
     Args.eraseArg(options::OPT__SLASH_Yc);
     Args.eraseArg(options::OPT__SLASH_Yu);
     YcArg = YuArg = nullptr;
   }
 
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);

test/Driver/cl-pch.cpp

 // RUN: %clang_cl /TP -Werror /Ycpchfile.h /FIpchfile.h /c -### -- %S/Inputs/file.prof 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-NoSourceTP %s
 // CHECK-NoSourceTP: cc1
 // CHECK-NoSourceTP: -emit-pch
 // CHECK-NoSourceTP: -o
 // CHECK-NoSourceTP: pchfile.pch
 // CHECK-NoSourceTP: -x
 // CHECK-NoSourceTP: "c++"
+
+// If only preprocessing, PCH options are ignored.
+// RUN: %clang_cl /P /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-YC-P %s
+// CHECK-YC-P-NOT: -emit-pch
+// CHECK-YC-P-NOT: -include-pch
+
+// RUN: %clang_cl /E /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-YC-E %s
+// CHECK-YC-E-NOT: -emit-pch
+// CHECK-YC-E-NOT: -include-pch
+
+// RUN: %clang_cl /P /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-YU-P %s
+// CHECK-YU-P-NOT: -emit-pch
+// CHECK-YU-P-NOT: -include-pch
+
+// RUN: %clang_cl /E /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-YU-E %s
+// CHECK-YU-E-NOT: -emit-pch
+// CHECK-YU-E-NOT: -include-pch