[clang] e030ce3 - [Tooling] Handle compilation databases containing commands with double dashes
Alexandre Ganea via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 24 13:01:56 PDT 2021
Author: Janusz Nykiel
Date: 2021-03-24T16:01:47-04:00
New Revision: e030ce3ec790a0017ec789b4f487afec99e1cac9
URL: https://github.com/llvm/llvm-project/commit/e030ce3ec790a0017ec789b4f487afec99e1cac9
DIFF: https://github.com/llvm/llvm-project/commit/e030ce3ec790a0017ec789b4f487afec99e1cac9.diff
LOG: [Tooling] Handle compilation databases containing commands with double dashes
As of CMake commit https://gitlab.kitware.com/cmake/cmake/-/commit/d993ebd4,
which first appeared in CMake 3.19.x series, in the compile commands for
clang-cl, CMake puts `--` before the input file. When operating on such a
database, the `InterpolatingCompilationDatabase` - specifically, the
`TransferableCommand` constructor - does not recognize that pattern and so, does
not strip the input, or the double dash when 'transferring' the compile command.
This results in a incorrect compile command - with the double dash and old input
file left in, and the language options and new input file appended after them,
where they're all treated as inputs, including the language version option.
Test files for some tests have names similar enough to be matched to commands
from the database, e.g.:
`.../path-mappings.test.tmp/server/bar.cpp`
can be matched to:
`.../Driver/ToolChains/BareMetal.cpp`
etc. When that happens, the tool being tested tries to use the matched, and
incorrectly 'transferred' compile command, and fails, reporting errors similar
to:
`error: no such file or directory: '/std:c++14'; did you mean '/std:c++14'? [clang-diagnostic-error]`
This happens in at least 4 tests:
Clang Tools :: clang-tidy/checkers/performance-trivially-destructible.cpp
Clangd :: check-fail.test
Clangd :: check.test
Clangd :: path-mappings.test
The fix for `TransferableCommand` removes the `--` and everything after it when
determining the arguments that apply to the new file. `--` is inserted in the
'transferred' command if the new file name starts with `-` and when operating in
clang-cl mode, also `/`. Additionally, other places in the code known to do
argument adjustment without accounting for the `--` and causing the tests to
fail are fixed as well.
Differential Revision: https://reviews.llvm.org/D98824
Added:
Modified:
clang/lib/Tooling/ArgumentsAdjusters.cpp
clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
clang/lib/Tooling/Tooling.cpp
clang/unittests/Tooling/CompilationDatabaseTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Tooling/ArgumentsAdjusters.cpp b/clang/lib/Tooling/ArgumentsAdjusters.cpp
index bcfb5b39a0770..d94673bd2ab90 100644
--- a/clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ b/clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -62,7 +62,8 @@ ArgumentsAdjuster getClangSyntaxOnlyAdjuster() {
HasSyntaxOnly = true;
}
if (!HasSyntaxOnly)
- AdjustedArgs.push_back("-fsyntax-only");
+ AdjustedArgs =
+ getInsertArgumentAdjuster("-fsyntax-only")(AdjustedArgs, "");
return AdjustedArgs;
};
}
@@ -137,7 +138,7 @@ ArgumentsAdjuster getInsertArgumentAdjuster(const CommandLineArguments &Extra,
CommandLineArguments::iterator I;
if (Pos == ArgumentInsertPosition::END) {
- I = Return.end();
+ I = std::find(Return.begin(), Return.end(), "--");
} else {
I = Return.begin();
++I; // To leave the program name in place
diff --git a/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp b/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
index 6f97d2867ae5d..3b65504b98ea3 100644
--- a/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ b/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -177,6 +177,10 @@ struct TransferableCommand {
Opt.matches(OPT__SLASH_Fo))))
continue;
+ // ...including when the inputs are passed after --.
+ if (Opt.matches(OPT__DASH_DASH))
+ break;
+
// Strip -x, but record the overridden language.
if (const auto GivenType = tryParseTypeArg(*Arg)) {
Type = *GivenType;
@@ -235,6 +239,8 @@ struct TransferableCommand {
llvm::Twine(ClangCLMode ? "/std:" : "-std=") +
LangStandard::getLangStandardForKind(Std).getName()).str());
}
+ if (Filename.startswith("-") || (ClangCLMode && Filename.startswith("/")))
+ Result.CommandLine.push_back("--");
Result.CommandLine.push_back(std::string(Filename));
return Result;
}
diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp
index 79851ac723da0..b28e8f6a7c967 100644
--- a/clang/lib/Tooling/Tooling.cpp
+++ b/clang/lib/Tooling/Tooling.cpp
@@ -440,8 +440,9 @@ static void injectResourceDir(CommandLineArguments &Args, const char *Argv0,
return;
// If there's no override in place add our resource dir.
- Args.push_back("-resource-dir=" +
- CompilerInvocation::GetResourcesPath(Argv0, MainAddr));
+ Args = getInsertArgumentAdjuster(
+ ("-resource-dir=" + CompilerInvocation::GetResourcesPath(Argv0, MainAddr))
+ .c_str())(Args, "");
}
int ClangTool::run(ToolAction *Action) {
diff --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
index ba40a7a643c42..8ff3387d3c185 100644
--- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -725,14 +725,14 @@ class InterpolateTest : public MemDBTest {
protected:
// Look up the command from a relative path, and return it in string form.
// The input file is not included in the returned command.
- std::string getCommand(llvm::StringRef F) {
+ std::string getCommand(llvm::StringRef F, bool MakeNative = true) {
auto Results =
inferMissingCompileCommands(std::make_unique<MemCDB>(Entries))
- ->getCompileCommands(path(F));
+ ->getCompileCommands(MakeNative ? path(F) : F);
if (Results.empty())
return "none";
// drop the input file argument, so tests don't have to deal with path().
- EXPECT_EQ(Results[0].CommandLine.back(), path(F))
+ EXPECT_EQ(Results[0].CommandLine.back(), MakeNative ? path(F) : F)
<< "Last arg should be the file";
Results[0].CommandLine.pop_back();
return llvm::join(Results[0].CommandLine, " ");
@@ -812,6 +812,28 @@ TEST_F(InterpolateTest, Strip) {
EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
}
+TEST_F(InterpolateTest, StripDoubleDash) {
+ add("dir/foo.cpp", "-o foo.o -std=c++14 -Wall -- dir/foo.cpp");
+ // input file and output option are removed
+ // -Wall flag isn't
+ // -std option gets re-added as the last argument before the input file
+ // -- is removed as it's not necessary - the new input file doesn't start with
+ // a dash
+ EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall -std=c++14");
+}
+
+TEST_F(InterpolateTest, InsertDoubleDash) {
+ add("dir/foo.cpp", "-o foo.o -std=c++14 -Wall");
+ EXPECT_EQ(getCommand("-dir/bar.cpp", false),
+ "clang -D dir/foo.cpp -Wall -std=c++14 --");
+}
+
+TEST_F(InterpolateTest, InsertDoubleDashForClangCL) {
+ add("dir/foo.cpp", "clang-cl", "/std:c++14 /W4");
+ EXPECT_EQ(getCommand("/dir/bar.cpp", false),
+ "clang-cl -D dir/foo.cpp /W4 /std:c++14 --");
+}
+
TEST_F(InterpolateTest, Case) {
add("FOO/BAR/BAZ/SHOUT.cc");
add("foo/bar/baz/quiet.cc");
@@ -831,7 +853,7 @@ TEST_F(InterpolateTest, ClangCL) {
add("foo.cpp", "clang-cl", "/W4");
// Language flags should be added with CL syntax.
- EXPECT_EQ(getCommand("foo.h"), "clang-cl -D foo.cpp /W4 /TP");
+ EXPECT_EQ(getCommand("foo.h", false), "clang-cl -D foo.cpp /W4 /TP");
}
TEST_F(InterpolateTest, DriverModes) {
@@ -839,8 +861,10 @@ TEST_F(InterpolateTest, DriverModes) {
add("bar.cpp", "clang", "--driver-mode=cl");
// --driver-mode overrides should be respected.
- EXPECT_EQ(getCommand("foo.h"), "clang-cl -D foo.cpp --driver-mode=gcc -x c++-header");
- EXPECT_EQ(getCommand("bar.h"), "clang -D bar.cpp --driver-mode=cl /TP");
+ EXPECT_EQ(getCommand("foo.h"),
+ "clang-cl -D foo.cpp --driver-mode=gcc -x c++-header");
+ EXPECT_EQ(getCommand("bar.h", false),
+ "clang -D bar.cpp --driver-mode=cl /TP");
}
TEST(TransferCompileCommandTest, Smoke) {
More information about the cfe-commits
mailing list