diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp index f43ce928463b90..f4e8e7e74a3bee 100644 --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/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 #include #include @@ -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 OriginalArgs; diff --git a/clang-tools-extra/clangd/CompileCommands.h b/clang-tools-extra/clangd/CompileCommands.h index 1b37f44f0b9db4..1ee0dba2dba80d 100644 --- a/clang-tools-extra/clangd/CompileCommands.h +++ b/clang-tools-extra/clangd/CompileCommands.h @@ -51,11 +51,8 @@ struct CommandMangler { llvm::StringRef TargetFile) const; private: - CommandMangler(); - Memoize> ResolvedDrivers; Memoize> ResolvedDriversNoFollow; - llvm::cl::TokenizerCallback Tokenizer; }; // Removes args from a command-line in a semantically-aware way. diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index d1833759917a30..5bec7966a9c3a9 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/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 #include #include @@ -244,7 +246,16 @@ static std::unique_ptr 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) diff --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp index 28f0d85d332caa..772177b60b5eed 100644 --- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp +++ b/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 Parts; llvm::SplitString(Argv, Parts); diff --git a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp index 2a6ae9c325b736..a2ffdefe1bbcb2 100644 --- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp +++ b/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.