fix: Handle -Yc without filepath for msvc (#1448)

This PR further fixes the #1384. Fixes the case when the `/Yc` option is
without the filepath and the `/Fppathname` is also defined.

The `/Yc` can be passed in two ways with and w/o the filepath. When it's
passed w/o the filepath it must take the value of the `/Fp` option and
if the `/Fp` option isn't defined then the resulting .pch file must have
the same base name as the base source file with appended `.pch`
extension (this case isn't handled correctly by ccache).

The `/Yc` option doesn't support passing a filepath with the space
between like `/Yc filepath`. Because of this the `TAKES_ARG` must be
removed. All occurrences of `compopt_takes_path()` can't be invoked for

I have also added a new unittest for this case: "MSVC PCH options with
empty -Yc" and also tested it manually on my example project. Just now,
I have also tested it on a big project with 200 TU with qmake and cmake
build systems with msvc and also clang-cl with msvc compilers with 100%
cache hits (PCH enabled and /Zi replaced with /Z7).

diff --git a/src/ccache/argprocessing.cpp b/src/ccache/argprocessing.cpp
index ef08d134de5502d7854023d0736a4b688a538701..e0f1462ef15fe11a82c52f3dcbf01275eb482ca5 100644 (file)
--- a/src/ccache/argprocessing.cpp
+++ b/src/ccache/argprocessing.cpp
@@ -1090,7 +1090,7 @@ process_option_arg(const Context& ctx,
     return Statistic::none;
   }
 
-  if (compopt_takes_path(arg)) {
+  if (compopt_takes_arg(arg) && compopt_takes_path(arg)) {
     if (i == args.size() - 1) {
       LOG("Missing argument to {}", args[i]);
       return Statistic::bad_compiler_arguments;

diff --git a/src/ccache/compopt.cpp b/src/ccache/compopt.cpp
index ee4dcb7fb2f0f1f34fc3adec1e4299b8eab01c6f..7a810d5c0f50098d7edd56f272753ee66f8ccb7f 100644 (file)
--- a/src/ccache/compopt.cpp
+++ b/src/ccache/compopt.cpp
@@ -97,7 +97,7 @@ const CompOpt compopts[] = {
   {"-Xcompiler", AFFECTS_CPP | TAKES_ARG}, // nvcc
   {"-Xlinker", TAKES_ARG | TAKES_CONCAT_ARG | AFFECTS_COMP},
   {"-Xpreprocessor", AFFECTS_CPP | TAKES_ARG},
-  {"-Yc", AFFECTS_CPP | TAKES_ARG | TAKES_CONCAT_ARG | TAKES_PATH}, // msvc
+  {"-Yc", AFFECTS_CPP | TAKES_CONCAT_ARG | TAKES_PATH},             // msvc
   {"-Yu", AFFECTS_CPP | TAKES_ARG | TAKES_CONCAT_ARG | TAKES_PATH}, // msvc
   {"-all_load", AFFECTS_COMP},
   {"-analyze", TOO_HARD}, // Clang

diff --git a/unittest/test_argprocessing.cpp b/unittest/test_argprocessing.cpp
index 3de398dd630c6ec4e28a48109cb0e30b6fce1215..3a20a314cd078fb1a5119bf1ac6ce5a9faede58b 100644 (file)
--- a/unittest/test_argprocessing.cpp
+++ b/unittest/test_argprocessing.cpp
@@ -764,6 +764,49 @@ TEST_CASE("MSVC PCH options")
   }
 }
 
+TEST_CASE("MSVC PCH options with empty -Yc")
+{
+  TestContext test_context;
+  Context ctx;
+  ctx.config.set_compiler_type(CompilerType::msvc);
+  util::write_file("foo.cpp", "");
+  util::write_file("pch.h", "");
+  util::write_file("pch.cpp", "");
+
+  SUBCASE("Create PCH")
+  {
+    ctx.orig_args = Args::from_string(
+      "cl.exe /Yc /Fppch.cpp.pch /FIpch.h /Fopch.cpp.obj /c pch.cpp");
+    const ProcessArgsResult result = process_args(ctx);
+    REQUIRE(!result.error);
+    CHECK(ctx.args_info.generating_pch);
+    CHECK(ctx.args_info.included_pch_file == "pch.cpp.pch");
+    CHECK(ctx.args_info.output_obj == "pch.cpp.obj");
+    CHECK(result.preprocessor_args.to_string()
+          == "cl.exe /Yc /Fppch.cpp.pch /FIpch.h");
+    CHECK(result.compiler_args.to_string()
+          == "cl.exe /Yc /Fppch.cpp.pch /FIpch.h -c");
+  }
+
+  util::write_file("pch.cpp.pch", "");
+  ctx.config.update_from_map({{"sloppiness", "pch_defines,time_macros"}});
+
+  SUBCASE("Consume PCH")
+  {
+    ctx.orig_args = Args::from_string(
+      "cl.exe /Yupch.h /Fppch.cpp.pch /FIpch.h /Fofoo.cpp.obj /c foo.cpp");
+    const ProcessArgsResult result = process_args(ctx);
+    REQUIRE(!result.error);
+    CHECK(!ctx.args_info.generating_pch);
+    CHECK(ctx.args_info.included_pch_file == "pch.cpp.pch");
+    CHECK(ctx.args_info.output_obj == "foo.cpp.obj");
+    CHECK(result.preprocessor_args.to_string()
+          == "cl.exe /Yupch.h /Fppch.cpp.pch /FIpch.h");
+    CHECK(result.compiler_args.to_string()
+          == "cl.exe /Yupch.h /Fppch.cpp.pch /FIpch.h -c");
+  }
+}
+
 TEST_CASE("MSVC debug information format options")
 {
   TestContext test_context;