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

[kadircet]

Summary

Depends on D106527.

[kadircet]

• Rebase

[sammccall]

clang-tools-extra/clangd/CompileCommands.cpp

216

Is this because we're not meant to be parsing argv0?
Maybe check that and fix that here if so, or leave a FIXME if we don't want to check now?

226

nit: Fairly sure this is copying File twice, init-lists are dumb. maybe just two push_backs instead?

[kadircet]

• push_back instead of insert
• don't parse argv[0] rather than skipping it

clang-tools-extra/clangd/CompileCommands.h

 //===--- CompileCommands.h - Manipulation of compile flags -------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILECOMMANDS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILECOMMANDS_H
 
 #include "support/Threading.h"
 #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>
 
 namespace clang {
 namespace clangd {
 
 // CommandMangler transforms compile commands from some external source
struct CommandMangler {
   // This is hermetic for unit-tests, but won't work well in production.
   static CommandMangler forTests();
   // Probe the system and build a command-mangler that knows the toolchain.
   //  - try to find clang on $PATH, otherwise fake a path near clangd
   //  - find the resource directory installed near clangd
   //  - 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:
   CommandMangler() = default;
   Memoize<llvm::StringMap<std::string>> ResolvedDrivers;
   Memoize<llvm::StringMap<std::string>> ResolvedDriversNoFollow;
 };
 

clang-tools-extra/clangd/CompileCommands.cpp

CommandMangler CommandMangler::detect() {
   Result.ClangPath = detectClangPath();
   Result.ResourceDir = detectStandardResourceDir();
   Result.Sysroot = detectSysroot();
   return Result;
 }
 
 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.
   llvm::SmallVector<const char *, 16> OriginalArgs;
   OriginalArgs.reserve(Cmd.size());
   for (const std::string &S : Cmd)
     OriginalArgs.push_back(S.c_str());
   // ParseArgs propagates missig arg/opt counts on error, but preserves
   // everything it could parse in ArgList. So we just ignore those counts.
   unsigned IgnoredCount;
   auto ArgList = OptTable.ParseArgs(OriginalArgs, IgnoredCount, IgnoredCount);
 
   // Move the inputs to the end, separated via `--` from flags. This ensures
   // 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)) {
+      // Driver name is also considered an input, don't strip that away.
+      if (Input->getIndex() == 0)
+        continue;
+      IndicesToDrop.push_back(Input->getIndex());
+      llvm::errs() << "Dropping: " << Input->getAsString(ArgList) << '-'
+                   << Input->getValue(0) << '\n';
+    }
+    llvm::sort(IndicesToDrop);
+    llvm::for_each(llvm::reverse(IndicesToDrop),
+                   [&Cmd](unsigned Idx) { Cmd.erase(Cmd.begin() + Idx); });
+    Cmd.insert(Cmd.end(), {"--", File.str()});
   }
 
   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) {
void CommandMangler::adjust(std::vector<std::string> &Cmd,
   }
 }
 
 CommandMangler::operator clang::tooling::ArgumentsAdjuster() && {
   // ArgumentsAdjuster is a std::function and so must be copyable.
   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;
   };
 }
 
 // ArgStripper implementation
 namespace {
 
 // Determine total number of args consumed by this option.

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

llvm::Optional<tooling::CompileCommand> Cmd;
     if (It != Commands.end())
       Cmd = It->second;
   }
   if (!Cmd)
     Cmd = DelegatingCDB::getCompileCommand(File);
   if (!Cmd)
     return llvm::None;
   if (ArgsAdjuster)
-    Cmd->CommandLine = ArgsAdjuster(Cmd->CommandLine, Cmd->Filename);
+    Cmd->CommandLine = ArgsAdjuster(Cmd->CommandLine, File);
   return Cmd;
 }
 
 tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const {
   auto Cmd = DelegatingCDB::getFallbackCommand(File);
   std::lock_guard<std::mutex> Lock(Mutex);
   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;
 }
 
 void OverlayCDB::setCompileCommand(
     PathRef File, llvm::Optional<tooling::CompileCommand> Cmd) {
   // We store a canonical version internally to prevent mismatches between set
   // and get compile commands. Also it assures clients listening to broadcasts
   // doesn't receive different names for the same file.

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

 # CHECK-NEXT:      }
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "uri": "file://{{.*}}/foo.c",
 # CHECK-NEXT:    "version": 0
 # CHECK-NEXT:  }
 #
 # 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"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
 
 

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

 // Make use of all features and assert the exact command we get out.
 // Other tests just verify presence/absence of certain args.
 TEST(CommandMangler, Everything) {
   auto Mangler = CommandMangler::forTests();
   Mangler.ClangPath = testPath("fake/clang");
   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"), "--",
                                "foo.cc"));
 }
 
 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")));
 }
 
 TEST(CommandMangler, Sysroot) {
   auto Mangler = CommandMangler::forTests();
   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")));
 }
 
 TEST(CommandMangler, ClangPath) {
   auto Mangler = CommandMangler::forTests();
   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());
 }
 
 // Only run the PATH/symlink resolving test on unix, we need to fiddle
 // with permissions and environment variables...
 #ifdef LLVM_ON_UNIX
 MATCHER(Ok, "") {
   if (arg) {
TEST(CommandMangler, ClangPathResolve) {
   ASSERT_THAT(
       llvm::sys::fs::create_link(TempDir + "/lib/bar", TempDir + "/bin/foo"),
       Ok());
 
   // Test the case where the driver is an absolute path to a symlink.
   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());
 
   // Set PATH to point to temp/bin so we can find 'foo' on it.
   ASSERT_TRUE(::getenv("PATH"));
   auto RestorePath =
       llvm::make_scope_exit([OldPath = std::string(::getenv("PATH"))] {
         ::setenv("PATH", OldPath.c_str(), 1);
       });
   ::setenv("PATH", (TempDir + "/bin").str().c_str(), /*overwrite=*/1);
 
   // Test the case where the driver is a $PATH-relative path to a symlink.
   Mangler = CommandMangler::forTests();
   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
 
 TEST(CommandMangler, ConfigEdits) {
   auto Mangler = CommandMangler::forTests();
   std::vector<std::string> Cmd = {"clang++", "foo.cc"};
   {
     Config Cfg;
     Cfg.CompileFlags.Edits.push_back([](std::vector<std::string> &Argv) {
       for (auto &Arg : Argv)
         for (char &C : Arg)
           C = llvm::toUpper(C);
     });
     Cfg.CompileFlags.Edits.push_back([](std::vector<std::string> &Argv) {
       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.
   EXPECT_THAT(Cmd, ElementsAre(_, "--hello", "--", "FOO.CC"));
 }
 
 static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
   llvm::SmallVector<llvm::StringRef> Parts;

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

if (ImplicitHeaderGuard)
       Inputs.CompileCommand.CommandLine.push_back("-fno-ms-compatibility");
   }
   Argv.insert(Argv.end(), ExtraArgs.begin(), ExtraArgs.end());
   // Put the file name at the end -- this allows the extra arg (-xc++) to
   // override the language setting.
   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;
   if (OverlayRealFileSystemForModules)
     FS.OverlayRealFileSystemForModules = true;
   Inputs.TFS = &FS;
   Inputs.Opts = ParseOptions();
   if (ClangTidyProvider)