[clang][deps] Return the whole TU command line
authorJan Svoboda <jan_svoboda@apple.com>
Wed, 23 Feb 2022 13:51:40 +0000 (14:51 +0100)
committerJan Svoboda <jan_svoboda@apple.com>
Wed, 23 Feb 2022 14:46:20 +0000 (15:46 +0100)
The dependency scanner already generates canonical -cc1 command lines that can be used to compile discovered modular dependencies.

For translation unit command lines, the scanner only generates additional driver arguments the build system is expected to append to the original command line.

While this works most of the time, there are situations where that's not the case. For example with `-Wunused-command-line-argument`, Clang will complain about the `-fmodules-cache-path=` argument that's not being used in explicit modular builds. Combine that with `-Werror` and the build outright fails.

To prevent such failures, this patch changes the dependency scanner to return the full driver command line to compile the original translation unit. This gives us more opportunities to massage the arguments into something reasonable.

Reviewed By: Bigcheese

Differential Revision: https://reviews.llvm.org/D118986

13 files changed:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/test/ClangScanDeps/diagnostics.c
clang/test/ClangScanDeps/modules-context-hash.c
clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
clang/test/ClangScanDeps/modules-full.cpp
clang/test/ClangScanDeps/modules-inferred-explicit-build.m
clang/test/ClangScanDeps/modules-inferred.m
clang/test/ClangScanDeps/modules-pch-common-submodule.c
clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
clang/test/ClangScanDeps/modules-pch.c
clang/test/ClangScanDeps/modules-symlink.c
clang/tools/clang-scan-deps/ClangScanDeps.cpp

index 54c3c95..36447dd 100644 (file)
@@ -42,8 +42,10 @@ struct FullDependencies {
   /// determined that the differences are benign for this compilation.
   std::vector<ModuleID> ClangModuleDeps;
 
-  /// Get additional arguments suitable for appending to the original Clang
-  /// command line.
+  /// The original command line of the TU (excluding the compiler executable).
+  std::vector<std::string> OriginalCommandLine;
+
+  /// Get the full command line.
   ///
   /// \param LookupPCMPath This function is called to fill in "-fmodule-file="
   ///                      arguments and the "-o" argument. It needs to return
@@ -52,10 +54,13 @@ struct FullDependencies {
   /// \param LookupModuleDeps This function is called to collect the full
   ///                         transitive set of dependencies for this
   ///                         compilation.
-  std::vector<std::string> getAdditionalArgs(
+  std::vector<std::string> getCommandLine(
       std::function<StringRef(ModuleID)> LookupPCMPath,
       std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const;
 
+  /// Get the full command line, excluding -fmodule-file=" arguments.
+  std::vector<std::string> getCommandLineWithoutModulePaths() const;
+
   /// Get additional arguments suitable for appending to the original Clang
   /// command line, excluding "-fmodule-file=" arguments.
   std::vector<std::string> getAdditionalArgsWithoutModulePaths() const;
index 26f9196..2723d47 100644 (file)
@@ -13,10 +13,10 @@ namespace clang {
 namespace tooling {
 namespace dependencies {
 
-std::vector<std::string> FullDependencies::getAdditionalArgs(
+std::vector<std::string> FullDependencies::getCommandLine(
     std::function<StringRef(ModuleID)> LookupPCMPath,
     std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const {
-  std::vector<std::string> Ret = getAdditionalArgsWithoutModulePaths();
+  std::vector<std::string> Ret = getCommandLineWithoutModulePaths();
 
   std::vector<std::string> PCMPaths;
   std::vector<std::string> ModMapPaths;
@@ -29,6 +29,19 @@ std::vector<std::string> FullDependencies::getAdditionalArgs(
 }
 
 std::vector<std::string>
+FullDependencies::getCommandLineWithoutModulePaths() const {
+  std::vector<std::string> Args = OriginalCommandLine;
+
+  std::vector<std::string> AdditionalArgs =
+      getAdditionalArgsWithoutModulePaths();
+  Args.insert(Args.end(), AdditionalArgs.begin(), AdditionalArgs.end());
+
+  // TODO: Filter out implicit modules leftovers (e.g. "-fmodules-cache-path=").
+
+  return Args;
+}
+
+std::vector<std::string>
 FullDependencies::getAdditionalArgsWithoutModulePaths() const {
   std::vector<std::string> Args{
       "-fno-implicit-modules",
@@ -138,9 +151,13 @@ DependencyScanningTool::getFullDependencies(
       ContextHash = std::move(Hash);
     }
 
-    FullDependenciesResult getFullDependencies() const {
+    FullDependenciesResult getFullDependencies(
+        const std::vector<std::string> &OriginalCommandLine) const {
       FullDependencies FD;
 
+      FD.OriginalCommandLine =
+          ArrayRef<std::string>(OriginalCommandLine).slice(1);
+
       FD.ID.ContextHash = std::move(ContextHash);
 
       FD.FileDeps.assign(Dependencies.begin(), Dependencies.end());
@@ -181,7 +198,7 @@ DependencyScanningTool::getFullDependencies(
       Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
   if (Result)
     return std::move(Result);
-  return Consumer.getFullDependencies();
+  return Consumer.getFullDependencies(CommandLine);
 }
 
 } // end namespace dependencies
index ce4eff7..0dcac47 100644 (file)
@@ -38,7 +38,7 @@
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "command-line": [
-// CHECK-NEXT:         "-fno-implicit-modules"
+// CHECK:              "-fno-implicit-modules"
 // CHECK-NEXT:         "-fno-implicit-module-maps"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "file-deps": [
index dfa3328..10c7d18 100644 (file)
@@ -50,7 +50,7 @@
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "command-line": [
-// CHECK-NEXT:         "-fno-implicit-modules",
+// CHECK:              "-fno-implicit-modules",
 // CHECK-NEXT:         "-fno-implicit-module-maps"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "file-deps": [
@@ -91,7 +91,7 @@
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "command-line": [
-// CHECK-NEXT:         "-fno-implicit-modules",
+// CHECK:              "-fno-implicit-modules",
 // CHECK-NEXT:         "-fno-implicit-module-maps"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "file-deps": [
index f9d4d89..8469b49 100644 (file)
@@ -43,7 +43,7 @@
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "command-line": [
-// CHECK-NEXT:         "-fno-implicit-modules"
+// CHECK:              "-fno-implicit-modules"
 // CHECK-NEXT:         "-fno-implicit-module-maps"
 // CHECK-NEXT:         "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2]]/header2-{{[A-Z0-9]+}}.pcm"
 // CHECK-NEXT:       ],
index d7cdf4c..2a7b662 100644 (file)
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "command-line": [
-// CHECK-NEXT:         "-fno-implicit-modules"
+// CHECK:              "-fno-implicit-modules"
 // CHECK-NEXT:         "-fno-implicit-module-maps"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
 // CHECK-ABS-NEXT:     "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "command-line": [
-// CHECK-NEXT:         "-fno-implicit-modules"
+// CHECK:              "-fno-implicit-modules"
 // CHECK-NEXT:         "-fno-implicit-module-maps"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}},
 // CHECK-ABS-NEXT:     "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "command-line": [
-// CHECK-NEXT:         "-fno-implicit-modules"
+// CHECK:              "-fno-implicit-modules"
 // CHECK-NEXT:         "-fno-implicit-module-maps"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
 // CHECK-ABS-NEXT:     "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "command-line": [
-// CHECK-NEXT:         "-fno-implicit-modules"
+// CHECK:              "-fno-implicit-modules"
 // CHECK-NEXT:         "-fno-implicit-module-maps"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
 // CHECK-ABS-NEXT:     "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm"
index 4471eb3..09101ff 100644 (file)
@@ -12,9 +12,7 @@
 // RUN: %python %S/../../utils/module-deps-to-rsp.py %t.db --tu-index=0 > %t.tu.rsp
 // RUN: %clang @%t.inferred.cc1.rsp -pedantic -Werror
 // RUN: %clang @%t.system.cc1.rsp -pedantic -Werror
-// RUN: %clang -x objective-c -fsyntax-only %t.dir/modules_cdb_input.cpp \
-// RUN:   -F%S/Inputs/frameworks -fmodules -fimplicit-module-maps \
-// RUN:   -pedantic -Werror @%t.tu.rsp
+// RUN: %clang @%t.tu.rsp -pedantic -Werror -Wno-unused-command-line-argument
 
 #include <Inferred/Inferred.h>
 #include <System/System.h>
index 15e7aa3..3fdf4ed 100644 (file)
@@ -47,7 +47,7 @@ inferred a = 0;
 // CHECK-NEXT:         }
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "command-line": [
-// CHECK-NEXT:         "-fno-implicit-modules",
+// CHECK:              "-fno-implicit-modules",
 // CHECK-NEXT:         "-fno-implicit-module-maps",
 // CHECK-NEXT:         "-fmodule-file=[[PREFIX]]/module-cache/[[HASH_INFERRED]]/Inferred-{{[A-Z0-9]+}}.pcm"
 // CHECK-NEXT:       ],
index c7f3e76..574100d 100644 (file)
@@ -51,7 +51,7 @@
 // CHECK-PCH-NEXT:         }
 // CHECK-PCH-NEXT:       ],
 // CHECK-PCH-NEXT:       "command-line": [
-// CHECK-PCH-NEXT:         "-fno-implicit-modules"
+// CHECK-PCH:              "-fno-implicit-modules"
 // CHECK-PCH-NEXT:         "-fno-implicit-module-maps"
 // CHECK-PCH-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON]]/ModCommon-{{.*}}.pcm"
 // CHECK-PCH-NEXT:       ],
@@ -72,8 +72,7 @@
 // RUN:   --tu-index=0 > %t/pch.rsp
 //
 // RUN: %clang @%t/mod_common.cc1.rsp
-// RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t/cache -o %t/pch.h.gch @%t/pch.rsp
+// RUN: %clang @%t/pch.rsp
 
 // Scan dependencies of the TU:
 //
 // CHECK-TU-NEXT:         }
 // CHECK-TU-NEXT:       ],
 // CHECK-TU-NEXT:       "command-line": [
-// CHECK-TU-NEXT:         "-fno-implicit-modules",
+// CHECK-TU:              "-fno-implicit-modules",
 // CHECK-TU-NEXT:         "-fno-implicit-module-maps",
 // CHECK-TU-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU:.*]]/ModTU-{{.*}}.pcm"
 // CHECK-TU-NEXT:       ],
 // RUN:   --tu-index=0 > %t/tu.rsp
 //
 // RUN: %clang @%t/mod_tu.cc1.rsp
-// RUN: %clang -fsyntax-only %t/tu.c -fmodules -gmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t/cache -include %t/pch.h -o %t/tu.o @%t/tu.rsp
+// RUN: %clang @%t/tu.rsp
index e63e310..4d1a702 100644 (file)
@@ -48,7 +48,7 @@
 // CHECK-PCH-NEXT:         }
 // CHECK-PCH-NEXT:       ],
 // CHECK-PCH-NEXT:       "command-line": [
-// CHECK-PCH-NEXT:         "-fno-implicit-modules"
+// CHECK-PCH:              "-fno-implicit-modules"
 // CHECK-PCH-NEXT:         "-fno-implicit-module-maps"
 // CHECK-PCH-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON]]/ModCommon-{{.*}}.pcm"
 // CHECK-PCH-NEXT:       ],
@@ -69,8 +69,7 @@
 // RUN:   --tu-index=0 > %t/pch.rsp
 //
 // RUN: %clang @%t/mod_common.cc1.rsp
-// RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t/cache -o %t/pch.h.gch @%t/pch.rsp
+// RUN: %clang @%t/pch.rsp
 
 // Scan dependencies of the TU:
 //
 // CHECK-TU-NEXT:         }
 // CHECK-TU-NEXT:       ],
 // CHECK-TU-NEXT:       "command-line": [
-// CHECK-TU-NEXT:         "-fno-implicit-modules",
+// CHECK-TU:              "-fno-implicit-modules",
 // CHECK-TU-NEXT:         "-fno-implicit-module-maps",
 // CHECK-TU-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU:.*]]/ModTU-{{.*}}.pcm"
 // CHECK-TU-NEXT:       ],
 // RUN:   --tu-index=0 > %t/tu.rsp
 //
 // RUN: %clang @%t/mod_tu.cc1.rsp
-// RUN: %clang -fsyntax-only %t/tu.c -fmodules -gmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t/cache -include %t/pch.h -o %t/tu.o @%t/tu.rsp
+// RUN: %clang @%t/tu.rsp
index 2be1774..89b6b6f 100644 (file)
@@ -91,7 +91,7 @@
 // CHECK-PCH-NEXT:         }
 // CHECK-PCH-NEXT:       ],
 // CHECK-PCH-NEXT:       "command-line": [
-// CHECK-PCH-NEXT:         "-fno-implicit-modules",
+// CHECK-PCH:              "-fno-implicit-modules",
 // CHECK-PCH-NEXT:         "-fno-implicit-module-maps",
 // CHECK-PCH-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_1]]/ModCommon1-{{.*}}.pcm",
 // CHECK-PCH-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_2]]/ModCommon2-{{.*}}.pcm",
 // RUN: %clang @%t/mod_common_1.cc1.rsp
 // RUN: %clang @%t/mod_common_2.cc1.rsp
 // RUN: %clang @%t/mod_pch.cc1.rsp
-// RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t/cache -o %t/pch.h.gch @%t/pch.rsp
+// RUN: %clang @%t/pch.rsp
 
 // Scan dependencies of the TU:
 //
 // CHECK-TU-NEXT:         }
 // CHECK-TU-NEXT:       ],
 // CHECK-TU-NEXT:       "command-line": [
-// CHECK-TU-NEXT:         "-fno-implicit-modules",
+// CHECK-TU:              "-fno-implicit-modules",
 // CHECK-TU-NEXT:         "-fno-implicit-module-maps",
 // CHECK-TU-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU]]/ModTU-{{.*}}.pcm"
 // CHECK-TU-NEXT:       ],
 // RUN:   --tu-index=0 > %t/tu.rsp
 //
 // RUN: %clang @%t/mod_tu.cc1.rsp
-// RUN: %clang -fsyntax-only %t/tu.c -fmodules -gmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t/cache -include %t/pch.h -o %t/tu.o @%t/tu.rsp
+// RUN: %clang @%t/tu.rsp
 
 // Scan dependencies of the TU that has common modules with the PCH:
 //
 // CHECK-TU-WITH-COMMON-NEXT:         }
 // CHECK-TU-WITH-COMMON-NEXT:       ],
 // CHECK-TU-WITH-COMMON-NEXT:       "command-line": [
-// CHECK-TU-WITH-COMMON-NEXT:         "-fno-implicit-modules",
+// CHECK-TU-WITH-COMMON:              "-fno-implicit-modules",
 // CHECK-TU-WITH-COMMON-NEXT:         "-fno-implicit-module-maps",
 // CHECK-TU-WITH-COMMON-NEXT:         "-fmodule-file=[[PREFIX]]/build/{{.*}}/ModCommon2-{{.*}}.pcm",
 // CHECK-TU-WITH-COMMON-NEXT:         "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU_WITH_COMMON]]/ModTUWithCommon-{{.*}}.pcm"
 // RUN:   --tu-index=0 > %t/tu_with_common.rsp
 //
 // RUN: %clang @%t/mod_tu_with_common.cc1.rsp
-// RUN: %clang -fsyntax-only %t/tu_with_common.c -fmodules -gmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t/cache -include %t/pch.h -o %t/tu_with_common.o @%t/tu_with_common.rsp
+// RUN: %clang @%t/tu_with_common.rsp
index 5b62817..14bc811 100644 (file)
@@ -49,8 +49,7 @@ static int foo = MACRO; // Macro usage that will trigger
 // RUN:   --tu-index=0 > %t/pch.rsp
 //
 // RUN: %clang @%t/mod.cc1.rsp
-// RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \
-// RUN:   -fmodules-cache-path=%t/cache -o %t/pch.h.gch -I %t @%t/pch.rsp
+// RUN: %clang @%t/pch.rsp
 
 // RUN: sed -e "s|DIR|%/t|g" %t/cdb_tu.json > %t/cdb.json
 // RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
index 49cc97b..e88290d 100644 (file)
@@ -296,14 +296,13 @@ public:
       Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
     }
 
-    ID.AdditionalCommandLine =
-        GenerateModulesPathArgs
-            ? FD.getAdditionalArgs(
-                  [&](ModuleID MID) { return lookupPCMPath(MID); },
-                  [&](ModuleID MID) -> const ModuleDeps & {
-                    return lookupModuleDeps(MID);
-                  })
-            : FD.getAdditionalArgsWithoutModulePaths();
+    ID.CommandLine = GenerateModulesPathArgs
+                         ? FD.getCommandLine(
+                               [&](ModuleID MID) { return lookupPCMPath(MID); },
+                               [&](ModuleID MID) -> const ModuleDeps & {
+                                 return lookupModuleDeps(MID);
+                               })
+                         : FD.getCommandLineWithoutModulePaths();
 
     Inputs.push_back(std::move(ID));
   }
@@ -353,7 +352,7 @@ public:
           {"clang-context-hash", I.ContextHash},
           {"file-deps", I.FileDeps},
           {"clang-module-deps", toJSONSorted(I.ModuleDeps)},
-          {"command-line", I.AdditionalCommandLine},
+          {"command-line", I.CommandLine},
       };
       TUs.push_back(std::move(O));
     }
@@ -415,7 +414,7 @@ private:
     std::string ContextHash;
     std::vector<std::string> FileDeps;
     std::vector<ModuleID> ModuleDeps;
-    std::vector<std::string> AdditionalCommandLine;
+    std::vector<std::string> CommandLine;
   };
 
   std::mutex Lock;