[PATCH] D116721: [Tooling] When transferring compile commands between files, always use '--'

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4258d68dc737: [Tooling] When transferring compile commands between files, always use '--' (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116721/new/

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)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116721.398780.patch
Type: text/x-patch
Size: 3987 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220111/5b7a1575/attachment.bin>