[PATCH] D50640: Fix for bug 38508 - Don't do PCH processing when only generating preprocessor output

2018-08-17 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340025: Fix for bug 38508 - Dont do PCH processing 
when only generating preprocessor… (authored by erichkeane, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50640?vs=161116=161229#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50640

Files:
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/test/Driver/cl-pch.cpp
  cfe/trunk/test/PCH/Inputs/pch-through-use3c.cpp
  cfe/trunk/test/PCH/Inputs/pch-through3c.h
  cfe/trunk/test/PCH/pch-through3c.cpp


Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -3010,9 +3010,10 @@
 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);
Index: cfe/trunk/test/Driver/cl-pch.cpp
===
--- cfe/trunk/test/Driver/cl-pch.cpp
+++ cfe/trunk/test/Driver/cl-pch.cpp
@@ -345,3 +345,24 @@
 // 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
Index: cfe/trunk/test/PCH/Inputs/pch-through-use3c.cpp
===
--- cfe/trunk/test/PCH/Inputs/pch-through-use3c.cpp
+++ cfe/trunk/test/PCH/Inputs/pch-through-use3c.cpp
@@ -0,0 +1,2 @@
+int a = A;
+// expected-no-diagnostics
Index: cfe/trunk/test/PCH/Inputs/pch-through3c.h
===
--- cfe/trunk/test/PCH/Inputs/pch-through3c.h
+++ cfe/trunk/test/PCH/Inputs/pch-through3c.h
@@ -0,0 +1 @@
+#define A 1
Index: cfe/trunk/test/PCH/pch-through3c.cpp
===
--- cfe/trunk/test/PCH/pch-through3c.cpp
+++ cfe/trunk/test/PCH/pch-through3c.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -I %S -emit-pch \
+// RUN:   -include Inputs/pch-through3c.h \
+// RUN:   -pch-through-header=Inputs/pch-through3c.h -o %t.3c %s
+
+// RUN: %clang_cc1 -verify -I %S -include-pch %t.3c \
+// RUN:   -include Inputs/pch-through3c.h \
+// RUN:   -pch-through-header=Inputs/pch-through3c.h \
+// RUN:   %S/Inputs/pch-through-use3c.cpp 2>&1


Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -3010,9 +3010,10 @@
 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);
Index: cfe/trunk/test/Driver/cl-pch.cpp
===
--- cfe/trunk/test/Driver/cl-pch.cpp
+++ cfe/trunk/test/Driver/cl-pch.cpp
@@ -345,3 +345,24 @@
 // 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: 

[PATCH] D50640: Fix for bug 38508 - Don't do PCH processing when only generating preprocessor output

2018-08-16 Thread Mike Rice via Phabricator via cfe-commits
mikerice added a comment.

In https://reviews.llvm.org/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!


https://reviews.llvm.org/D50640



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


[PATCH] D50640: Fix for bug 38508 - Don't do PCH processing when only generating preprocessor output

2018-08-16 Thread Nico Weber via Phabricator via cfe-commits
thakis accepted this revision.
thakis added a comment.

Thanks! Do you need someone to land this?


https://reviews.llvm.org/D50640



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


[PATCH] D50640: Fix for bug 38508 - Don't do PCH processing when only generating preprocessor output

2018-08-16 Thread Mike Rice via Phabricator via cfe-commits
mikerice added a comment.

In https://reviews.llvm.org/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.


https://reviews.llvm.org/D50640



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


[PATCH] D50640: Fix for bug 38508 - Don't do PCH processing when only generating preprocessor output

2018-08-16 Thread Mike Rice via Phabricator via cfe-commits
mikerice updated this revision to Diff 161116.
mikerice marked an inline comment as done.
mikerice added a comment.

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


https://reviews.llvm.org/D50640

Files:
  lib/Driver/Driver.cpp
  test/Driver/cl-pch.cpp
  test/PCH/Inputs/pch-through-use3c.cpp
  test/PCH/Inputs/pch-through3c.h
  test/PCH/pch-through3c.cpp


Index: test/PCH/pch-through3c.cpp
===
--- /dev/null
+++ test/PCH/pch-through3c.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -I %S -emit-pch \
+// RUN:   -include Inputs/pch-through3c.h \
+// RUN:   -pch-through-header=Inputs/pch-through3c.h -o %t.3c %s
+
+// RUN: %clang_cc1 -verify -I %S -include-pch %t.3c \
+// RUN:   -include Inputs/pch-through3c.h \
+// RUN:   -pch-through-header=Inputs/pch-through3c.h \
+// RUN:   %S/Inputs/pch-through-use3c.cpp 2>&1
Index: test/PCH/Inputs/pch-through3c.h
===
--- /dev/null
+++ test/PCH/Inputs/pch-through3c.h
@@ -0,0 +1 @@
+#define A 1
Index: test/PCH/Inputs/pch-through-use3c.cpp
===
--- /dev/null
+++ test/PCH/Inputs/pch-through-use3c.cpp
@@ -0,0 +1,2 @@
+int a = A;
+// expected-no-diagnostics
Index: test/Driver/cl-pch.cpp
===
--- test/Driver/cl-pch.cpp
+++ test/Driver/cl-pch.cpp
@@ -345,3 +345,24 @@
 // 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
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2998,9 +2998,10 @@
 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);


Index: test/PCH/pch-through3c.cpp
===
--- /dev/null
+++ test/PCH/pch-through3c.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -I %S -emit-pch \
+// RUN:   -include Inputs/pch-through3c.h \
+// RUN:   -pch-through-header=Inputs/pch-through3c.h -o %t.3c %s
+
+// RUN: %clang_cc1 -verify -I %S -include-pch %t.3c \
+// RUN:   -include Inputs/pch-through3c.h \
+// RUN:   -pch-through-header=Inputs/pch-through3c.h \
+// RUN:   %S/Inputs/pch-through-use3c.cpp 2>&1
Index: test/PCH/Inputs/pch-through3c.h
===
--- /dev/null
+++ test/PCH/Inputs/pch-through3c.h
@@ -0,0 +1 @@
+#define A 1
Index: test/PCH/Inputs/pch-through-use3c.cpp
===
--- /dev/null
+++ test/PCH/Inputs/pch-through-use3c.cpp
@@ -0,0 +1,2 @@
+int a = A;
+// expected-no-diagnostics
Index: test/Driver/cl-pch.cpp
===
--- test/Driver/cl-pch.cpp
+++ test/Driver/cl-pch.cpp
@@ -345,3 +345,24 @@
 // 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 

[PATCH] D50640: Fix for bug 38508 - Don't do PCH processing when only generating preprocessor output

2018-08-15 Thread Nico Weber via Phabricator via cfe-commits
thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.

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?




Comment at: test/Driver/cl-pch.cpp:368
+// CHECK-YU-E-NOT: -emit-pch
+// CHECK-YU-E-NOT: -include-pch

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.


Repository:
  rC Clang

https://reviews.llvm.org/D50640



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


[PATCH] D50640: Fix for bug 38508 - Don't do PCH processing when only generating preprocessor output

2018-08-13 Thread Mike Rice via Phabricator via cfe-commits
mikerice created this revision.
Herald added a subscriber: cfe-commits.

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


Repository:
  rC Clang

https://reviews.llvm.org/D50640

Files:
  lib/Driver/Driver.cpp
  test/Driver/cl-pch.cpp


Index: test/Driver/cl-pch.cpp
===
--- test/Driver/cl-pch.cpp
+++ test/Driver/cl-pch.cpp
@@ -345,3 +345,24 @@
 // 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
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2998,9 +2998,10 @@
 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);


Index: test/Driver/cl-pch.cpp
===
--- test/Driver/cl-pch.cpp
+++ test/Driver/cl-pch.cpp
@@ -345,3 +345,24 @@
 // 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
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2998,9 +2998,10 @@
 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);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits