From 9e9ac4138890425b92d2196344ab59305caa32d7 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya Date: Mon, 12 Apr 2021 17:21:21 +0200 Subject: [PATCH] [clangd] Drop optional on ExternalIndexSpec Differential Revision: https://reviews.llvm.org/D100308 --- clang-tools-extra/clangd/Config.h | 4 +-- clang-tools-extra/clangd/ConfigCompile.cpp | 2 +- clang-tools-extra/clangd/index/ProjectAware.cpp | 4 +-- clang-tools-extra/clangd/index/ProjectAware.h | 1 + .../clangd/unittests/ConfigCompileTests.cpp | 31 ++++++++++++---------- .../clangd/unittests/ProjectAwareIndexTests.cpp | 8 +++--- 6 files changed, 27 insertions(+), 23 deletions(-) diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index fe6f4d7..7269478 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 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 31beb6a..16d66f6 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 a6cdc86..3d5430d 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 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 888ef3a..c05f3d3 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( const Config::ExternalIndexSpec &, AsyncTaskRunner *)>; diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp index f999e0e..5e7fce8 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 c8ce0c0..724160d 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. -- 2.7.4