jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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 this doesn't work
quite well. 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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D118986
Files:
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: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===================================================================
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -296,14 +296,13 @@
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 @@
{"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 @@
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;
Index: clang/test/ClangScanDeps/modules-symlink.c
===================================================================
--- clang/test/ClangScanDeps/modules-symlink.c
+++ clang/test/ClangScanDeps/modules-symlink.c
@@ -49,8 +49,7 @@
// 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: clang/test/ClangScanDeps/modules-pch.c
===================================================================
--- clang/test/ClangScanDeps/modules-pch.c
+++ 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
Index: clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
===================================================================
--- clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
+++ 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
Index: clang/test/ClangScanDeps/modules-pch-common-submodule.c
===================================================================
--- clang/test/ClangScanDeps/modules-pch-common-submodule.c
+++ 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
Index: clang/test/ClangScanDeps/modules-inferred.m
===================================================================
--- clang/test/ClangScanDeps/modules-inferred.m
+++ clang/test/ClangScanDeps/modules-inferred.m
@@ -47,7 +47,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/[[HASH_INFERRED]]/Inferred-{{[A-Z0-9]+}}.pcm"
// CHECK-NEXT: ],
Index: clang/test/ClangScanDeps/modules-inferred-explicit-build.m
===================================================================
--- clang/test/ClangScanDeps/modules-inferred-explicit-build.m
+++ clang/test/ClangScanDeps/modules-inferred-explicit-build.m
@@ -10,11 +10,9 @@
// RUN: %python %S/../../utils/module-deps-to-rsp.py %t.db --module-name=Inferred > %t.inferred.cc1.rsp
// RUN: %python %S/../../utils/module-deps-to-rsp.py %t.db --module-name=System > %t.system.cc1.rsp
// 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.inferred.cc1.rsp
+// RUN: %clang @%t.system.cc1.rsp
+// RUN: %clang @%t.tu.rsp -Wno-unused-command-line-argument
#include <Inferred/Inferred.h>
#include <System/System.h>
Index: clang/test/ClangScanDeps/modules-full.cpp
===================================================================
--- clang/test/ClangScanDeps/modules-full.cpp
+++ 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"
Index: clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
===================================================================
--- clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
+++ 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: ],
Index: clang/test/ClangScanDeps/modules-context-hash.c
===================================================================
--- clang/test/ClangScanDeps/modules-context-hash.c
+++ 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": [
Index: clang/test/ClangScanDeps/diagnostics.c
===================================================================
--- clang/test/ClangScanDeps/diagnostics.c
+++ 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": [
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -13,10 +13,10 @@
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,11 +29,13 @@
}
std::vector<std::string>
-FullDependencies::getAdditionalArgsWithoutModulePaths() const {
- std::vector<std::string> Args{
- "-fno-implicit-modules",
- "-fno-implicit-module-maps",
- };
+FullDependencies::getCommandLineWithoutModulePaths() const {
+ std::vector<std::string> Args = OriginalCommandLine;
+
+ // TODO: Filter out implicit modules leftovers (e.g. "-fmodules-cache-path=").
+
+ Args.push_back("-fno-implicit-modules");
+ Args.push_back("-fno-implicit-module-maps");
for (const PrebuiltModuleDep &PMD : PrebuiltModuleDeps)
Args.push_back("-fmodule-file=" + PMD.PCMFile);
@@ -138,9 +140,13 @@
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 +187,7 @@
Worker.computeDependencies(CWD, CommandLine, Consumer, ModuleName);
if (Result)
return std::move(Result);
- return Consumer.getFullDependencies();
+ return Consumer.getFullDependencies(CommandLine);
}
} // end namespace dependencies
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -42,8 +42,11 @@
/// 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.
+ /// Get the full command line of the original TU, excluding the compiler
+ /// executable.
+ std::vector<std::string> OriginalCommandLine;
+
+ /// Get full command line.
///
/// \param LookupPCMPath This function is called to fill in "-fmodule-file="
/// arguments and the "-o" argument. It needs to return
@@ -53,14 +56,13 @@
/// transitive set of dependencies for this
/// compilation and fill in "-fmodule-map-file="
/// arguments.
- std::vector<std::string> getAdditionalArgs(
+ std::vector<std::string> getCommandLine(
std::function<StringRef(ModuleID)> LookupPCMPath,
std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const;
- /// Get additional arguments suitable for appending to the original Clang
- /// command line, excluding arguments containing modules-related paths:
- /// "-fmodule-file=", "-fmodule-map-file=".
- std::vector<std::string> getAdditionalArgsWithoutModulePaths() const;
+ /// Get the full command line of the original TU, excluding the compiler
+ /// executable and "-fmodule-file=" arguments.
+ std::vector<std::string> getCommandLineWithoutModulePaths() const;
};
struct FullDependenciesResult {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
