From 19017c2435d76fe453a2500eeafd045ba92ece67 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Wed, 23 Feb 2022 14:51:40 +0100 Subject: [PATCH] [clang][deps] Return the whole TU command line 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 --- .../DependencyScanning/DependencyScanningTool.h | 11 +++++++--- .../DependencyScanning/DependencyScanningTool.cpp | 25 ++++++++++++++++++---- clang/test/ClangScanDeps/diagnostics.c | 2 +- clang/test/ClangScanDeps/modules-context-hash.c | 4 ++-- .../modules-fmodule-name-no-module-built.m | 2 +- clang/test/ClangScanDeps/modules-full.cpp | 8 +++---- .../modules-inferred-explicit-build.m | 4 +--- clang/test/ClangScanDeps/modules-inferred.m | 2 +- .../ClangScanDeps/modules-pch-common-submodule.c | 10 ++++----- .../modules-pch-common-via-submodule.c | 10 ++++----- clang/test/ClangScanDeps/modules-pch.c | 15 ++++++------- clang/test/ClangScanDeps/modules-symlink.c | 3 +-- clang/tools/clang-scan-deps/ClangScanDeps.cpp | 19 ++++++++-------- 13 files changed, 63 insertions(+), 52 deletions(-) diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h index 54c3c95..36447dd 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h @@ -42,8 +42,10 @@ struct FullDependencies { /// determined that the differences are benign for this compilation. std::vector 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 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 getAdditionalArgs( + std::vector getCommandLine( std::function LookupPCMPath, std::function LookupModuleDeps) const; + /// Get the full command line, excluding -fmodule-file=" arguments. + std::vector getCommandLineWithoutModulePaths() const; + /// Get additional arguments suitable for appending to the original Clang /// command line, excluding "-fmodule-file=" arguments. std::vector getAdditionalArgsWithoutModulePaths() const; diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp index 26f9196..2723d47 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp @@ -13,10 +13,10 @@ namespace clang { namespace tooling { namespace dependencies { -std::vector FullDependencies::getAdditionalArgs( +std::vector FullDependencies::getCommandLine( std::function LookupPCMPath, std::function LookupModuleDeps) const { - std::vector Ret = getAdditionalArgsWithoutModulePaths(); + std::vector Ret = getCommandLineWithoutModulePaths(); std::vector PCMPaths; std::vector ModMapPaths; @@ -29,6 +29,19 @@ std::vector FullDependencies::getAdditionalArgs( } std::vector +FullDependencies::getCommandLineWithoutModulePaths() const { + std::vector Args = OriginalCommandLine; + + std::vector 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 FullDependencies::getAdditionalArgsWithoutModulePaths() const { std::vector Args{ "-fno-implicit-modules", @@ -138,9 +151,13 @@ DependencyScanningTool::getFullDependencies( ContextHash = std::move(Hash); } - FullDependenciesResult getFullDependencies() const { + FullDependenciesResult getFullDependencies( + const std::vector &OriginalCommandLine) const { FullDependencies FD; + FD.OriginalCommandLine = + ArrayRef(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 diff --git a/clang/test/ClangScanDeps/diagnostics.c b/clang/test/ClangScanDeps/diagnostics.c index ce4eff7..0dcac47 100644 --- a/clang/test/ClangScanDeps/diagnostics.c +++ b/clang/test/ClangScanDeps/diagnostics.c @@ -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": [ diff --git a/clang/test/ClangScanDeps/modules-context-hash.c b/clang/test/ClangScanDeps/modules-context-hash.c index dfa3328..10c7d18 100644 --- a/clang/test/ClangScanDeps/modules-context-hash.c +++ b/clang/test/ClangScanDeps/modules-context-hash.c @@ -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": [ diff --git a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m index f9d4d89..8469b49 100644 --- a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m +++ b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m @@ -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: ], diff --git a/clang/test/ClangScanDeps/modules-full.cpp b/clang/test/ClangScanDeps/modules-full.cpp index d7cdf4c..2a7b662 100644 --- a/clang/test/ClangScanDeps/modules-full.cpp +++ b/clang/test/ClangScanDeps/modules-full.cpp @@ -103,7 +103,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-NO-ABS-NOT: "-fmodule-file={{.*}}" // CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm" @@ -123,7 +123,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-NO-ABS-NOT: "-fmodule-file={{.*}}, // CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm" @@ -143,7 +143,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-NO-ABS-NOT: "-fmodule-file={{.*}}" // CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm" @@ -163,7 +163,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-NO-ABS-NOT: "-fmodule-file={{.*}}" // CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm" diff --git a/clang/test/ClangScanDeps/modules-inferred-explicit-build.m b/clang/test/ClangScanDeps/modules-inferred-explicit-build.m index 4471eb3..09101ff 100644 --- a/clang/test/ClangScanDeps/modules-inferred-explicit-build.m +++ b/clang/test/ClangScanDeps/modules-inferred-explicit-build.m @@ -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 #include diff --git a/clang/test/ClangScanDeps/modules-inferred.m b/clang/test/ClangScanDeps/modules-inferred.m index 15e7aa3..3fdf4ed 100644 --- a/clang/test/ClangScanDeps/modules-inferred.m +++ b/clang/test/ClangScanDeps/modules-inferred.m @@ -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: ], diff --git a/clang/test/ClangScanDeps/modules-pch-common-submodule.c b/clang/test/ClangScanDeps/modules-pch-common-submodule.c index c7f3e76..574100d 100644 --- a/clang/test/ClangScanDeps/modules-pch-common-submodule.c +++ b/clang/test/ClangScanDeps/modules-pch-common-submodule.c @@ -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: // @@ -115,7 +114,7 @@ // 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: ], @@ -137,5 +136,4 @@ // 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 diff --git a/clang/test/ClangScanDeps/modules-pch-common-via-submodule.c b/clang/test/ClangScanDeps/modules-pch-common-via-submodule.c index e63e310..4d1a702 100644 --- a/clang/test/ClangScanDeps/modules-pch-common-via-submodule.c +++ b/clang/test/ClangScanDeps/modules-pch-common-via-submodule.c @@ -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: // @@ -113,7 +112,7 @@ // 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: ], @@ -135,5 +134,4 @@ // 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 diff --git a/clang/test/ClangScanDeps/modules-pch.c b/clang/test/ClangScanDeps/modules-pch.c index 2be1774..89b6b6f 100644 --- a/clang/test/ClangScanDeps/modules-pch.c +++ b/clang/test/ClangScanDeps/modules-pch.c @@ -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", @@ -120,8 +120,7 @@ // 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: // @@ -161,7 +160,7 @@ // 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: ], @@ -183,8 +182,7 @@ // 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: // @@ -225,7 +223,7 @@ // 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" @@ -248,5 +246,4 @@ // 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 diff --git a/clang/test/ClangScanDeps/modules-symlink.c b/clang/test/ClangScanDeps/modules-symlink.c index 5b62817..14bc811 100644 --- a/clang/test/ClangScanDeps/modules-symlink.c +++ b/clang/test/ClangScanDeps/modules-symlink.c @@ -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 \ diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index 49cc97b..e88290d 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -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 FileDeps; std::vector ModuleDeps; - std::vector AdditionalCommandLine; + std::vector CommandLine; }; std::mutex Lock; -- 2.7.4