[PATCH] D50640: Fix for bug 38508 - Don't do PCH processing when only generating preprocessor output
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
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
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
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
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
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
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