From 4f1bbc0b842621d2048ed8687e328e2eab375b0a Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya Date: Fri, 5 Mar 2021 12:07:25 +0100 Subject: [PATCH] [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 --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 5 +- clang-tools-extra/clangd/ClangdLSPServer.h | 3 - clang-tools-extra/clangd/test/log.test | 6 +- clang-tools-extra/clangd/tool/Check.cpp | 7 +- clang-tools-extra/clangd/tool/ClangdMain.cpp | 137 ++++++++++++++++++--------- 5 files changed, 101 insertions(+), 57 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 8875f85..cd13e01 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("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(CDBOpts); diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 01d0ec2..fc13c62 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -48,9 +48,6 @@ public: /// 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 CompileCommandsDir; /// The offset-encoding to use, or None to negotiate it over LSP. llvm::Optional 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 5d60c10..7a53d36 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 &1 >/dev/null | FileCheck %s +# RUN: env CLANGD_FLAGS=-compile-commands-dir=no-such-dir not clangd -lit-test &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 9e3e439..00a4609 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 @@ public: 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 BaseCDB = std::make_unique(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 3afcd5f..eca30ee 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 #include #include +#include #include #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(std::make_unique()); - 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 + getFragments(const config::Params &, + config::DiagnosticCallback) const override { + return {Frag}; + } + +public: + FlagsConfigProvider() { + llvm::Optional CDBSearch; + llvm::Optional IndexSpec; + llvm::Optional 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> IdxStack; std::unique_ptr StaticIdx; - std::future 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())); - AsyncIndexLoad = runAsync([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 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()); + std::vector 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; -- 2.7.4