Revert "Revert "[clangd] Adjust compile flags to contain only the requested file as input""

This reverts commit 04e21fb.

Author: kadircet

clang-tools-extra/clangd/CompileCommands.cpp

@@ -196,7 +196,8 @@ CommandMangler CommandMangler::detect() {
 
 CommandMangler CommandMangler::forTests() { return CommandMangler(); }
 
-void CommandMangler::adjust(std::vector<std::string> &Cmd) const {
+void CommandMangler::adjust(std::vector<std::string> &Cmd,
+                            llvm::StringRef File) const {
   trace::Span S("AdjustCompileFlags");
   auto &OptTable = clang::driver::getDriverOptTable();
   // OriginalArgs needs to outlive ArgList.
@@ -211,8 +212,10 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd) const {
   // ParseArgs propagates missig arg/opt counts on error, but preserves
   // everything it could parse in ArgList. So we just ignore those counts.
   unsigned IgnoredCount;
+  // Drop the executable name, as ParseArgs doesn't expect it. This means
+  // indices are actually of by one between ArgList and OriginalArgs.
   auto ArgList = OptTable.ParseArgs(
-      OriginalArgs, IgnoredCount, IgnoredCount,
+      llvm::makeArrayRef(OriginalArgs).drop_front(), IgnoredCount, IgnoredCount,
       /*FlagsToInclude=*/
       IsCLMode ? (driver::options::CLOption | driver::options::CoreOption)
                : /*everything*/ 0,
@@ -223,12 +226,17 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd) const {
   // modifications done in the following steps apply in more cases (like setting
   // -x, which only affects inputs that come after it).
   if (!ArgList.hasArgNoClaim(driver::options::OPT__DASH_DASH)) {
-    // In theory there might be more than one input, but clangd can't deal with
-    // them anyway.
-    if (auto *Input = ArgList.getLastArg(driver::options::OPT_INPUT)) {
-      Cmd.insert(Cmd.end(), {"--", Input->getAsString(ArgList)});
-      Cmd.erase(Cmd.begin() + Input->getIndex());
-    }
+    // Drop all the inputs and only add one for the current file.
+    llvm::SmallVector<unsigned, 1> IndicesToDrop;
+    for (auto *Input : ArgList.filtered(driver::options::OPT_INPUT))
+      IndicesToDrop.push_back(Input->getIndex());
+    llvm::sort(IndicesToDrop);
+    llvm::for_each(llvm::reverse(IndicesToDrop),
+                   // +1 to account for the executable name in Cmd[0] that
+                   // doesn't exist in ArgList.
+                   [&Cmd](unsigned Idx) { Cmd.erase(Cmd.begin() + Idx + 1); });
+    Cmd.push_back("--");
+    Cmd.push_back(File.str());
   }
 
   for (auto &Edit : Config::current().CompileFlags.Edits)
@@ -278,7 +286,7 @@ CommandMangler::operator clang::tooling::ArgumentsAdjuster() && {
   return [Mangler = std::make_shared<CommandMangler>(std::move(*this))](
              const std::vector<std::string> &Args, llvm::StringRef File) {
     auto Result = Args;
-    Mangler->adjust(Result);
+    Mangler->adjust(Result, File);
     return Result;
   };
 }

clang-tools-extra/clangd/CompileCommands.h

@@ -12,6 +12,7 @@
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include <deque>
 #include <string>
 #include <vector>
@@ -46,7 +47,7 @@ struct CommandMangler {
   //  - on mac, find clang and isysroot by querying the `xcrun` launcher
   static CommandMangler detect();
 
-  void adjust(std::vector<std::string> &Cmd) const;
+  void adjust(std::vector<std::string> &Cmd, llvm::StringRef File) const;
   explicit operator clang::tooling::ArgumentsAdjuster() &&;
 
 private:

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

@@ -763,7 +763,7 @@ OverlayCDB::getCompileCommand(PathRef File) const {
   if (!Cmd)
     return llvm::None;
   if (ArgsAdjuster)
-    Cmd->CommandLine = ArgsAdjuster(Cmd->CommandLine, Cmd->Filename);
+    Cmd->CommandLine = ArgsAdjuster(Cmd->CommandLine, File);
   return Cmd;
 }
 
@@ -773,7 +773,7 @@ tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const {
   Cmd.CommandLine.insert(Cmd.CommandLine.end(), FallbackFlags.begin(),
                          FallbackFlags.end());
   if (ArgsAdjuster)
-    Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename);
+    Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, File);
   return Cmd;
 }
 

clang-tools-extra/clangd/test/did-change-configuration-params.test

@@ -48,7 +48,7 @@
 #
 # ERR: ASTWorker building file {{.*}}foo.c version 0 with command
 # ERR: [{{.*}}clangd-test2]
-# ERR: clang -c -Wall -Werror {{.*}} -- foo.c
+# ERR: clang -c -Wall -Werror {{.*}} -- {{.*}}foo.c
 ---
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---

clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp

@@ -44,7 +44,7 @@ TEST(CommandMangler, Everything) {
   Mangler.ResourceDir = testPath("fake/resources");
   Mangler.Sysroot = testPath("fake/sysroot");
   std::vector<std::string> Cmd = {"clang++", "--", "foo.cc"};
-  Mangler.adjust(Cmd);
+  Mangler.adjust(Cmd, "foo.cc");
   EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"),
                                "-resource-dir=" + testPath("fake/resources"),
                                "-isysroot", testPath("fake/sysroot"), "--",
@@ -55,7 +55,7 @@ TEST(CommandMangler, ResourceDir) {
   auto Mangler = CommandMangler::forTests();
   Mangler.ResourceDir = testPath("fake/resources");
   std::vector<std::string> Cmd = {"clang++", "foo.cc"};
-  Mangler.adjust(Cmd);
+  Mangler.adjust(Cmd, "foo.cc");
   EXPECT_THAT(Cmd, Contains("-resource-dir=" + testPath("fake/resources")));
 }
 
@@ -64,7 +64,7 @@ TEST(CommandMangler, Sysroot) {
   Mangler.Sysroot = testPath("fake/sysroot");
 
   std::vector<std::string> Cmd = {"clang++", "foo.cc"};
-  Mangler.adjust(Cmd);
+  Mangler.adjust(Cmd, "foo.cc");
   EXPECT_THAT(llvm::join(Cmd, " "),
               HasSubstr("-isysroot " + testPath("fake/sysroot")));
 }
@@ -74,19 +74,19 @@ TEST(CommandMangler, ClangPath) {
   Mangler.ClangPath = testPath("fake/clang");
 
   std::vector<std::string> Cmd = {"clang++", "foo.cc"};
-  Mangler.adjust(Cmd);
+  Mangler.adjust(Cmd, "foo.cc");
   EXPECT_EQ(testPath("fake/clang++"), Cmd.front());
 
   Cmd = {"unknown-binary", "foo.cc"};
-  Mangler.adjust(Cmd);
+  Mangler.adjust(Cmd, "foo.cc");
   EXPECT_EQ(testPath("fake/unknown-binary"), Cmd.front());
 
   Cmd = {testPath("path/clang++"), "foo.cc"};
-  Mangler.adjust(Cmd);
+  Mangler.adjust(Cmd, "foo.cc");
   EXPECT_EQ(testPath("path/clang++"), Cmd.front());
 
   Cmd = {"foo/unknown-binary", "foo.cc"};
-  Mangler.adjust(Cmd);
+  Mangler.adjust(Cmd, "foo.cc");
   EXPECT_EQ("foo/unknown-binary", Cmd.front());
 }
 
@@ -129,7 +129,7 @@ TEST(CommandMangler, ClangPathResolve) {
   auto Mangler = CommandMangler::forTests();
   Mangler.ClangPath = testPath("fake/clang");
   std::vector<std::string> Cmd = {(TempDir + "/bin/foo").str(), "foo.cc"};
-  Mangler.adjust(Cmd);
+  Mangler.adjust(Cmd, "foo.cc");
   // Directory based on resolved symlink, basename preserved.
   EXPECT_EQ((TempDir + "/lib/foo").str(), Cmd.front());
 
@@ -146,13 +146,13 @@ TEST(CommandMangler, ClangPathResolve) {
   Mangler.ClangPath = testPath("fake/clang");
   // Driver found on PATH.
   Cmd = {"foo", "foo.cc"};
-  Mangler.adjust(Cmd);
+  Mangler.adjust(Cmd, "foo.cc");
   // Found the symlink and resolved the path as above.
   EXPECT_EQ((TempDir + "/lib/foo").str(), Cmd.front());
 
   // Symlink not resolved with -no-canonical-prefixes.
   Cmd = {"foo", "-no-canonical-prefixes", "foo.cc"};
-  Mangler.adjust(Cmd);
+  Mangler.adjust(Cmd, "foo.cc");
   EXPECT_EQ((TempDir + "/bin/foo").str(), Cmd.front());
 }
 #endif
@@ -171,7 +171,7 @@ TEST(CommandMangler, ConfigEdits) {
       Argv = tooling::getInsertArgumentAdjuster("--hello")(Argv, "");
     });
     WithContextValue WithConfig(Config::Key, std::move(Cfg));
-    Mangler.adjust(Cmd);
+    Mangler.adjust(Cmd, "foo.cc");
   }
   // Edits are applied in given order and before other mangling and they always
   // go before filename.
@@ -350,7 +350,7 @@ TEST(CommandMangler, InputsAfterDashDash) {
   const auto Mangler = CommandMangler::forTests();
   {
     std::vector<std::string> Args = {"clang", "/Users/foo.cc"};
-    Mangler.adjust(Args);
+    Mangler.adjust(Args, "/Users/foo.cc");
     EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2),
                 ElementsAre("--", "/Users/foo.cc"));
     EXPECT_THAT(llvm::makeArrayRef(Args).drop_back(2),
@@ -361,11 +361,21 @@ TEST(CommandMangler, InputsAfterDashDash) {
   {
     std::vector<std::string> Args = {"clang", "--driver-mode=cl", "bar.cc",
                                      "/Users/foo.cc"};
-    Mangler.adjust(Args);
+    Mangler.adjust(Args, "bar.cc");
     EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2),
                 ElementsAre("--", "bar.cc"));
     EXPECT_THAT(llvm::makeArrayRef(Args).drop_back(2), Not(Contains("bar.cc")));
   }
+  // All inputs but the main file is dropped.
+  {
+    std::vector<std::string> Args = {"clang", "foo.cc", "bar.cc"};
+    Mangler.adjust(Args, "baz.cc");
+    EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2),
+                ElementsAre("--", "baz.cc"));
+    EXPECT_THAT(
+        llvm::makeArrayRef(Args).drop_back(2),
+        testing::AllOf(Not(Contains("foo.cc")), Not(Contains("bar.cc"))));
+  }
 }
 } // namespace
 } // namespace clangd

clang-tools-extra/clangd/unittests/TestTU.cpp

@@ -59,7 +59,7 @@ ParseInputs TestTU::inputs(MockFS &FS) const {
   Argv.push_back(FullFilename);
 
   auto Mangler = CommandMangler::forTests();
-  Mangler.adjust(Inputs.CompileCommand.CommandLine);
+  Mangler.adjust(Inputs.CompileCommand.CommandLine, FullFilename);
   Inputs.CompileCommand.Filename = FullFilename;
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;