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

[sammccall]

Summary

"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.

[kadircet]

thanks, lgtm!

clang-tools-extra/clangd/CompileCommands.cpp

void CommandMangler::adjust(std::vector<std::string> &Cmd,
   Cmd.push_back(File.str());
 
   if (TransferFrom) {
     tooling::CompileCommand TransferCmd;
     TransferCmd.Filename = std::move(*TransferFrom);
     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)
     Edit(Cmd);
 
   // Check whether the flag exists, either as -flag or -flag=*
   auto Has = [&](llvm::StringRef Flag) {
     for (llvm::StringRef Arg : Cmd) {

clang/include/clang/Tooling/CompilationDatabase.h

private:
   /// This is built up to contain a single entry vector to be returned from
   /// getCompileCommands after adding the positional argument.
   std::vector<CompileCommand> CompileCommands;
 };
 
 /// 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);
 
 /// Returns a wrapped CompilationDatabase that defers to the provided one,
 /// but getCompileCommands() will infer commands for unknown files.
 /// The return value of getAllFiles() or getAllCompileCommands() is unchanged.
 /// See InterpolatingCompilationDatabase.cpp for details on heuristics.
 std::unique_ptr<CompilationDatabase>

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp

CompileCommand transferTo(StringRef Filename) && {
     }
     // --std flag may only be transferred if the language is the same.
     // We may consider "translating" these, e.g. c++11 -> c11.
     if (Std != LangStandard::lang_unspecified && foldType(TargetType) == Type) {
       Result.CommandLine.emplace_back((
           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;
   }
 
 private:
   // Map the language from the --std flag to that of the -x flag.
   static types::ID toType(Language Lang) {
     switch (Lang) {

clang/unittests/Tooling/CompilationDatabaseTest.cpp

auto Results =
         inferMissingCompileCommands(std::make_unique<MemCDB>(Entries))
             ->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(), 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, " ");
   }
 
   // Parse the file whose command was used out of the Heuristic string.
   std::string getProxy(llvm::StringRef F) {
     auto Results =
         inferMissingCompileCommands(std::make_unique<MemCDB>(Entries))
             ->getCompileCommands(path(F));
TEST_F(InterpolateTest, StripDoubleDash) {
   // 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");
   // Case mismatches are completely ignored, so we choose the name match.
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
 TEST_F(InterpolateTest, Aliasing) {

 TEST(TransferCompileCommandTest, Smoke) {
   CompileCommand Cmd;
   Cmd.Filename = "foo.cc";
   Cmd.CommandLine = {"clang", "-Wall", "foo.cc"};
   Cmd.Directory = "dir";
   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");
 }
 
 TEST(CompileCommandTest, EqualityOperator) {
   CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
   CompileCommand CCTest = CCRef;
 
   EXPECT_TRUE(CCRef == CCTest);