Revert "[clangd] Provide a way to disable external index"
authorSterling Augustine <saugustine@google.com>
Mon, 12 Apr 2021 21:17:49 +0000 (14:17 -0700)
committerSterling Augustine <saugustine@google.com>
Mon, 12 Apr 2021 21:39:13 +0000 (14:39 -0700)
This reverts commit 63bc9e443502ab6def2dec0b5ffe64a522f801cc.

This breaks llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp:570:11:

with error: enumeration value 'None' not handled in switch [-Werror,-Wswitch]

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/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

index fe6f4d7..7064edd 100644 (file)
@@ -70,7 +70,7 @@ struct Config {
   enum class BackgroundPolicy { Build, Skip };
   /// Describes an external index configuration.
   struct ExternalIndexSpec {
-    enum { None, File, Server } Kind;
+    enum { File, Server } Kind;
     /// This is one of:
     /// - Address of a clangd-index-server, in the form of "ip:port".
     /// - Absolute path to an index produced by clangd-indexer.
index a574572..1185eb7 100644 (file)
@@ -329,11 +329,10 @@ struct FragmentCompiler {
     }
 #endif
     // Make sure exactly one of the Sources is set.
-    unsigned SourceCount = External.File.hasValue() +
-                           External.Server.hasValue() + *External.IsNone;
+    unsigned SourceCount =
+        External.File.hasValue() + External.Server.hasValue();
     if (SourceCount != 1) {
-      diag(Error, "Exactly one of File, Server or None must be set.",
-           BlockRange);
+      diag(Error, "Exactly one of File or Server must be set.", BlockRange);
       return;
     }
     Config::ExternalIndexSpec Spec;
@@ -347,29 +346,20 @@ struct FragmentCompiler {
       if (!AbsPath)
         return;
       Spec.Location = std::move(*AbsPath);
-    } else {
-      assert(*External.IsNone);
-      Spec.Kind = Config::ExternalIndexSpec::None;
     }
-    if (Spec.Kind != Config::ExternalIndexSpec::None) {
-      // Make sure MountPoint is an absolute path with forward slashes.
-      if (!External.MountPoint)
-        External.MountPoint.emplace(FragmentDirectory);
-      if ((**External.MountPoint).empty()) {
-        diag(Error, "A mountpoint is required.", BlockRange);
-        return;
-      }
-      auto AbsPath = makeAbsolute(std::move(*External.MountPoint), "MountPoint",
-                                  llvm::sys::path::Style::posix);
-      if (!AbsPath)
-        return;
-      Spec.MountPoint = std::move(*AbsPath);
+    // Make sure MountPoint is an absolute path with forward slashes.
+    if (!External.MountPoint)
+      External.MountPoint.emplace(FragmentDirectory);
+    if ((**External.MountPoint).empty()) {
+      diag(Error, "A mountpoint is required.", BlockRange);
+      return;
     }
+    auto AbsPath = makeAbsolute(std::move(*External.MountPoint), "MountPoint",
+                                llvm::sys::path::Style::posix);
+    if (!AbsPath)
+      return;
+    Spec.MountPoint = std::move(*AbsPath);
     Out.Apply.push_back([Spec(std::move(Spec))](const Params &P, Config &C) {
-      if (Spec.Kind == Config::ExternalIndexSpec::None) {
-        C.Index.External.reset();
-        return;
-      }
       if (P.Path.empty() || !pathStartsWith(Spec.MountPoint, P.Path,
                                             llvm::sys::path::Style::posix))
         return;
index 6b58b88..1365ed4 100644 (file)
@@ -173,9 +173,6 @@ struct Fragment {
     /// usually prepared using clangd-indexer.
     /// Exactly one source (File/Server) should be configured.
     struct ExternalBlock {
-      /// Whether the block is explicitly set to `None`. Can be used to clear
-      /// any external index specified before.
-      Located<bool> IsNone = false;
       /// Path to an index file generated by clangd-indexer. Relative paths may
       /// be used, if config fragment is associated with a directory.
       llvm::Optional<Located<std::string>> File;
index f5739de..d50c011 100644 (file)
@@ -12,7 +12,6 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/YAMLParser.h"
-#include <string>
 #include <system_error>
 
 namespace clang {
@@ -150,34 +149,13 @@ private:
                 [&](Node &N) { F.Background = scalarValue(N, "Background"); });
     Dict.handle("External", [&](Node &N) {
       Fragment::IndexBlock::ExternalBlock External;
-      // External block can either be a mapping or a scalar value. Dispatch
-      // accordingly.
-      if (N.getType() == Node::NK_Mapping) {
-        parse(External, N);
-      } else if (N.getType() == Node::NK_Scalar ||
-                 N.getType() == Node::NK_BlockScalar) {
-        parse(External, scalarValue(N, "External").getValue());
-      } else {
-        error("External must be either a scalar or a mapping.", N);
-        return;
-      }
+      parse(External, N);
       F.External.emplace(std::move(External));
       F.External->Range = N.getSourceRange();
     });
     Dict.parse(N);
   }
 
-  void parse(Fragment::IndexBlock::ExternalBlock &F,
-             Located<std::string> ExternalVal) {
-    if (!llvm::StringRef(*ExternalVal).equals_lower("none")) {
-      error("Only scalar value supported for External is 'None'",
-            ExternalVal.Range);
-      return;
-    }
-    F.IsNone = true;
-    F.IsNone.Range = ExternalVal.Range;
-  }
-
   void parse(Fragment::IndexBlock::ExternalBlock &F, Node &N) {
     DictParser Dict("External", this);
     Dict.handle("File", [&](Node &N) { F.File = scalarValue(N, "File"); });
index 23347e2..f2c27c2 100644 (file)
@@ -327,31 +327,21 @@ TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
 #ifdef CLANGD_ENABLE_REMOTE
   EXPECT_THAT(
       Diags.Diagnostics,
-      Contains(
-          AllOf(DiagMessage("Exactly one of File, Server or None must be set."),
-                DiagKind(llvm::SourceMgr::DK_Error))));
+      Contains(AllOf(DiagMessage("Exactly one of File or Server must be set."),
+                     DiagKind(llvm::SourceMgr::DK_Error))));
 #else
   ASSERT_TRUE(Conf.Index.External.hasValue());
   EXPECT_EQ(Conf.Index.External->Kind, Config::ExternalIndexSpec::File);
 #endif
 }
 
-TEST_F(ConfigCompileTests, ExternalBlockDisableWithNone) {
-  Fragment::IndexBlock::ExternalBlock External;
-  External.IsNone = true;
-  Frag.Index.External = std::move(External);
-  compileAndApply();
-  EXPECT_FALSE(Conf.Index.External.hasValue());
-}
-
 TEST_F(ConfigCompileTests, ExternalBlockErrOnNoSource) {
   Frag.Index.External.emplace(Fragment::IndexBlock::ExternalBlock{});
   compileAndApply();
   EXPECT_THAT(
       Diags.Diagnostics,
-      Contains(
-          AllOf(DiagMessage("Exactly one of File, Server or None must be set."),
-                DiagKind(llvm::SourceMgr::DK_Error))));
+      Contains(AllOf(DiagMessage("Exactly one of File or Server must be set."),
+                     DiagKind(llvm::SourceMgr::DK_Error))));
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockDisablesBackgroundIndex) {
index d8fbf1a..0c216c2 100644 (file)
@@ -147,23 +147,6 @@ horrible
   ASSERT_THAT(Results, IsEmpty());
 }
 
-TEST(ParseYAML, ExternalBlockNone) {
-  CapturedDiags Diags;
-  Annotations YAML(R"yaml(
-Index:
-  External: None
-  )yaml");
-  auto Results =
-      Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
-  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_EQ(Results.size(), 1u);
-  ASSERT_TRUE(Results[0].Index.External);
-  EXPECT_FALSE(Results[0].Index.External.getValue()->File.hasValue());
-  EXPECT_FALSE(Results[0].Index.External.getValue()->MountPoint.hasValue());
-  EXPECT_FALSE(Results[0].Index.External.getValue()->Server.hasValue());
-  EXPECT_THAT(*Results[0].Index.External.getValue()->IsNone, testing::Eq(true));
-}
-
 TEST(ParseYAML, ExternalBlock) {
   CapturedDiags Diags;
   Annotations YAML(R"yaml(