[clangd] Config: Index.Background

[sammccall]

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.

[kadircet]

clang-tools-extra/clangd/ConfigCompile.cpp

100

should we assert on multiple matches of the same name ?

212

nit: I know this isn't what the patch is about, but.... what about making these lambdas/functions of Out.diag with first parameter bound to DK_Error or DK_Warning? (or having them at an anonymous namespace instead of a member)

clang-tools-extra/clangd/ConfigFragment.h

135

i think saying "outside the current file" might not be the best description. Maybe rather "Controls clangd's understanding for entities(symbols, refs, relations) in the codebase" ?

140

why not store the enum in here directly and generate diagnostics during the parsing stage?

we seem to be doing this for syntactic errors, e.g. expected a dictionary and getting an unexpected enum value sounds similar to that?

[sammccall]

Address some comments

clang-tools-extra/clangd/ConfigCompile.cpp

212

I'm not sure about the readability of lambdas here, if we wanted to do this I think the simplest thing would just be to make them methods?

Can we defer this question until after the branch cut?

clang-tools-extra/clangd/ConfigFragment.h

135

I don't really understand this - clangd's understanding for symbols, refs and, relations is all based on the AST, except when we're talking about outside the current file. (And refs and relations aren't terms that make much sense to users)

Expanded this a bit and gave an example.

140

Because then you have to do it twice for JSON vs YAML. You're right this isn't really principled, and I think more data points will help us resolve it.
OK to defer this to after the branch cut?

[kadircet]

let's ship it!

clang-tools-extra/clangd/ConfigCompile.cpp

212

SG 👍

clang-tools-extra/clangd/ConfigFragment.h

140

sure

clang-tools-extra/clangd/Config.h

struct Config {
   Config &operator=(Config &&) = default;
 
   /// Controls how the compile command for the current file is determined.
   struct {
     // Edits to apply to the compile command, in sequence.
     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
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/ConfigCompile.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "CompileCommands.h"
 #include "Config.h"
 #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"
 
 namespace clang {
 namespace clangd {
 namespace config {
 namespace {
llvm::Optional<llvm::Regex> compileRegex(const Located<std::string> &Text) {
     std::string RegexError;
     if (!Result.isValid(RegexError)) {
       diag(Error, "Invalid regex " + Anchored + ": " + RegexError, Text.Range);
       return llvm::None;
     }
     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) {
     if (F.HasUnrecognizedCondition)
       Out.Conditions.push_back([&](const Params &) { return false; });
 
     auto PathMatch = std::make_unique<std::vector<llvm::Regex>>();
     for (auto &Entry : F.PathMatch) {
if (!F.Add.empty()) {
       Out.Apply.push_back([Add(std::move(Add))](Config &C) {
         C.CompileFlags.Edits.push_back([Add](std::vector<std::string> &Args) {
           Args.insert(Args.end(), Add.begin(), Add.end());
         });
       });
     }
   }
 
+  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)
       Diagnostic(SourceMgr->GetMessage(Range.Start, Kind, Message, Range));
     else
       Diagnostic(llvm::SMDiagnostic("", Kind, Message));
   }
 };

clang-tools-extra/clangd/ConfigFragment.h

struct Fragment {
   /// searches for compile_commands.json in parents of the source file.
   ///
   /// This section modifies how the compile command is constructed.
   struct CompileFlagsBlock {
     /// List of flags to append to the compile command.
     std::vector<Located<std::string>> Add;
     /// List of flags to remove from the compile command.
     ///
     /// - If the value is a recognized clang flag (like "-I") then it will be
     ///   removed along with any arguments. Synonyms like --include-dir= will
     ///   also be removed.
     /// - Otherwise, if the value ends in * (like "-DFOO=*") then any argument
     ///   with the prefix will be removed.
     /// - Otherwise any argument exactly matching the value is removed.
     ///
     /// In all cases, -Xclang is also removed where needed.
     ///
     /// Example:
     ///   Command: clang++ --include-directory=/usr/include -DFOO=42 foo.cc
     ///   Remove: [-I, -DFOO=*]
     ///   Result: clang++ foo.cc
     ///
     /// Flags added by the same CompileFlags entry will not be removed.
     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
 } // namespace clangd
 } // namespace clang
 
 #endif

clang-tools-extra/clangd/ConfigYAML.cpp

 //===--- ConfigYAML.cpp - Loading configuration fragments from YAML files -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "ConfigFragment.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/YAMLParser.h"
 #include <system_error>
 
 namespace clang {
void parse(Fragment::CompileFlagsBlock &F, Node &N) {
     });
     Dict.handle("Remove", [&](Node &N) {
       if (auto Values = scalarValues(N))
         F.Remove = std::move(*Values);
     });
     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 {
     llvm::StringRef Description;
     std::vector<std::pair<llvm::StringRef, std::function<void(Node &)>>> Keys;
     std::function<void(llvm::StringRef)> Unknown;
     Parser *Outer;
 

clang-tools-extra/clangd/index/Background.cpp

 //===-- Background.cpp - Build an index in a background thread ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "index/Background.h"
 #include "Compiler.h"
+#include "Config.h"
 #include "Headers.h"
 #include "ParsedAST.h"
 #include "SourceCode.h"
 #include "Symbol.h"
 #include "URI.h"
 #include "index/BackgroundIndexLoader.h"
 #include "index/FileIndex.h"
 #include "index/IndexAction.h"
llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) {
   return llvm::Error::success();
 }
 
 // Restores shards for \p MainFiles from index storage. Then checks staleness of
 // those shards and returns a list of TUs that needs to be indexed to update
 // 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 =
       loadIndexShards(MainFiles, IndexStorageFactory, CDB);
   size_t LoadedShards = 0;
   {
     // Update in-memory state.
     std::lock_guard<std::mutex> Lock(ShardVersionsMu);

clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp

TEST_F(BackgroundIndexTest, NoCrashOnErrorFile) {
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
 }
 
 TEST_F(BackgroundIndexTest, Config) {
   MockFS FS;
   // 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 + "\"";
     FS.Files[Header] = "namespace " + Name + " { int one; }";
     tooling::CompileCommand Cmd;
     Cmd.Filename = Filename;
     Cmd.Directory = testRoot();
     Cmd.CommandLine = {"clang++", Filename};
     Cmds.push_back(std::move(Cmd));
   }
   // 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.
   llvm::StringMap<std::string> Storage;
   size_t CacheHits = 0;
   MemoryShardStorage MSS(Storage, CacheHits);
   // We need the CommandMangler, because that applies the config we're testing.
   OverlayCDB CDB(/*Base=*/nullptr, /*FallbackFlags=*/{},

clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

TEST_F(ConfigCompileTests, CompileCommands) {
   std::vector<std::string> Argv = {"clang", "-I", "bar/", "a.cc"};
   EXPECT_TRUE(compileAndApply());
   EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(2));
   for (auto &Edit : Conf.CompileFlags.Edits)
     Edit(Argv);
   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
 } // namespace clang