[clang-tools-extra] dbf486c - [clangd] Config: Index.Background
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 14 10:04:18 PDT 2020
Author: Sam McCall
Date: 2020-07-14T19:04:11+02:00
New Revision: dbf486c0de92c76df77c1a1f815cf16533ecbb3a
URL: https://github.com/llvm/llvm-project/commit/dbf486c0de92c76df77c1a1f815cf16533ecbb3a
DIFF: https://github.com/llvm/llvm-project/commit/dbf486c0de92c76df77c1a1f815cf16533ecbb3a.diff
LOG: [clangd] Config: Index.Background
Summary:
We only support Build/Skip for now, but with 'Load' or similar as an
option for future (load existing shards but don't build new ones).
This requires creating the config for each TU on startup. In LLVM, this
is 4000 occurrences for a total of 800ms on my machine.
But together with caching from D83755 it is only 25ms.
Reviewers: kadircet
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D83768
Added:
Modified:
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index d2c3ef37abd5..f8dc2df51814 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -55,6 +55,13 @@ struct Config {
std::vector<llvm::unique_function<void(std::vector<std::string> &) const>>
Edits;
} CompileFlags;
+
+ enum class BackgroundPolicy { Build, Skip };
+ /// Controls background-index behavior.
+ struct {
+ /// Whether this TU should be indexed.
+ BackgroundPolicy Background = BackgroundPolicy::Build;
+ } Index;
};
} // namespace clangd
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index 568e029b5c0a..9b8a48fdaf7b 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -28,7 +28,9 @@
#include "ConfigFragment.h"
#include "support/Logger.h"
#include "support/Trace.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/Regex.h"
#include "llvm/Support/SMLoc.h"
#include "llvm/Support/SourceMgr.h"
@@ -80,9 +82,56 @@ struct FragmentCompiler {
return Result;
}
+ // Helper with similar API to StringSwitch, for parsing enum values.
+ template <typename T> class EnumSwitch {
+ FragmentCompiler &Outer;
+ llvm::StringRef EnumName;
+ const Located<std::string> &Input;
+ llvm::Optional<T> Result;
+ llvm::SmallVector<llvm::StringLiteral, 8> ValidValues;
+
+ public:
+ EnumSwitch(llvm::StringRef EnumName, const Located<std::string> &In,
+ FragmentCompiler &Outer)
+ : Outer(Outer), EnumName(EnumName), Input(In) {}
+
+ EnumSwitch &map(llvm::StringLiteral Name, T Value) {
+ assert(!llvm::is_contained(ValidValues, Name) && "Duplicate value!");
+ ValidValues.push_back(Name);
+ if (!Result && *Input == Name)
+ Result = Value;
+ return *this;
+ }
+
+ llvm::Optional<T> value() {
+ if (!Result)
+ Outer.diag(
+ Warning,
+ llvm::formatv("Invalid {0} value '{1}'. Valid values are {2}.",
+ EnumName, *Input, llvm::join(ValidValues, ", "))
+ .str(),
+ Input.Range);
+ return Result;
+ };
+ };
+
+ // Attempt to parse a specified string into an enum.
+ // Yields llvm::None and produces a diagnostic on failure.
+ //
+ // Optional<T> Value = compileEnum<En>("Foo", Frag.Foo)
+ // .map("Foo", Enum::Foo)
+ // .map("Bar", Enum::Bar)
+ // .value();
+ template <typename T>
+ EnumSwitch<T> compileEnum(llvm::StringRef EnumName,
+ const Located<std::string> &In) {
+ return EnumSwitch<T>(EnumName, In, *this);
+ }
+
void compile(Fragment &&F) {
compile(std::move(F.If));
compile(std::move(F.CompileFlags));
+ compile(std::move(F.Index));
}
void compile(Fragment::IfBlock &&F) {
@@ -148,7 +197,20 @@ struct FragmentCompiler {
}
}
+ void compile(Fragment::IndexBlock &&F) {
+ if (F.Background) {
+ if (auto Val = compileEnum<Config::BackgroundPolicy>("Background",
+ **F.Background)
+ .map("Build", Config::BackgroundPolicy::Build)
+ .map("Skip", Config::BackgroundPolicy::Skip)
+ .value())
+ Out.Apply.push_back([Val](Config &C) { C.Index.Background = *Val; });
+ }
+ }
+
constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
+ constexpr static llvm::SourceMgr::DiagKind Warning =
+ llvm::SourceMgr::DK_Warning;
void diag(llvm::SourceMgr::DiagKind Kind, llvm::StringRef Message,
llvm::SMRange Range) {
if (Range.isValid() && SourceMgr != nullptr)
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index 5cc8749c5efa..330f157326f2 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -150,6 +150,17 @@ struct Fragment {
std::vector<Located<std::string>> Remove;
};
CompileFlagsBlock CompileFlags;
+
+ /// Controls how clangd understands code outside the current file.
+ /// clangd's indexes provide information about symbols that isn't available
+ /// to clang's parser, such as incoming references.
+ struct IndexBlock {
+ /// Whether files are built in the background to produce a project index.
+ /// This is checked for translation units only, not headers they include.
+ /// Legal values are "Build" or "Skip".
+ llvm::Optional<Located<std::string>> Background;
+ };
+ IndexBlock Index;
};
} // namespace config
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index 7e86e3721248..16639f6649c2 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "ConfigFragment.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/MemoryBuffer.h"
@@ -70,6 +71,13 @@ class Parser {
Dict.parse(N);
}
+ void parse(Fragment::IndexBlock &F, Node &N) {
+ DictParser Dict("Index", this);
+ Dict.handle("Background",
+ [&](Node &N) { F.Background = scalarValue(N, "Background"); });
+ Dict.parse(N);
+ }
+
// Helper for parsing mapping nodes (dictionaries).
// We don't use YamlIO as we want to control over unknown keys.
class DictParser {
diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp
index 5024ace66b7c..a22785b01d64 100644
--- a/clang-tools-extra/clangd/index/Background.cpp
+++ b/clang-tools-extra/clangd/index/Background.cpp
@@ -8,6 +8,7 @@
#include "index/Background.h"
#include "Compiler.h"
+#include "Config.h"
#include "Headers.h"
#include "ParsedAST.h"
#include "SourceCode.h"
@@ -354,6 +355,14 @@ llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) {
// staleness.
std::vector<std::string>
BackgroundIndex::loadProject(std::vector<std::string> MainFiles) {
+ // Drop files where background indexing is disabled in config.
+ if (ContextProvider)
+ llvm::erase_if(MainFiles, [&](const std::string &TU) {
+ // Load the config for each TU, as indexing may be selectively enabled.
+ WithContext WithProvidedContext(ContextProvider(TU));
+ return Config::current().Index.Background ==
+ Config::BackgroundPolicy::Skip;
+ });
Rebuilder.startLoading();
// Load shards for all of the mainfiles.
const std::vector<LoadedShard> Result =
diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
index cb4d23e0be34..70d5156b1072 100644
--- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -113,7 +113,7 @@ TEST_F(BackgroundIndexTest, Config) {
// Set up two identical TUs, foo and bar.
// They define foo::one and bar::one.
std::vector<tooling::CompileCommand> Cmds;
- for (std::string Name : {"foo", "bar"}) {
+ for (std::string Name : {"foo", "bar", "baz"}) {
std::string Filename = Name + ".cpp";
std::string Header = Name + ".h";
FS.Files[Filename] = "#include \"" + Header + "\"";
@@ -126,11 +126,14 @@ TEST_F(BackgroundIndexTest, Config) {
}
// Context provider that installs a configuration mutating foo's command.
// This causes it to define foo::two instead of foo::one.
+ // It also disables indexing of baz entirely.
auto ContextProvider = [](PathRef P) {
Config C;
if (P.endswith("foo.cpp"))
C.CompileFlags.Edits.push_back(
[](std::vector<std::string> &Argv) { Argv.push_back("-Done=two"); });
+ if (P.endswith("baz.cpp"))
+ C.Index.Background = Config::BackgroundPolicy::Skip;
return Context::current().derive(Config::Key, std::move(C));
};
// Create the background index.
diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 033734789bed..c8465dc70edb 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -100,6 +100,22 @@ TEST_F(ConfigCompileTests, CompileCommands) {
EXPECT_THAT(Argv, ElementsAre("clang", "a.cc", "-foo"));
}
+TEST_F(ConfigCompileTests, Index) {
+ Frag.Index.Background.emplace("Skip");
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Skip);
+
+ Frag = {};
+ Frag.Index.Background.emplace("Foo");
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Build)
+ << "by default";
+ EXPECT_THAT(
+ Diags.Diagnostics,
+ ElementsAre(DiagMessage(
+ "Invalid Background value 'Foo'. Valid values are Build, Skip.")));
+}
+
} // namespace
} // namespace config
} // namespace clangd
More information about the cfe-commits
mailing list