[clang-tools-extra] 4258d68 - [Tooling] When transferring compile commands between files, always use '--'

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-01-11T01:41:42+01:00
New Revision: 4258d68dc73789bc7fc491734c9c392809b1b29a

URL: https://github.com/llvm/llvm-project/commit/4258d68dc73789bc7fc491734c9c392809b1b29a
DIFF: https://github.com/llvm/llvm-project/commit/4258d68dc73789bc7fc491734c9c392809b1b29a.diff

LOG: [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

Added: 
    

Modified: 
    clang-tools-extra/clangd/CompileCommands.cpp
    clang/include/clang/Tooling/CompilationDatabase.h
    clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
    clang/unittests/Tooling/CompilationDatabaseTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index 5c98e40a87fdd..7d6f612cb8b96 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -290,16 +290,9 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd,
     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)

diff  --git a/clang/include/clang/Tooling/CompilationDatabase.h b/clang/include/clang/Tooling/CompilationDatabase.h
index 90af15536961e..fee584acb4862 100644
--- a/clang/include/clang/Tooling/CompilationDatabase.h
+++ b/clang/include/clang/Tooling/CompilationDatabase.h
@@ -216,6 +216,8 @@ class FixedCompilationDatabase : public CompilationDatabase {
 /// Transforms a compile command so that it applies the same configuration to
 /// a 
diff erent 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);
 

diff  --git a/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp b/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
index c1e25c41f7198..51e8439b6b79b 100644
--- a/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ b/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -243,8 +243,7 @@ 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("--");
     Result.CommandLine.push_back(std::string(Filename));
     return Result;
   }

diff  --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
index 218a352f86f06..adb9de0c1cbaa 100644
--- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -739,6 +739,9 @@ class InterpolateTest : public MemDBTest {
     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 @@ TEST_F(InterpolateTest, StripDoubleDash) {
   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 @@ TEST(TransferCompileCommandTest, Smoke) {
   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");
 }