[clang-tools-extra] 63bc9e4 - [clangd] Provide a way to disable external index
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 12 07:56:17 PDT 2021
Author: Kadir Cetinkaya
Date: 2021-04-12T16:43:23+02:00
New Revision: 63bc9e443502ab6def2dec0b5ffe64a522f801cc
URL: https://github.com/llvm/llvm-project/commit/63bc9e443502ab6def2dec0b5ffe64a522f801cc
DIFF: https://github.com/llvm/llvm-project/commit/63bc9e443502ab6def2dec0b5ffe64a522f801cc.diff
LOG: [clangd] Provide a way to disable external index
Users can reset any external index set by previous fragments by
putting a `None` for the external block, e.g:
```
Index:
External: None
```
Differential Revision: https://reviews.llvm.org/D100106
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index 7064edd76b8f4..fe6f4d7fa6e8e 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 { File, Server } Kind;
+ enum { None, 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 1185eb7255b4c..a5745727dca78 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -329,10 +329,11 @@ struct FragmentCompiler {
}
#endif
// Make sure exactly one of the Sources is set.
- unsigned SourceCount =
- External.File.hasValue() + External.Server.hasValue();
+ unsigned SourceCount = External.File.hasValue() +
+ External.Server.hasValue() + *External.IsNone;
if (SourceCount != 1) {
- diag(Error, "Exactly one of File or Server must be set.", BlockRange);
+ diag(Error, "Exactly one of File, Server or None must be set.",
+ BlockRange);
return;
}
Config::ExternalIndexSpec Spec;
@@ -346,20 +347,29 @@ struct FragmentCompiler {
if (!AbsPath)
return;
Spec.Location = std::move(*AbsPath);
+ } else {
+ assert(*External.IsNone);
+ 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;
+ 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);
}
- 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 1365ed4c1037c..6b58b88dfd490 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -173,6 +173,9 @@ 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;
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index d50c01168a8dd..f5739de0092d9 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -12,6 +12,7 @@
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/SourceMgr.h"
#include "llvm/Support/YAMLParser.h"
+#include <string>
#include <system_error>
namespace clang {
@@ -149,13 +150,34 @@ class Parser {
[&](Node &N) { F.Background = scalarValue(N, "Background"); });
Dict.handle("External", [&](Node &N) {
Fragment::IndexBlock::ExternalBlock External;
- parse(External, N);
+ // 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;
+ }
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"); });
diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index f2c27c22fdeb9..23347e2324feb 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -327,21 +327,31 @@ TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
#ifdef CLANGD_ENABLE_REMOTE
EXPECT_THAT(
Diags.Diagnostics,
- Contains(AllOf(DiagMessage("Exactly one of File or Server must be set."),
- DiagKind(llvm::SourceMgr::DK_Error))));
+ Contains(
+ AllOf(DiagMessage("Exactly one of File, Server or None 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 or Server must be set."),
- DiagKind(llvm::SourceMgr::DK_Error))));
+ Contains(
+ AllOf(DiagMessage("Exactly one of File, Server or None 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 0c216c208706b..d8fbf1a949a92 100644
--- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -147,6 +147,23 @@ 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(
More information about the cfe-commits
mailing list