kadircet created this revision.
kadircet added reviewers: sammccall, kbobyrev.
Herald added subscribers: usaxena95, jfb, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
This enables handling of queries like workspaceSymbols by the last used
external index, which might be null when the query comes from a file specific
context but that file doesn't have any ExternalIndex set (existing behaviour).
This also moves config::Params into Config, rather than just having a file path
inside the Config struct or deducing the environment from Context.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D103179
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigProvider.cpp
clang-tools-extra/clangd/ConfigProvider.h
clang-tools-extra/clangd/index/ProjectAware.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/ClangdTests.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
Index: clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
@@ -82,5 +82,44 @@
EXPECT_EQ(InvocationCount, 1U);
return;
}
+
+TEST(ProjectAware, LastIndex) {
+ IndexFactory Gen = [&](const Config::ExternalIndexSpec &Spec,
+ AsyncTaskRunner *) { return createIndex(); };
+
+ auto Idx = createProjectAwareIndex(std::move(Gen), true);
+ FuzzyFindRequest Req;
+ Req.Query = "1";
+ Req.AnyScope = true;
+
+ auto QueryWithoutPath = [&] {
+ Config C;
+ WithContextValue With(Config::Key, std::move(C));
+ return match(*Idx, Req);
+ };
+
+ // Empty without any external index before hand.
+ EXPECT_THAT(QueryWithoutPath(), IsEmpty());
+ // Now issue a request that will dispatch to an external index.
+ {
+ Config C;
+ C.Index.External.emplace();
+ C.Index.External->Location = "test";
+ WithContextValue With(Config::Key, std::move(C));
+ EXPECT_THAT(match(*Idx, Req), ElementsAre("1"));
+ }
+ // Should use the same external index.
+ EXPECT_THAT(QueryWithoutPath(), ElementsAre("1"));
+ // Issue a query without an external index.
+ {
+ Config C;
+ C.Parm.Path = "path";
+ WithContextValue With(Config::Key, std::move(C));
+ EXPECT_THAT(match(*Idx, Req), IsEmpty());
+ }
+ // Should use the last index, which was none.
+ EXPECT_THAT(QueryWithoutPath(), IsEmpty());
+ return;
+}
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp
@@ -24,6 +24,8 @@
using ::testing::ElementsAre;
using ::testing::IsEmpty;
+using Params = Config::Params;
+
// Provider that appends an arg to compile flags.
// The arg is prefix<N>, where N is the times getFragments() was called.
// It also yields a diagnostic each time it's called.
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -38,7 +38,7 @@
CapturedDiags Diags;
Config Conf;
Fragment Frag;
- Params Parm;
+ Config::Params Parm;
bool compileAndApply() {
Conf = Config();
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -338,7 +338,7 @@
// Provide conditional config that defines FOO for foo.cc.
class ConfigProvider : public config::Provider {
std::vector<config::CompiledFragment>
- getFragments(const config::Params &,
+ getFragments(const Config::Params &,
config::DiagnosticCallback DC) const override {
config::Fragment F;
F.If.PathMatch.emplace_back(".*foo.cc");
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -593,7 +593,7 @@
config::CompiledFragment Frag;
std::vector<config::CompiledFragment>
- getFragments(const config::Params &,
+ getFragments(const Config::Params &,
config::DiagnosticCallback) const override {
return {Frag};
}
@@ -647,7 +647,7 @@
BGPolicy = Config::BackgroundPolicy::Skip;
}
- Frag = [=](const config::Params &, Config &C) {
+ Frag = [=](const Config::Params &, Config &C) {
if (CDBSearch)
C.CompileFlags.CDBSearch = *CDBSearch;
if (IndexSpec)
Index: clang-tools-extra/clangd/index/ProjectAware.cpp
===================================================================
--- clang-tools-extra/clangd/index/ProjectAware.cpp
+++ clang-tools-extra/clangd/index/ProjectAware.cpp
@@ -20,9 +20,11 @@
#include "support/Trace.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ErrorHandling.h"
+#include <atomic>
#include <map>
#include <memory>
#include <mutex>
@@ -72,6 +74,8 @@
std::unique_ptr<SymbolIndex>>
IndexForSpec;
mutable std::unique_ptr<AsyncTaskRunner> Tasks;
+ // Points at the external index used to resolve last query. Might be null.
+ mutable std::atomic<SymbolIndex *> LastIndex = {};
const IndexFactory Gen;
};
@@ -128,14 +132,26 @@
SymbolIndex *ProjectAwareIndex::getIndex() const {
const auto &C = Config::current();
- if (!C.Index.External)
- return nullptr;
- const auto &External = *C.Index.External;
- std::lock_guard<std::mutex> Lock(Mu);
- auto Entry = IndexForSpec.try_emplace(External, nullptr);
- if (Entry.second)
- Entry.first->getSecond() = Gen(External, Tasks.get());
- return Entry.first->second.get();
+ auto GetIndexFromSpec =
+ [this](llvm::Optional<Config::ExternalIndexSpec> Spec) -> SymbolIndex * {
+ if (!Spec)
+ return nullptr;
+ const auto &External = *Spec;
+ std::lock_guard<std::mutex> Lock(Mu);
+ auto Entry = IndexForSpec.try_emplace(External, nullptr);
+ if (Entry.second)
+ Entry.first->getSecond() = Gen(External, Tasks.get());
+ return Entry.first->second.get();
+ };
+
+ // Use the last index when request is not originating from a particular file.
+ // This enables features like workspaceSymbols to keep working.
+ // Note that this might set LastIndex to nullptr when the user operates on a
+ // file without an external index. This is deliberate to support users with
+ // files open from projects with and without an external index.
+ if (C.Index.External || !C.Parm.Path.empty())
+ LastIndex = GetIndexFromSpec(C.Index.External);
+ return LastIndex;
}
} // namespace
Index: clang-tools-extra/clangd/ConfigProvider.h
===================================================================
--- clang-tools-extra/clangd/ConfigProvider.h
+++ clang-tools-extra/clangd/ConfigProvider.h
@@ -17,6 +17,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGPROVIDER_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGPROVIDER_H
+#include "Config.h"
#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/SMLoc.h"
@@ -27,22 +28,9 @@
namespace clang {
namespace clangd {
-struct Config;
class ThreadsafeFS;
namespace config {
-/// Describes the context used to evaluate configuration fragments.
-struct Params {
- /// Absolute path to a source file we're applying the config to. Unix slashes.
- /// Empty if not configuring a particular file.
- llvm::StringRef Path;
- /// Hint that stale data is OK to improve performance (e.g. avoid IO).
- /// FreshTime sets a bound for how old the data can be.
- /// By default, providers should validate caches against the data source.
- std::chrono::steady_clock::time_point FreshTime =
- std::chrono::steady_clock::time_point::max();
-};
-
/// Used to report problems in parsing or interpreting a config.
/// Errors reflect structurally invalid config that should be user-visible.
/// Warnings reflect e.g. unknown properties that are recoverable.
@@ -55,7 +43,7 @@
///
/// Calling it updates the configuration to reflect settings from the fragment.
/// Returns true if the condition was met and the settings were used.
-using CompiledFragment = std::function<bool(const Params &, Config &)>;
+using CompiledFragment = std::function<bool(const Config::Params &, Config &)>;
/// A source of configuration fragments.
/// Generally these providers reflect a fixed policy for obtaining config,
@@ -87,7 +75,7 @@
static std::unique_ptr<Provider> combine(std::vector<const Provider *>);
/// Build a config based on this provider.
- Config getConfig(const Params &, DiagnosticCallback) const;
+ Config getConfig(const Config::Params &, DiagnosticCallback) const;
private:
/// Provide fragments that may be relevant to the file.
@@ -99,7 +87,7 @@
///
/// When parsing/compiling, the DiagnosticCallback is used to report errors.
virtual std::vector<CompiledFragment>
- getFragments(const Params &, DiagnosticCallback) const = 0;
+ getFragments(const Config::Params &, DiagnosticCallback) const = 0;
};
} // namespace config
Index: clang-tools-extra/clangd/ConfigProvider.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -26,6 +26,8 @@
namespace clangd {
namespace config {
+using Params = Config::Params;
+
// Threadsafe cache around reading a YAML config file from disk.
class FileConfigCache : public FileCache {
mutable llvm::SmallVector<CompiledFragment, 1> CachedValue;
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -54,6 +54,8 @@
namespace config {
namespace {
+using Params = Config::Params;
+
// Returns an empty stringref if Path is not under FragmentDir. Returns Path
// as-is when FragmentDir is empty.
llvm::StringRef configRelative(llvm::StringRef Path,
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -29,6 +29,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringSet.h"
+#include <chrono>
#include <string>
#include <vector>
@@ -52,6 +53,21 @@
Config(Config &&) = default;
Config &operator=(Config &&) = default;
+ /// Describes the context used to evaluate configuration fragments.
+ struct Params {
+ /// Absolute path to a source file we're applying the config to. Unix
+ /// slashes. Empty if not configuring a particular file.
+ llvm::StringRef Path;
+ /// Hint that stale data is OK to improve performance (e.g. avoid IO).
+ /// FreshTime sets a bound for how old the data can be.
+ /// By default, providers should validate caches against the data source.
+ std::chrono::steady_clock::time_point FreshTime =
+ std::chrono::steady_clock::time_point::max();
+ };
+
+ /// Information about the environment used when generating the config.
+ Params Parm;
+
struct CDBSearchSpec {
enum { Ancestors, FixedDir, NoCDBSearch } Policy = Ancestors;
// Absolute, native slashes, no trailing slash.
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -274,20 +274,19 @@
: Provider(Provider), Publish(Publish) {}
Context operator()(llvm::StringRef File) {
- config::Params Params;
+ Config::Params P;
// Don't reread config files excessively often.
// FIXME: when we see a config file change event, use the event timestamp?
- Params.FreshTime =
- std::chrono::steady_clock::now() - std::chrono::seconds(5);
+ P.FreshTime = std::chrono::steady_clock::now() - std::chrono::seconds(5);
llvm::SmallString<256> PosixPath;
if (!File.empty()) {
assert(llvm::sys::path::is_absolute(File));
llvm::sys::path::native(File, PosixPath, llvm::sys::path::Style::posix);
- Params.Path = PosixPath.str();
+ P.Path = PosixPath.str();
}
llvm::StringMap<std::vector<Diag>> ReportableDiagnostics;
- Config C = Provider->getConfig(Params, [&](const llvm::SMDiagnostic &D) {
+ Config C = Provider->getConfig(P, [&](const llvm::SMDiagnostic &D) {
// Create the map entry even for note diagnostics we don't report.
// This means that when the file is parsed with no warnings, we
// publish an empty set of diagnostics, clearing any the client has.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
