kadircet updated this revision to Diff 349378.
kadircet marked an inline comment as done.
kadircet added a comment.
Add comment to IndexFactory about pre-condition of spec never being none.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100308/new/
https://reviews.llvm.org/D100308
Files:
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
Index: clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
@@ -45,8 +45,8 @@
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 @@
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.
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -351,11 +351,14 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
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
}
Index: clang-tools-extra/clangd/index/ProjectAware.h
===================================================================
--- clang-tools-extra/clangd/index/ProjectAware.h
+++ clang-tools-extra/clangd/index/ProjectAware.h
@@ -20,6 +20,7 @@
/// 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 *)>;
Index: clang-tools-extra/clangd/index/ProjectAware.cpp
===================================================================
--- clang-tools-extra/clangd/index/ProjectAware.cpp
+++ clang-tools-extra/clangd/index/ProjectAware.cpp
@@ -128,9 +128,9 @@
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)
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -376,7 +376,7 @@
}
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,
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -70,7 +70,7 @@
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 {
/// Whether this TU should be indexed.
BackgroundPolicy Background = BackgroundPolicy::Build;
- llvm::Optional<ExternalIndexSpec> External;
+ ExternalIndexSpec External;
} Index;
/// Controls warnings and errors when parsing code.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
