From c2ad7c23707cece995ee9070283a72c4afc8c0fe Mon Sep 17 00:00:00 2001 From: Sterling Augustine Date: Mon, 12 Apr 2021 14:17:49 -0700 Subject: [PATCH] Revert "[clangd] Provide a way to disable external index" 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 | 2 +- clang-tools-extra/clangd/ConfigCompile.cpp | 38 ++++++++-------------- clang-tools-extra/clangd/ConfigFragment.h | 3 -- clang-tools-extra/clangd/ConfigYAML.cpp | 24 +------------- .../clangd/unittests/ConfigCompileTests.cpp | 18 +++------- .../clangd/unittests/ConfigYAMLTests.cpp | 17 ---------- 6 files changed, 20 insertions(+), 82 deletions(-) diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index fe6f4d7..7064edd 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 { 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. diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index a574572..1185eb7 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -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; diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index 6b58b88..1365ed4 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -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 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> File; diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index f5739de..d50c011 100644 --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -12,7 +12,6 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/SourceMgr.h" #include "llvm/Support/YAMLParser.h" -#include #include 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 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"); }); diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp index 23347e2..f2c27c2 100644 --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -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) { diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp index d8fbf1a..0c216c20 100644 --- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp @@ -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( -- 2.7.4