[PATCH] D90775: [clangd] ExternalIndex turns off BackgroundIndex only if it isn't set

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: ormris, MaskRay, ilya-biryukov.

This will enable users to have a `Background: Build` in their in-project
configs, while having an external index specification in their user
config file.

Depends on D90749 <https://reviews.llvm.org/D90749>.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90775

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp


Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -117,7 +117,7 @@
   Frag = {};
   Frag.Index.Background.emplace("Foo");
   EXPECT_TRUE(compileAndApply());
-  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Build)
+  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Auto)
       << "by default";
   EXPECT_THAT(
       Diags.Diagnostics,
@@ -205,13 +205,27 @@
           DiagKind(llvm::SourceMgr::DK_Error))));
 }
 
-TEST_F(ConfigCompileTests, ExternalBlockDisablesBackgroundIndex) {
+TEST_F(ConfigCompileTests, ExternalBlockAndBackgroundIndex) {
   Parm.Path = "/foo/bar/baz.h";
-  Frag.Index.Background.emplace("Build");
-  Fragment::IndexBlock::ExternalBlock External;
-  External.File.emplace("/foo");
-  External.MountPoint.emplace("/foo/bar");
-  Frag.Index.External = std::move(External);
+  auto CreateFragWithBackground = [](llvm::StringRef Background) {
+    Fragment::IndexBlock::ExternalBlock External;
+    External.File.emplace("/foo");
+    External.MountPoint.emplace("/foo/bar");
+
+    Fragment Frag;
+    Frag.Index.External = std::move(External);
+    if (!Background.empty())
+      Frag.Index.Background.emplace(Background.str());
+    return Frag;
+  };
+
+  // Preserves when explicitly mentioned.
+  Frag = CreateFragWithBackground("Build");
+  compileAndApply();
+  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Build);
+
+  // Overwrites to Skip otherwise.
+  Frag = CreateFragWithBackground("");
   compileAndApply();
   EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Skip);
 }
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -302,10 +302,10 @@
           else
             C.Index.External.Server.emplace(std::move(**External.Server));
 
-          // Disable background indexing for the files under the mountpoint.
-          // Note that this will overwrite statements in any previous fragments
-          // (including the current one).
-          C.Index.Background = Config::BackgroundPolicy::Skip;
+          // Disable background indexing for the files under the mountpoint. If
+          // user didn't take any explicit actions.
+          if (C.Index.Background == Config::BackgroundPolicy::Auto)
+            C.Index.Background = Config::BackgroundPolicy::Skip;
         });
   }
 
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -57,11 +57,11 @@
         Edits;
   } CompileFlags;
 
-  enum class BackgroundPolicy { Build, Skip };
+  enum class BackgroundPolicy { Auto, Build, Skip };
   /// Controls background-index behavior.
   struct {
     /// Whether this TU should be indexed.
-    BackgroundPolicy Background = BackgroundPolicy::Build;
+    BackgroundPolicy Background = BackgroundPolicy::Auto;
     /// Describes an external index configuration. At most one of Server or File
     /// will be set at a time.
     struct {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D90775.302895.patch
Type: text/x-patch
Size: 3336 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201104/efaec870/attachment.bin>