sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
sammccall requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
"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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116721
Files:
clang-tools-extra/clangd/CompileCommands.cpp
clang/include/clang/Tooling/CompilationDatabase.h
clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
clang/unittests/Tooling/CompilationDatabaseTest.cpp
Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===================================================================
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -739,6 +739,9 @@
EXPECT_EQ(Results[0].CommandLine.back(), MakeNative ? path(F) : F)
<< "Last arg should be the file";
Results[0].CommandLine.pop_back();
+ EXPECT_EQ(Results[0].CommandLine.back(), "--")
+ << "Second-last arg should be --";
+ Results[0].CommandLine.pop_back();
return llvm::join(Results[0].CommandLine, " ");
}
@@ -826,18 +829,6 @@
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");
@@ -879,7 +870,7 @@
CompileCommand Transferred = transferCompileCommand(std::move(Cmd), "foo.h");
EXPECT_EQ(Transferred.Filename, "foo.h");
EXPECT_THAT(Transferred.CommandLine,
- ElementsAre("clang", "-Wall", "-x", "c++-header", "foo.h"));
+ ElementsAre("clang", "-Wall", "-x", "c++-header", "--",
"foo.h"));
EXPECT_EQ(Transferred.Directory, "dir");
}
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -243,8 +243,7 @@
llvm::Twine(ClangCLMode ? "/std:" : "-std=") +
LangStandard::getLangStandardForKind(Std).getName()).str());
}
- if (Filename.startswith("-") || (ClangCLMode && Filename.startswith("/")))
- Result.CommandLine.push_back("--");
+ Result.CommandLine.push_back("--");
Result.CommandLine.push_back(std::string(Filename));
return Result;
}
Index: clang/include/clang/Tooling/CompilationDatabase.h
===================================================================
--- clang/include/clang/Tooling/CompilationDatabase.h
+++ clang/include/clang/Tooling/CompilationDatabase.h
@@ -216,6 +216,8 @@
/// Transforms a compile command so that it applies the same configuration to
/// a different file. Most args are left intact, but tweaks may be needed
/// to certain flags (-x, -std etc).
+///
+/// The output command will always end in {"--", Filename}.
tooling::CompileCommand transferCompileCommand(tooling::CompileCommand,
StringRef Filename);
Index: clang-tools-extra/clangd/CompileCommands.cpp
===================================================================
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ clang-tools-extra/clangd/CompileCommands.cpp
@@ -290,16 +290,9 @@
TransferCmd.CommandLine = std::move(Cmd);
TransferCmd = transferCompileCommand(std::move(TransferCmd), File);
Cmd = std::move(TransferCmd.CommandLine);
-
- // Restore the canonical "driver --opts -- filename" form we expect.
- // FIXME: This is ugly and coupled. Make transferCompileCommand ensure it?
- assert(!Cmd.empty() && Cmd.back() == File);
- Cmd.pop_back();
- if (!Cmd.empty() && Cmd.back() == "--")
- Cmd.pop_back();
- assert(!llvm::is_contained(Cmd, "--"));
- Cmd.push_back("--");
- Cmd.push_back(File.str());
+ assert(Cmd.size() >= 2 && Cmd.back() == File &&
+ Cmd[Cmd.size() - 2] == "--" &&
+ "TransferCommand should produce a command ending in -- filename");
}
for (auto &Edit : Config::current().CompileFlags.Edits)
Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===================================================================
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -739,6 +739,9 @@
EXPECT_EQ(Results[0].CommandLine.back(), MakeNative ? path(F) : F)
<< "Last arg should be the file";
Results[0].CommandLine.pop_back();
+ EXPECT_EQ(Results[0].CommandLine.back(), "--")
+ << "Second-last arg should be --";
+ Results[0].CommandLine.pop_back();
return llvm::join(Results[0].CommandLine, " ");
}
@@ -826,18 +829,6 @@
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");
@@ -879,7 +870,7 @@
CompileCommand Transferred = transferCompileCommand(std::move(Cmd), "foo.h");
EXPECT_EQ(Transferred.Filename, "foo.h");
EXPECT_THAT(Transferred.CommandLine,
- ElementsAre("clang", "-Wall", "-x", "c++-header", "foo.h"));
+ ElementsAre("clang", "-Wall", "-x", "c++-header", "--", "foo.h"));
EXPECT_EQ(Transferred.Directory, "dir");
}
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -243,8 +243,7 @@
llvm::Twine(ClangCLMode ? "/std:" : "-std=") +
LangStandard::getLangStandardForKind(Std).getName()).str());
}
- if (Filename.startswith("-") || (ClangCLMode && Filename.startswith("/")))
- Result.CommandLine.push_back("--");
+ Result.CommandLine.push_back("--");
Result.CommandLine.push_back(std::string(Filename));
return Result;
}
Index: clang/include/clang/Tooling/CompilationDatabase.h
===================================================================
--- clang/include/clang/Tooling/CompilationDatabase.h
+++ clang/include/clang/Tooling/CompilationDatabase.h
@@ -216,6 +216,8 @@
/// Transforms a compile command so that it applies the same configuration to
/// a different file. Most args are left intact, but tweaks may be needed
/// to certain flags (-x, -std etc).
+///
+/// The output command will always end in {"--", Filename}.
tooling::CompileCommand transferCompileCommand(tooling::CompileCommand,
StringRef Filename);
Index: clang-tools-extra/clangd/CompileCommands.cpp
===================================================================
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ clang-tools-extra/clangd/CompileCommands.cpp
@@ -290,16 +290,9 @@
TransferCmd.CommandLine = std::move(Cmd);
TransferCmd = transferCompileCommand(std::move(TransferCmd), File);
Cmd = std::move(TransferCmd.CommandLine);
-
- // Restore the canonical "driver --opts -- filename" form we expect.
- // FIXME: This is ugly and coupled. Make transferCompileCommand ensure it?
- assert(!Cmd.empty() && Cmd.back() == File);
- Cmd.pop_back();
- if (!Cmd.empty() && Cmd.back() == "--")
- Cmd.pop_back();
- assert(!llvm::is_contained(Cmd, "--"));
- Cmd.push_back("--");
- Cmd.push_back(File.str());
+ assert(Cmd.size() >= 2 && Cmd.back() == File &&
+ Cmd[Cmd.size() - 2] == "--" &&
+ "TransferCommand should produce a command ending in -- filename");
}
for (auto &Edit : Config::current().CompileFlags.Edits)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
