[clangd] Expand response files before CDB interpolation (#75753)

Summary:

After https://reviews.llvm.org/D143436 response files stopped working
with CDB interpolation. It has happened because interpolation removes
all unknown flags and extra input files. Response file is treated as an
extra input because it is not a flag. Moreover inference needs full
command line for driver mode and file type detection so all response
files have to be expanded for correct inference.

This patch partially reverts D143436 and add additional response file
expansion in OverlayCDB for CDBs pushed via LSP.

Test Plan: check-clangd

Tasks: https://github.com/llvm/llvm-project/issues/69690

clang-tools-extra/clangd/CompileCommands.cpp

@@ -28,7 +28,6 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
-#include "llvm/TargetParser/Host.h"
 #include <iterator>
 #include <optional>
 #include <string>
@@ -187,12 +186,6 @@ static std::string resolveDriver(llvm::StringRef Driver, bool FollowSymlink,
 
 } // namespace
 
-CommandMangler::CommandMangler() {
-  Tokenizer = llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows()
-                  ? llvm::cl::TokenizeWindowsCommandLine
-                  : llvm::cl::TokenizeGNUCommandLine;
-}
-
 CommandMangler CommandMangler::detect() {
   CommandMangler Result;
   Result.ClangPath = detectClangPath();
@@ -213,14 +206,6 @@ void CommandMangler::operator()(tooling::CompileCommand &Command,
   if (Cmd.empty())
     return;
 
-  // FS used for expanding response files.
-  // FIXME: ExpandResponseFiles appears not to provide the usual
-  // thread-safety guarantees, as the access to FS is not locked!
-  // For now, use the real FS, which is known to be threadsafe (if we don't
-  // use/change working directory, which ExpandResponseFiles doesn't).
-  auto FS = llvm::vfs::getRealFileSystem();
-  tooling::addExpandedResponseFiles(Cmd, Command.Directory, Tokenizer, *FS);
-
   auto &OptTable = clang::driver::getDriverOptTable();
   // OriginalArgs needs to outlive ArgList.
   llvm::SmallVector<const char *, 16> OriginalArgs;

clang-tools-extra/clangd/CompileCommands.h

@@ -51,11 +51,8 @@ struct CommandMangler {
                   llvm::StringRef TargetFile) const;
 
 private:
-  CommandMangler();
-
   Memoize<llvm::StringMap<std::string>> ResolvedDrivers;
   Memoize<llvm::StringMap<std::string>> ResolvedDriversNoFollow;
-  llvm::cl::TokenizerCallback Tokenizer;
 };
 
 // Removes args from a command-line in a semantically-aware way.

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

@@ -18,6 +18,7 @@
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/CompilationDatabasePluginRegistry.h"
 #include "clang/Tooling/JSONCompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -25,6 +26,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/TargetParser/Host.h"
 #include <atomic>
 #include <chrono>
 #include <condition_variable>
@@ -244,7 +246,16 @@ static std::unique_ptr<tooling::CompilationDatabase>
 parseJSON(PathRef Path, llvm::StringRef Data, std::string &Error) {
   if (auto CDB = tooling::JSONCompilationDatabase::loadFromBuffer(
           Data, Error, tooling::JSONCommandLineSyntax::AutoDetect)) {
-    return tooling::inferMissingCompileCommands(std::move(CDB));
+    // FS used for expanding response files.
+    // FIXME: ExpandResponseFilesDatabase appears not to provide the usual
+    // thread-safety guarantees, as the access to FS is not locked!
+    // For now, use the real FS, which is known to be threadsafe (if we don't
+    // use/change working directory, which ExpandResponseFilesDatabase doesn't).
+    // NOTE: response files have to be expanded before inference because
+    // inference needs full command line to check/fix driver mode and file type.
+    auto FS = llvm::vfs::getRealFileSystem();
+    return tooling::inferMissingCompileCommands(
+        expandResponseFiles(std::move(CDB), std::move(FS)));
   }
   return nullptr;
 }
@@ -744,6 +755,22 @@ OverlayCDB::getCompileCommand(PathRef File) const {
     if (It != Commands.end())
       Cmd = It->second;
   }
+  if (Cmd) {
+    // FS used for expanding response files.
+    // FIXME: ExpandResponseFiles appears not to provide the usual
+    // thread-safety guarantees, as the access to FS is not locked!
+    // For now, use the real FS, which is known to be threadsafe (if we don't
+    // use/change working directory, which ExpandResponseFiles doesn't).
+    auto FS = llvm::vfs::getRealFileSystem();
+    auto Tokenizer = llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows()
+                         ? llvm::cl::TokenizeWindowsCommandLine
+                         : llvm::cl::TokenizeGNUCommandLine;
+    // Compile command pushed via LSP protocol may have response files that need
+    // to be expanded before further processing. For CDB for files it happens in
+    // the main CDB when reading it from the JSON file.
+    tooling::addExpandedResponseFiles(Cmd->CommandLine, Cmd->Directory,
+                                      Tokenizer, *FS);
+  }
   if (!Cmd)
     Cmd = DelegatingCDB::getCompileCommand(File);
   if (!Cmd)

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

@@ -209,21 +209,6 @@ TEST(CommandMangler, ConfigEdits) {
               ElementsAre(_, "--driver-mode=g++", "--hello", "--", "FOO.CC"));
 }
 
-TEST(CommandMangler, ExpandedResponseFiles) {
-  SmallString<1024> Path;
-  int FD;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("args", "", FD, Path));
-  llvm::raw_fd_ostream OutStream(FD, true);
-  OutStream << "-Wall";
-  OutStream.close();
-
-  auto Mangler = CommandMangler::forTests();
-  tooling::CompileCommand Cmd;
-  Cmd.CommandLine = {"clang", ("@" + Path).str(), "foo.cc"};
-  Mangler(Cmd, "foo.cc");
-  EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "-Wall", "--", "foo.cc"));
-}
-
 static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
   llvm::SmallVector<llvm::StringRef> Parts;
   llvm::SplitString(Argv, Parts);

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

@@ -163,6 +163,21 @@ TEST_F(OverlayCDBTest, Adjustments) {
                                            "-DFallback", "-DAdjust_baz.cc"));
 }
 
+TEST_F(OverlayCDBTest, ExpandedResponseFiles) {
+  SmallString<1024> Path;
+  int FD;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("args", "", FD, Path));
+  llvm::raw_fd_ostream OutStream(FD, true);
+  OutStream << "-Wall";
+  OutStream.close();
+
+  OverlayCDB CDB(Base.get(), {"-DFallback"});
+  auto Override = cmd(testPath("foo.cc"), ("@" + Path).str());
+  CDB.setCompileCommand(testPath("foo.cc"), Override);
+  EXPECT_THAT(CDB.getCompileCommand(testPath("foo.cc"))->CommandLine,
+              Contains("-Wall"));
+}
+
 TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
   const char *const CDBOuter =
       R"cdb(
@@ -421,6 +436,45 @@ TEST_F(OverlayCDBTest, GetProjectInfo) {
   EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
   EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
 }
+
+TEST(GlobalCompilationDatabaseTest, InferenceWithResponseFile) {
+  MockFS FS;
+  auto Command = [&](llvm::StringRef Relative) {
+    DirectoryBasedGlobalCompilationDatabase::Options Opts(FS);
+    return DirectoryBasedGlobalCompilationDatabase(Opts)
+        .getCompileCommand(testPath(Relative))
+        .value_or(tooling::CompileCommand())
+        .CommandLine;
+  };
+  EXPECT_THAT(Command("foo.cc"), IsEmpty());
+
+  // Have to use real FS for response file.
+  SmallString<1024> Path;
+  int FD;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("args", "", FD, Path));
+  llvm::raw_fd_ostream OutStream(FD, true);
+  OutStream << "-DXYZZY";
+  OutStream.close();
+
+  const char *const CDB =
+      R"cdb(
+      [
+        {
+          "file": "{0}/foo.cc",
+          "command": "clang @{1} {0}/foo.cc",
+          "directory": "{0}",
+        }
+      ]
+      )cdb";
+  FS.Files[testPath("compile_commands.json")] =
+      llvm::formatv(CDB, llvm::sys::path::convert_to_slash(testRoot()),
+                    llvm::sys::path::convert_to_slash(Path));
+
+  // File from CDB.
+  EXPECT_THAT(Command("foo.cc"), Contains("-DXYZZY"));
+  // File not in CDB, use inference.
+  EXPECT_THAT(Command("foo.h"), Contains("-DXYZZY"));
+}
 } // namespace
 
 // Friend test has access to internals.