Author: Kadir Cetinkaya Date: 2021-03-11T13:35:05+01:00 New Revision: 4f1bbc0b842621d2048ed8687e328e2eab375b0a
URL: https://github.com/llvm/llvm-project/commit/4f1bbc0b842621d2048ed8687e328e2eab375b0a DIFF: https://github.com/llvm/llvm-project/commit/4f1bbc0b842621d2048ed8687e328e2eab375b0a.diff LOG: [clangd] Introduce a CommandLineConfigProvider This enables unifying command line flags with config options in clangd internals. This patch changes behaviour in 2 places: - BackgroundIndex was previously disabled when -remote-index was provided. After this patch, it will be enabled but all files will have bkgindex policy set to Skip. - -index-file was loaded at startup (at least load was initiated), now the load will happen through ProjectAwareIndex with first index query. Unfortunately this doesn't simplify any options initially, as - CompileCommandsDir is also used by clangd --check workflow, which doesn't use configs. - EnableBackgroundIndex option controls whether the component will be created at all, which implies creation of extra threads registering a listener for compilation database discoveries. Differential Revision: https://reviews.llvm.org/D98029 Added: Modified: clang-tools-extra/clangd/ClangdLSPServer.cpp clang-tools-extra/clangd/ClangdLSPServer.h clang-tools-extra/clangd/test/log.test clang-tools-extra/clangd/tool/Check.cpp clang-tools-extra/clangd/tool/ClangdMain.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 8875f85c8b70..cd13e013aa50 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -467,11 +467,10 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, if (Server) return Reply(llvm::make_error<LSPError>("server already initialized", ErrorCode::InvalidRequest)); - if (const auto &Dir = Params.initializationOptions.compilationDatabasePath) - Opts.CompileCommandsDir = Dir; if (Opts.UseDirBasedCDB) { DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS); - CDBOpts.CompileCommandsDir = Opts.CompileCommandsDir; + if (const auto &Dir = Params.initializationOptions.compilationDatabasePath) + CDBOpts.CompileCommandsDir = Dir; CDBOpts.ContextProvider = Opts.ContextProvider; BaseCDB = std::make_unique<DirectoryBasedGlobalCompilationDatabase>(CDBOpts); diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 01d0ec20f098..fc13c6257ee6 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -48,9 +48,6 @@ class ClangdLSPServer : private ClangdServer::Callbacks, /// Look for compilation databases, rather than using compile commands /// set via LSP (extensions) only. bool UseDirBasedCDB = true; - /// A fixed directory to search for a compilation database in. - /// If not set, we search upward from the source file. - llvm::Optional<Path> CompileCommandsDir; /// The offset-encoding to use, or None to negotiate it over LSP. llvm::Optional<OffsetEncoding> Encoding; /// If set, periodically called to release memory. diff --git a/clang-tools-extra/clangd/test/log.test b/clang-tools-extra/clangd/test/log.test index 5d60c10eb917..7a53d361ddde 100644 --- a/clang-tools-extra/clangd/test/log.test +++ b/clang-tools-extra/clangd/test/log.test @@ -1,9 +1,9 @@ -# RUN: env CLANGD_FLAGS=-index-file=no-such-index not clangd -lit-test </dev/null 2>&1 >/dev/null | FileCheck %s +# RUN: env CLANGD_FLAGS=-compile-commands-dir=no-such-dir not clangd -lit-test </dev/null 2>&1 >/dev/null | FileCheck %s CHECK: I[{{.*}}]{{.*}} clangd version {{.*}} CHECK: Working directory: {{.*}} CHECK: argv[0]: clangd CHECK: argv[1]: -lit-test -CHECK: CLANGD_FLAGS: -index-file=no-such-index -CHECK: E[{{.*}}] Can't open no-such-index +CHECK: CLANGD_FLAGS: -compile-commands-dir=no-such-dir +CHECK: E[{{.*}}] Path specified by --compile-commands-dir does not exist. CHECK: Starting LSP over stdin/stdout diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp index 9e3e439ae70d..00a4609e5134 100644 --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -26,6 +26,7 @@ #include "ClangdLSPServer.h" #include "CodeComplete.h" +#include "Config.h" #include "GlobalCompilationDatabase.h" #include "Hover.h" #include "ParsedAST.h" @@ -93,7 +94,8 @@ class Checker { bool buildCommand(const ThreadsafeFS &TFS) { log("Loading compilation database..."); DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS); - CDBOpts.CompileCommandsDir = Opts.CompileCommandsDir; + CDBOpts.CompileCommandsDir = + Config::current().CompileFlags.CDBSearch.FixedCDBPath; std::unique_ptr<GlobalCompilationDatabase> BaseCDB = std::make_unique<DirectoryBasedGlobalCompilationDatabase>(CDBOpts); BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs), @@ -245,6 +247,9 @@ bool check(llvm::StringRef File, const ThreadsafeFS &TFS, } log("Testing on source file {0}", File); + auto ContextProvider = ClangdServer::createConfiguredContextProvider( + Opts.ConfigProvider, nullptr); + WithContext Ctx(ContextProvider("")); Checker C(File, Opts); if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) || !C.buildAST()) diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index 3afcd5f08333..eca30eec3679 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -8,6 +8,8 @@ #include "ClangdLSPServer.h" #include "CodeComplete.h" +#include "Config.h" +#include "ConfigProvider.h" #include "Features.inc" #include "PathMapping.h" #include "Protocol.h" @@ -43,6 +45,7 @@ #include <mutex> #include <string> #include <thread> +#include <utility> #include <vector> #ifndef _WIN32 @@ -544,15 +547,91 @@ loadExternalIndex(const Config::ExternalIndexSpec &External, log("Associating {0} with monolithic index at {1}.", External.MountPoint, External.Location); auto NewIndex = std::make_unique<SwapIndex>(std::make_unique<MemIndex>()); - Tasks.runAsync("Load-index:" + External.Location, - [File = External.Location, PlaceHolder = NewIndex.get()] { - if (auto Idx = loadIndex(File, /*UseDex=*/true)) - PlaceHolder->reset(std::move(Idx)); - }); + auto IndexLoadTask = [File = External.Location, + PlaceHolder = NewIndex.get()] { + if (auto Idx = loadIndex(File, /*UseDex=*/true)) + PlaceHolder->reset(std::move(Idx)); + }; + // FIXME: The signature should contain a null-able TaskRunner istead, and + // the task should be scheduled accordingly. + if (Sync) { + IndexLoadTask(); + } else { + Tasks.runAsync("Load-index:" + External.Location, + std::move(IndexLoadTask)); + } return std::move(NewIndex); } llvm_unreachable("Invalid ExternalIndexKind."); } + +class FlagsConfigProvider : public config::Provider { +private: + config::CompiledFragment Frag; + + std::vector<config::CompiledFragment> + getFragments(const config::Params &, + config::DiagnosticCallback) const override { + return {Frag}; + } + +public: + FlagsConfigProvider() { + llvm::Optional<Config::CDBSearchSpec> CDBSearch; + llvm::Optional<Config::ExternalIndexSpec> IndexSpec; + llvm::Optional<Config::BackgroundPolicy> BGPolicy; + + // If --compile-commands-dir arg was invoked, check value and override + // default path. + if (!CompileCommandsDir.empty()) { + if (llvm::sys::fs::exists(CompileCommandsDir)) { + // We support passing both relative and absolute paths to the + // --compile-commands-dir argument, but we assume the path is absolute + // in the rest of clangd so we make sure the path is absolute before + // continuing. + llvm::SmallString<128> Path(CompileCommandsDir); + if (std::error_code EC = llvm::sys::fs::make_absolute(Path)) { + elog("Error while converting the relative path specified by " + "--compile-commands-dir to an absolute path: {0}. The argument " + "will be ignored.", + EC.message()); + } else { + CDBSearch = {Config::CDBSearchSpec::FixedDir, Path.str().str()}; + } + } else { + elog("Path specified by --compile-commands-dir does not exist. The " + "argument will be ignored."); + } + } + if (!IndexFile.empty()) { + Config::ExternalIndexSpec Spec; + Spec.Kind = Spec.File; + Spec.Location = IndexFile; + IndexSpec = std::move(Spec); + } + if (!RemoteIndexAddress.empty()) { + assert(!ProjectRoot.empty() && IndexFile.empty()); + Config::ExternalIndexSpec Spec; + Spec.Kind = Spec.Server; + Spec.Location = RemoteIndexAddress; + Spec.MountPoint = ProjectRoot; + IndexSpec = std::move(Spec); + BGPolicy = Config::BackgroundPolicy::Skip; + } + if (!EnableBackgroundIndex) { + BGPolicy = Config::BackgroundPolicy::Skip; + } + Frag = [=](const config::Params &, Config &C) { + if (CDBSearch) + C.CompileFlags.CDBSearch = *CDBSearch; + if (IndexSpec) + C.Index.External = *IndexSpec; + if (BGPolicy) + C.Index.Background = *BGPolicy; + return true; + }; + } +}; } // namespace } // namespace clangd } // namespace clang @@ -693,29 +772,6 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var ClangdLSPServer::Options Opts; Opts.UseDirBasedCDB = (CompileArgsFrom == FilesystemCompileArgs); - // If --compile-commands-dir arg was invoked, check value and override default - // path. - if (!CompileCommandsDir.empty()) { - if (llvm::sys::fs::exists(CompileCommandsDir)) { - // We support passing both relative and absolute paths to the - // --compile-commands-dir argument, but we assume the path is absolute in - // the rest of clangd so we make sure the path is absolute before - // continuing. - llvm::SmallString<128> Path(CompileCommandsDir); - if (std::error_code EC = llvm::sys::fs::make_absolute(Path)) { - elog("Error while converting the relative path specified by " - "--compile-commands-dir to an absolute path: {0}. The argument " - "will be ignored.", - EC.message()); - } else { - Opts.CompileCommandsDir = std::string(Path.str()); - } - } else { - elog("Path specified by --compile-commands-dir does not exist. The " - "argument will be ignored."); - } - } - switch (PCHStorage) { case PCHStorageFlag::Memory: Opts.StorePreamblesInMemory = true; @@ -729,18 +785,6 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var Opts.BuildDynamicSymbolIndex = true; std::vector<std::unique_ptr<SymbolIndex>> IdxStack; std::unique_ptr<SymbolIndex> StaticIdx; - std::future<void> AsyncIndexLoad; // Block exit while loading the index. - if (!IndexFile.empty()) { - // Load the index asynchronously. Meanwhile SwapIndex returns no results. - SwapIndex *Placeholder; - StaticIdx.reset(Placeholder = new SwapIndex(std::make_unique<MemIndex>())); - AsyncIndexLoad = runAsync<void>([Placeholder] { - if (auto Idx = loadIndex(IndexFile, /*UseDex=*/true)) - Placeholder->reset(std::move(Idx)); - }); - if (Sync) - AsyncIndexLoad.wait(); - } #if CLANGD_ENABLE_REMOTE if (RemoteIndexAddress.empty() != ProjectRoot.empty()) { llvm::errs() << "remote-index-address and project-path have to be " @@ -750,8 +794,6 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var if (!RemoteIndexAddress.empty()) { if (IndexFile.empty()) { log("Connecting to remote index at {0}", RemoteIndexAddress); - StaticIdx = remote::getClient(RemoteIndexAddress, ProjectRoot); - EnableBackgroundIndex = false; } else { elog("When enabling remote index, IndexFile should not be specified. " "Only one can be used at time. Remote index will ignored."); @@ -802,12 +844,13 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var } else { elog("Couldn't determine user config file, not loading"); } - std::vector<const config::Provider *> ProviderPointers; - for (const auto &P : ProviderStack) - ProviderPointers.push_back(P.get()); - Config = config::Provider::combine(std::move(ProviderPointers)); - Opts.ConfigProvider = Config.get(); } + ProviderStack.push_back(std::make_unique<FlagsConfigProvider>()); + std::vector<const config::Provider *> ProviderPointers; + for (const auto &P : ProviderStack) + ProviderPointers.push_back(P.get()); + Config = config::Provider::combine(std::move(ProviderPointers)); + Opts.ConfigProvider = Config.get(); // Create an empty clang-tidy option. TidyProvider ClangTidyOptProvider; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
