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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
