[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
This commit is contained in:
Jan Svoboda 2022-02-23 14:51:40 +01:00
parent 80a696898c
commit 19017c2435
13 changed files with 63 additions and 52 deletions

View 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;

View 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;
@ -28,6 +28,19 @@ std::vector<std::string> FullDependencies::getAdditionalArgs(
return Ret;
}
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{
@ -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

View 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": [

View 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": [

View 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: ],

View File

@ -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"

View 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>

View 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: ],

View 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:
//
@ -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

View 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:
//
@ -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

View 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",
@ -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

View 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 \

View 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;