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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits