diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -76,6 +76,9 @@ /// Profiles resource-usage. void profile(MemoryTree &MT) const; + /// Methods exposed for testing. + llvm::Optional getCompileCommand(PathRef File); + private: // Implement ClangdServer::Callbacks. void onDiagnosticsReady(PathRef File, llvm::StringRef Version, diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -466,6 +466,7 @@ if (Server) return Reply(llvm::make_error("server already initialized", ErrorCode::InvalidRequest)); + std::unique_ptr Extractor; if (Opts.UseDirBasedCDB) { DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS); if (const auto &Dir = Params.initializationOptions.compilationDatabasePath) @@ -473,10 +474,10 @@ CDBOpts.ContextProvider = Opts.ContextProvider; BaseCDB = std::make_unique(CDBOpts); - BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs), - std::move(BaseCDB)); + Extractor = + getSystemIncludeExtractor(llvm::makeArrayRef(Opts.QueryDriverGlobs)); } - auto Mangler = CommandMangler::detect(); + auto Mangler = CommandMangler::detect(std::move(Extractor)); if (Opts.ResourceDir) Mangler->ResourceDir = *Opts.ResourceDir; CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags, @@ -1816,5 +1817,12 @@ }); } } + +llvm::Optional +ClangdLSPServer::getCompileCommand(PathRef File) { + WithContext Context(Opts.ContextProvider(File)); + return CDB->getCompileCommand(File); +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/CompileCommands.h b/clang-tools-extra/clangd/CompileCommands.h --- a/clang-tools-extra/clangd/CompileCommands.h +++ b/clang-tools-extra/clangd/CompileCommands.h @@ -40,15 +40,20 @@ // - 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 std::unique_ptr detect(); + static std::unique_ptr + detect(std::unique_ptr SystemIncludeExtractor = + nullptr); void adjust(tooling::CompileCommand &Cmd, llvm::StringRef File) const override; // Needs to be public so make_unique implementation can call it. - CommandMangler() = default; + CommandMangler( + std::unique_ptr SystemIncludeExtractor = nullptr) + : SystemIncludeExtractor(std::move(SystemIncludeExtractor)) {} private: + std::unique_ptr SystemIncludeExtractor; Memoize> ResolvedDrivers; Memoize> ResolvedDriversNoFollow; }; diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/clang-tools-extra/clangd/CompileCommands.cpp @@ -184,8 +184,10 @@ } // namespace -std::unique_ptr CommandMangler::detect() { - std::unique_ptr Result = std::make_unique(); +std::unique_ptr CommandMangler::detect( + std::unique_ptr SystemIncludeExtractor) { + std::unique_ptr Result = + std::make_unique(std::move(SystemIncludeExtractor)); Result->ClangPath = detectClangPath(); Result->ResourceDir = detectStandardResourceDir(); Result->Sysroot = detectSysroot(); @@ -300,6 +302,17 @@ for (auto &Edit : Config::current().CompileFlags.Edits) Edit(Cmd); + // The system include extractor needs to run: + // - AFTER transferCompileCommand(), because the -x flag it adds may be + // necessary for the system include extractor to identify the file type + // - AFTER applying CompileFlags.Edits, because the name of the compiler + // that needs to be invoked may come from the CompileFlags->Compiler key + // - BEFORE resolveDriver() because that can mess up the driver path, + // e.g. changing gcc to /path/to/clang/bin/gcc + if (SystemIncludeExtractor) { + SystemIncludeExtractor->adjust(Command, File); + } + // Check whether the flag exists, either as -flag or -flag=* auto Has = [&](llvm::StringRef Flag) { for (llvm::StringRef Arg : Cmd) { diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h @@ -171,11 +171,10 @@ }; /// Extracts system include search path from drivers matching QueryDriverGlobs -/// and adds them to the compile flags. Base may not be nullptr. -/// Returns Base when \p QueryDriverGlobs is empty. -std::unique_ptr -getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs, - std::unique_ptr Base); +/// and adds them to the compile flags. +/// Returns null when \p QueryDriverGlobs is empty. +std::unique_ptr +getSystemIncludeExtractor(llvm::ArrayRef QueryDriverGlobs); /// Wraps another compilation database, and supports overriding the commands /// using an in-memory mapping. diff --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp b/clang-tools-extra/clangd/QueryDriverDatabase.cpp --- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp +++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp @@ -315,24 +315,21 @@ /// Extracts system includes from a trusted driver by parsing the output of /// include search path and appends them to the commands coming from underlying /// compilation database. -class QueryDriverDatabase : public DelegatingCDB { +class SystemIncludeExtractor : public CompileCommandsAdjuster { public: - QueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs, - std::unique_ptr Base) - : DelegatingCDB(std::move(Base)), - QueryDriverRegex(convertGlobsToRegex(QueryDriverGlobs)) {} + SystemIncludeExtractor(llvm::ArrayRef QueryDriverGlobs) + : QueryDriverRegex(convertGlobsToRegex(QueryDriverGlobs)) {} - llvm::Optional - getCompileCommand(PathRef File) const override { - auto Cmd = DelegatingCDB::getCompileCommand(File); - if (!Cmd || Cmd->CommandLine.empty()) - return Cmd; + void adjust(tooling::CompileCommand &Cmd, + llvm::StringRef File) const override { + if (Cmd.CommandLine.empty()) + return; llvm::StringRef Lang; - for (size_t I = 0, E = Cmd->CommandLine.size(); I < E; ++I) { - llvm::StringRef Arg = Cmd->CommandLine[I]; + for (size_t I = 0, E = Cmd.CommandLine.size(); I < E; ++I) { + llvm::StringRef Arg = Cmd.CommandLine[I]; if (Arg == "-x" && I + 1 < E) - Lang = Cmd->CommandLine[I + 1]; + Lang = Cmd.CommandLine[I + 1]; else if (Arg.startswith("-x")) Lang = Arg.drop_front(2).trim(); } @@ -341,26 +338,25 @@ auto Type = driver::types::lookupTypeForExtension(Ext); if (Type == driver::types::TY_INVALID) { elog("System include extraction: invalid file type for {0}", Ext); - return {}; + return; } Lang = driver::types::getTypeName(Type); } - llvm::SmallString<128> Driver(Cmd->CommandLine.front()); + llvm::SmallString<128> Driver(Cmd.CommandLine.front()); if (llvm::any_of(Driver, [](char C) { return llvm::sys::path::is_separator(C); })) // Driver is a not a single executable name but instead a path (either // relative or absolute). - llvm::sys::fs::make_absolute(Cmd->Directory, Driver); + llvm::sys::fs::make_absolute(Cmd.Directory, Driver); if (auto Info = QueriedDrivers.get(/*Key=*/(Driver + ":" + Lang).str(), [&] { return extractSystemIncludesAndTarget( - Driver, Lang, Cmd->CommandLine, QueryDriverRegex); + Driver, Lang, Cmd.CommandLine, QueryDriverRegex); })) { - setTarget(addSystemIncludes(*Cmd, Info->SystemIncludes), Info->Target); + setTarget(addSystemIncludes(Cmd, Info->SystemIncludes), Info->Target); } - return Cmd; } private: @@ -370,14 +366,11 @@ }; } // namespace -std::unique_ptr -getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs, - std::unique_ptr Base) { - assert(Base && "Null base to SystemIncludeExtractor"); +std::unique_ptr +getSystemIncludeExtractor(llvm::ArrayRef QueryDriverGlobs) { if (QueryDriverGlobs.empty()) - return Base; - return std::make_unique(QueryDriverGlobs, - std::move(Base)); + return nullptr; + return std::make_unique(QueryDriverGlobs); } } // namespace clangd diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -100,9 +100,9 @@ Config::current().CompileFlags.CDBSearch.FixedCDBPath; std::unique_ptr BaseCDB = std::make_unique(CDBOpts); - BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs), - std::move(BaseCDB)); - auto Mangler = CommandMangler::detect(); + std::unique_ptr Extractor = + getSystemIncludeExtractor(llvm::makeArrayRef(Opts.QueryDriverGlobs)); + auto Mangler = CommandMangler::detect(std::move(Extractor)); if (Opts.ResourceDir) Mangler->ResourceDir = *Opts.ResourceDir; auto CDB = std::make_unique( diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp --- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp @@ -71,6 +71,7 @@ MockFS FS; ClangdLSPServer::Options Opts; FeatureModuleSet FeatureModules; + llvm::Optional Server; private: class Logger : public clang::clangd::Logger { @@ -97,7 +98,6 @@ Logger L; LoggingSession LogSession; - llvm::Optional Server; llvm::Optional ServerThread; LSPClient Client; }; @@ -229,6 +229,33 @@ diagMessage("Use of undeclared identifier 'BAR'")))); } +// Tests that clangd can run --query-driver on a compiler specified +// via the Compiler key in the config file. +TEST_F(LSPTest, QueryCompilerFromConfig) { + auto CfgProvider = + config::Provider::fromAncestorRelativeYAMLFiles(".clangd", FS); + Opts.ConfigProvider = CfgProvider.get(); + Opts.QueryDriverGlobs = {"/**/*"}; + + FS.Files[".clangd"] = R"yaml( +CompileFlags: + Compiler: gcc + )yaml"; + // Just to ensure there's a CDB entry + FS.Files["compile_flags.txt"] = "-DSOME_FLAG"; + + auto &Client = start(); + Client.sync(); // wait for onInitialize() to run + + auto CompileCommand = Server->getCompileCommand(testPath("foo.cpp")); + ASSERT_TRUE(bool(CompileCommand)); + EXPECT_GE(llvm::count_if(CompileCommand->CommandLine, + [](const std::string &Arg) { + return StringRef(Arg).contains("isystem"); + }), + 1); +} + TEST_F(LSPTest, ModulesTest) { class MathModule final : public FeatureModule { OutgoingNotification Changed;