[clang-tools-extra] 4f1bbc0 - [clangd] Introduce a CommandLineConfigProvider
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 11 04:36:58 PST 2021
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;
More information about the cfe-commits
mailing list