From: Silver Zachara <silver.zachara@gmail.com>
Date: Wed, 15 May 2024 18:44:21 +0000 (+0200)
Subject: fix: Handle -Yc without filepath for msvc (#1448)
X-Git-Tag: v4.10~21
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d04c6f3b7338f2780b0579359cc0beb785f1edd9;p=thirdparty%2Fccache.git

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 ef08d134..e0f1462e 100644
--- 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 ee4dcb7f..7a810d5c 100644
--- 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 3de398dd..3a20a314 100644
--- 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;