[clang-tools-extra] 9e9ac41 - [clangd] Drop optional on ExternalIndexSpec

Kadir Cetinkaya via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 2 14:27:28 PDT 2021


Author: Kadir Cetinkaya
Date: 2021-06-02T23:26:37+02:00
New Revision: 9e9ac4138890425b92d2196344ab59305caa32d7

URL: https://github.com/llvm/llvm-project/commit/9e9ac4138890425b92d2196344ab59305caa32d7
DIFF: https://github.com/llvm/llvm-project/commit/9e9ac4138890425b92d2196344ab59305caa32d7.diff

LOG: [clangd] Drop optional on ExternalIndexSpec

Differential Revision: https://reviews.llvm.org/D100308

Added: 
    

Modified: 
    clang-tools-extra/clangd/Config.h
    clang-tools-extra/clangd/ConfigCompile.cpp
    clang-tools-extra/clangd/index/ProjectAware.cpp
    clang-tools-extra/clangd/index/ProjectAware.h
    clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
    clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index fe6f4d7fa6e8e..72694788cfbfa 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -70,7 +70,7 @@ struct Config {
   enum class BackgroundPolicy { Build, Skip };
   /// Describes an external index configuration.
   struct ExternalIndexSpec {
-    enum { None, File, Server } Kind;
+    enum { None, File, Server } Kind = None;
     /// 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.
@@ -83,7 +83,7 @@ struct Config {
   struct {
     /// Whether this TU should be indexed.
     BackgroundPolicy Background = BackgroundPolicy::Build;
-    llvm::Optional<ExternalIndexSpec> External;
+    ExternalIndexSpec External;
   } Index;
 
   /// Controls warnings and errors when parsing code.

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index 31beb6ac10cb2..16d66f6154f25 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -376,7 +376,7 @@ struct FragmentCompiler {
     }
     Out.Apply.push_back([Spec(std::move(Spec))](const Params &P, Config &C) {
       if (Spec.Kind == Config::ExternalIndexSpec::None) {
-        C.Index.External.reset();
+        C.Index.External = Spec;
         return;
       }
       if (P.Path.empty() || !pathStartsWith(Spec.MountPoint, P.Path,

diff  --git a/clang-tools-extra/clangd/index/ProjectAware.cpp b/clang-tools-extra/clangd/index/ProjectAware.cpp
index a6cdc86fdc249..3d5430d2b01d8 100644
--- a/clang-tools-extra/clangd/index/ProjectAware.cpp
+++ b/clang-tools-extra/clangd/index/ProjectAware.cpp
@@ -128,9 +128,9 @@ ProjectAwareIndex::indexedFiles() const {
 
 SymbolIndex *ProjectAwareIndex::getIndex() const {
   const auto &C = Config::current();
-  if (!C.Index.External)
+  if (C.Index.External.Kind == Config::ExternalIndexSpec::None)
     return nullptr;
-  const auto &External = *C.Index.External;
+  const auto &External = C.Index.External;
   std::lock_guard<std::mutex> Lock(Mu);
   auto Entry = IndexForSpec.try_emplace(External, nullptr);
   if (Entry.second)

diff  --git a/clang-tools-extra/clangd/index/ProjectAware.h b/clang-tools-extra/clangd/index/ProjectAware.h
index 888ef3add2ebb..c05f3d307c419 100644
--- a/clang-tools-extra/clangd/index/ProjectAware.h
+++ b/clang-tools-extra/clangd/index/ProjectAware.h
@@ -20,6 +20,7 @@ namespace clangd {
 /// A functor to create an index for an external index specification. Functor
 /// should perform any high latency operation in a separate thread through
 /// AsyncTaskRunner, if set.
+/// Spec is never `None`.
 using IndexFactory = std::function<std::unique_ptr<SymbolIndex>(
     const Config::ExternalIndexSpec &, AsyncTaskRunner *)>;
 

diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index f999e0ec2c357..5e7fce8016f12 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -328,7 +328,7 @@ TEST_F(ConfigCompileTests, ExternalServerNeedsTrusted) {
       ElementsAre(DiagMessage(
           "Remote index may not be specified by untrusted configuration. "
           "Copy this into user config to use it.")));
-  EXPECT_FALSE(Conf.Index.External.hasValue());
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
@@ -351,11 +351,14 @@ TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockDisableWithNone) {
+  compileAndApply();
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
+
   Fragment::IndexBlock::ExternalBlock External;
   External.IsNone = true;
   Frag.Index.External = std::move(External);
   compileAndApply();
-  EXPECT_FALSE(Conf.Index.External.hasValue());
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockErrOnNoSource) {
@@ -406,7 +409,7 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
           AllOf(DiagMessage("MountPoint must be an absolute path, because this "
                             "fragment is not associated with any directory."),
                 DiagKind(llvm::SourceMgr::DK_Error))));
-  ASSERT_FALSE(Conf.Index.External);
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
   FooPath = llvm::sys::path::convert_to_slash(FooPath);
@@ -414,22 +417,22 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // None defaults to ".".
   Frag = GetFrag(FooPath, llvm::None);
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // Without a file, external index is empty.
   Parm.Path = "";
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_FALSE(Conf.Index.External);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
@@ -438,7 +441,7 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_FALSE(Conf.Index.External);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
@@ -447,8 +450,8 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // Only matches case-insensitively.
   BazPath = testPath("fOo/baz.h", llvm::sys::path::Style::posix);
@@ -461,10 +464,10 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
 #ifdef CLANGD_PATH_CASE_INSENSITIVE
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 #else
-  ASSERT_FALSE(Conf.Index.External);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 #endif
 }
 

diff  --git a/clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp b/clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
index c8ce0c0d154d7..724160dbfb7ca 100644
--- a/clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
@@ -45,8 +45,8 @@ TEST(ProjectAware, Test) {
   EXPECT_THAT(match(*Idx, Req), IsEmpty());
 
   Config C;
-  C.Index.External.emplace();
-  C.Index.External->Location = "test";
+  C.Index.External.Kind = Config::ExternalIndexSpec::File;
+  C.Index.External.Location = "test";
   WithContextValue With(Config::Key, std::move(C));
   EXPECT_THAT(match(*Idx, Req), ElementsAre("1"));
   return;
@@ -71,8 +71,8 @@ TEST(ProjectAware, CreatedOnce) {
   EXPECT_EQ(InvocationCount, 0U);
 
   Config C;
-  C.Index.External.emplace();
-  C.Index.External->Location = "test";
+  C.Index.External.Kind = Config::ExternalIndexSpec::File;
+  C.Index.External.Location = "test";
   WithContextValue With(Config::Key, std::move(C));
   match(*Idx, Req);
   // Now it should be created.


        


More information about the cfe-commits mailing list