Skip to content

Commit 4258d68

Browse files
committed
[Tooling] When transferring compile commands between files, always use '--'
"driver <flags> -- <input>" is a particularly convenient form of the compile command to manipulate, with fewer special cases to handle. Guaranteeing that the output command is of that form is cheap and makes it easier to consume the result in some cases. Differential Revision: https://reviews.llvm.org/D116721
1 parent d789ea7 commit 4258d68

File tree

4 files changed

+10
-25
lines changed

4 files changed

+10
-25
lines changed

clang-tools-extra/clangd/CompileCommands.cpp

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -290,16 +290,9 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd,
290290
TransferCmd.CommandLine = std::move(Cmd);
291291
TransferCmd = transferCompileCommand(std::move(TransferCmd), File);
292292
Cmd = std::move(TransferCmd.CommandLine);
293-
294-
// Restore the canonical "driver --opts -- filename" form we expect.
295-
// FIXME: This is ugly and coupled. Make transferCompileCommand ensure it?
296-
assert(!Cmd.empty() && Cmd.back() == File);
297-
Cmd.pop_back();
298-
if (!Cmd.empty() && Cmd.back() == "--")
299-
Cmd.pop_back();
300-
assert(!llvm::is_contained(Cmd, "--"));
301-
Cmd.push_back("--");
302-
Cmd.push_back(File.str());
293+
assert(Cmd.size() >= 2 && Cmd.back() == File &&
294+
Cmd[Cmd.size() - 2] == "--" &&
295+
"TransferCommand should produce a command ending in -- filename");
303296
}
304297

305298
for (auto &Edit : Config::current().CompileFlags.Edits)

clang/include/clang/Tooling/CompilationDatabase.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,8 @@ class FixedCompilationDatabase : public CompilationDatabase {
216216
/// Transforms a compile command so that it applies the same configuration to
217217
/// a different file. Most args are left intact, but tweaks may be needed
218218
/// to certain flags (-x, -std etc).
219+
///
220+
/// The output command will always end in {"--", Filename}.
219221
tooling::CompileCommand transferCompileCommand(tooling::CompileCommand,
220222
StringRef Filename);
221223

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,7 @@ struct TransferableCommand {
243243
llvm::Twine(ClangCLMode ? "/std:" : "-std=") +
244244
LangStandard::getLangStandardForKind(Std).getName()).str());
245245
}
246-
if (Filename.startswith("-") || (ClangCLMode && Filename.startswith("/")))
247-
Result.CommandLine.push_back("--");
246+
Result.CommandLine.push_back("--");
248247
Result.CommandLine.push_back(std::string(Filename));
249248
return Result;
250249
}

clang/unittests/Tooling/CompilationDatabaseTest.cpp

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,9 @@ class InterpolateTest : public MemDBTest {
739739
EXPECT_EQ(Results[0].CommandLine.back(), MakeNative ? path(F) : F)
740740
<< "Last arg should be the file";
741741
Results[0].CommandLine.pop_back();
742+
EXPECT_EQ(Results[0].CommandLine.back(), "--")
743+
<< "Second-last arg should be --";
744+
Results[0].CommandLine.pop_back();
742745
return llvm::join(Results[0].CommandLine, " ");
743746
}
744747

@@ -826,18 +829,6 @@ TEST_F(InterpolateTest, StripDoubleDash) {
826829
EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall -std=c++14");
827830
}
828831

829-
TEST_F(InterpolateTest, InsertDoubleDash) {
830-
add("dir/foo.cpp", "-o foo.o -std=c++14 -Wall");
831-
EXPECT_EQ(getCommand("-dir/bar.cpp", false),
832-
"clang -D dir/foo.cpp -Wall -std=c++14 --");
833-
}
834-
835-
TEST_F(InterpolateTest, InsertDoubleDashForClangCL) {
836-
add("dir/foo.cpp", "clang-cl", "/std:c++14 /W4");
837-
EXPECT_EQ(getCommand("/dir/bar.cpp", false),
838-
"clang-cl -D dir/foo.cpp /W4 /std:c++14 --");
839-
}
840-
841832
TEST_F(InterpolateTest, Case) {
842833
add("FOO/BAR/BAZ/SHOUT.cc");
843834
add("foo/bar/baz/quiet.cc");
@@ -879,7 +870,7 @@ TEST(TransferCompileCommandTest, Smoke) {
879870
CompileCommand Transferred = transferCompileCommand(std::move(Cmd), "foo.h");
880871
EXPECT_EQ(Transferred.Filename, "foo.h");
881872
EXPECT_THAT(Transferred.CommandLine,
882-
ElementsAre("clang", "-Wall", "-x", "c++-header", "foo.h"));
873+
ElementsAre("clang", "-Wall", "-x", "c++-header", "--", "foo.h"));
883874
EXPECT_EQ(Transferred.Directory, "dir");
884875
}
885876

0 commit comments

Comments
 (0)