[clang-tools-extra] 1eb7fd0 - [clangd] Remove some obsolete options that are now always on
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 1 08:24:31 PST 2021
Author: Sam McCall
Date: 2021-02-01T17:24:03+01:00
New Revision: 1eb7fd089e2fcf3fe211f865b28e2fed12128c3f
URL: https://github.com/llvm/llvm-project/commit/1eb7fd089e2fcf3fe211f865b28e2fed12128c3f
DIFF: https://github.com/llvm/llvm-project/commit/1eb7fd089e2fcf3fe211f865b28e2fed12128c3f.diff
LOG: [clangd] Remove some obsolete options that are now always on
- always collect main-file refs when indexing
- always build preambles asynchronously
- always use dex for fast preamble index
Retire associated flags
Differential Revision: https://reviews.llvm.org/D95571
Added:
Modified:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/index/Background.h
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/FileIndex.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index ab9903197d08..fd89ec1c45dc 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -122,7 +122,6 @@ ClangdServer::Options ClangdServer::optsForTest() {
Opts.StorePreamblesInMemory = true;
Opts.AsyncThreadsCount = 4; // Consistent!
Opts.TheiaSemanticHighlighting = true;
- Opts.AsyncPreambleBuilds = true;
return Opts;
}
@@ -132,7 +131,6 @@ ClangdServer::Options::operator TUScheduler::Options() const {
Opts.RetentionPolicy = RetentionPolicy;
Opts.StorePreamblesInMemory = StorePreamblesInMemory;
Opts.UpdateDebounce = UpdateDebounce;
- Opts.AsyncPreambleBuilds = AsyncPreambleBuilds;
Opts.ContextProvider = ContextProvider;
return Opts;
}
@@ -141,10 +139,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
const ThreadsafeFS &TFS, const Options &Opts,
Callbacks *Callbacks)
: CDB(CDB), TFS(TFS),
- DynamicIdx(Opts.BuildDynamicSymbolIndex
- ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex,
- Opts.CollectMainFileRefs)
- : nullptr),
+ DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
ClangTidyProvider(Opts.ClangTidyProvider),
WorkspaceRoot(Opts.WorkspaceRoot),
// Pass a callback into `WorkScheduler` to extract symbols from a newly
@@ -175,7 +170,6 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
Callbacks->onBackgroundIndexProgress(S);
};
BGOpts.ContextProvider = Opts.ContextProvider;
- BGOpts.CollectMainFileRefs = Opts.CollectMainFileRefs;
BackgroundIdx = std::make_unique<BackgroundIndex>(
TFS, CDB,
BackgroundIndexStorage::createDiskBackedStorageFactory(
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index fd5a4bf9a40d..e3de7013ba32 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -97,22 +97,14 @@ class ClangdServer {
/// Cached preambles are potentially large. If false, store them on disk.
bool StorePreamblesInMemory = true;
- /// Reuse even stale preambles, and rebuild them in the background.
- /// This improves latency at the cost of accuracy.
- bool AsyncPreambleBuilds = true;
/// If true, ClangdServer builds a dynamic in-memory index for symbols in
/// opened files and uses the index to augment code completion results.
bool BuildDynamicSymbolIndex = false;
- /// Use a heavier and faster in-memory index implementation.
- bool HeavyweightDynamicSymbolIndex = true;
/// If true, ClangdServer automatically indexes files in the current project
/// on background threads. The index is stored in the project root.
bool BackgroundIndex = false;
- /// Store refs to main-file symbols in the index.
- bool CollectMainFileRefs = true;
-
/// If set, use this index to augment code completion results.
SymbolIndex *StaticIndex = nullptr;
diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp
index 1cd669945198..dd4312c4a7c2 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -596,8 +596,8 @@ ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
FileName(FileName), ContextProvider(Opts.ContextProvider), CDB(CDB),
Callbacks(Callbacks), Barrier(Barrier), Done(false),
Status(FileName, Callbacks),
- PreamblePeer(FileName, Callbacks, Opts.StorePreamblesInMemory,
- RunSync || !Opts.AsyncPreambleBuilds, Status, *this) {
+ PreamblePeer(FileName, Callbacks, Opts.StorePreamblesInMemory, RunSync,
+ Status, *this) {
// Set a fallback command because compile command can be accessed before
// `Inputs` is initialized. Other fields are only used after initialization
// from client inputs.
diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h
index c8b75b3f4127..095cd0b41ca5 100644
--- a/clang-tools-extra/clangd/TUScheduler.h
+++ b/clang-tools-extra/clangd/TUScheduler.h
@@ -196,10 +196,6 @@ class TUScheduler {
/// Determines when to keep idle ASTs in memory for future use.
ASTRetentionPolicy RetentionPolicy;
- /// Whether to run PreamblePeer asynchronously.
- /// No-op if AsyncThreadsCount is 0.
- bool AsyncPreambleBuilds = true;
-
/// Used to create a context that wraps each single operation.
/// Typically to inject per-file configuration.
/// If the path is empty, context sholud be "generic".
diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp
index e4ce1f57ff2f..d122d8aa2776 100644
--- a/clang-tools-extra/clangd/index/Background.cpp
+++ b/clang-tools-extra/clangd/index/Background.cpp
@@ -97,7 +97,6 @@ BackgroundIndex::BackgroundIndex(
BackgroundIndexStorage::Factory IndexStorageFactory, Options Opts)
: SwapIndex(std::make_unique<MemIndex>()), TFS(TFS), CDB(CDB),
ContextProvider(std::move(Opts.ContextProvider)),
- CollectMainFileRefs(Opts.CollectMainFileRefs),
Rebuilder(this, &IndexedSymbols, Opts.ThreadPoolSize),
IndexStorageFactory(std::move(IndexStorageFactory)),
Queue(std::move(Opts.OnProgress)),
@@ -304,7 +303,7 @@ llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) {
return false; // Skip files that haven't changed, without errors.
return true;
};
- IndexOpts.CollectMainFileRefs = CollectMainFileRefs;
+ IndexOpts.CollectMainFileRefs = true;
IndexFileIn Index;
auto Action = createStaticIndexingAction(
diff --git a/clang-tools-extra/clangd/index/Background.h b/clang-tools-extra/clangd/index/Background.h
index fbcec7014957..808061b17dff 100644
--- a/clang-tools-extra/clangd/index/Background.h
+++ b/clang-tools-extra/clangd/index/Background.h
@@ -143,8 +143,6 @@ class BackgroundIndex : public SwapIndex {
// file. Called with the empty string for other tasks.
// (When called, the context from BackgroundIndex construction is active).
std::function<Context(PathRef)> ContextProvider = nullptr;
- // Whether to collect references to main-file-only symbols.
- bool CollectMainFileRefs = false;
};
/// Creates a new background index and starts its threads.
@@ -198,7 +196,6 @@ class BackgroundIndex : public SwapIndex {
const ThreadsafeFS &TFS;
const GlobalCompilationDatabase &CDB;
std::function<Context(PathRef)> ContextProvider;
- bool CollectMainFileRefs;
llvm::Error index(tooling::CompileCommand);
diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp b/clang-tools-extra/clangd/index/FileIndex.cpp
index 26084c288674..9f05968020b0 100644
--- a/clang-tools-extra/clangd/index/FileIndex.cpp
+++ b/clang-tools-extra/clangd/index/FileIndex.cpp
@@ -217,11 +217,11 @@ FileShardedIndex::getShard(llvm::StringRef Uri) const {
return std::move(IF);
}
-SlabTuple indexMainDecls(ParsedAST &AST, bool CollectMainFileRefs) {
+SlabTuple indexMainDecls(ParsedAST &AST) {
return indexSymbols(
AST.getASTContext(), AST.getPreprocessorPtr(),
AST.getLocalTopLevelDecls(), &AST.getMacros(), AST.getCanonicalIncludes(),
- /*IsIndexMainAST=*/true, AST.version(), CollectMainFileRefs);
+ /*IsIndexMainAST=*/true, AST.version(), /*CollectMainFileRefs=*/true);
}
SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST,
@@ -410,9 +410,8 @@ void FileSymbols::profile(MemoryTree &MT) const {
}
}
-FileIndex::FileIndex(bool UseDex, bool CollectMainFileRefs)
- : MergedIndex(&MainFileIndex, &PreambleIndex), UseDex(UseDex),
- CollectMainFileRefs(CollectMainFileRefs),
+FileIndex::FileIndex()
+ : MergedIndex(&MainFileIndex, &PreambleIndex),
PreambleIndex(std::make_unique<MemIndex>()),
MainFileIndex(std::make_unique<MemIndex>()) {}
@@ -436,9 +435,8 @@ void FileIndex::updatePreamble(PathRef Path, llvm::StringRef Version,
/*CountReferences=*/false);
}
size_t IndexVersion = 0;
- auto NewIndex =
- PreambleSymbols.buildIndex(UseDex ? IndexType::Heavy : IndexType::Light,
- DuplicateHandling::PickOne, &IndexVersion);
+ auto NewIndex = PreambleSymbols.buildIndex(
+ IndexType::Heavy, DuplicateHandling::PickOne, &IndexVersion);
{
std::lock_guard<std::mutex> Lock(UpdateIndexMu);
if (IndexVersion <= PreambleIndexVersion) {
@@ -455,7 +453,7 @@ void FileIndex::updatePreamble(PathRef Path, llvm::StringRef Version,
}
void FileIndex::updateMain(PathRef Path, ParsedAST &AST) {
- auto Contents = indexMainDecls(AST, CollectMainFileRefs);
+ auto Contents = indexMainDecls(AST);
MainFileSymbols.update(
Path, std::make_unique<SymbolSlab>(std::move(std::get<0>(Contents))),
std::make_unique<RefSlab>(std::move(std::get<1>(Contents))),
diff --git a/clang-tools-extra/clangd/index/FileIndex.h b/clang-tools-extra/clangd/index/FileIndex.h
index 8ecae66373a5..29958ca30be2 100644
--- a/clang-tools-extra/clangd/index/FileIndex.h
+++ b/clang-tools-extra/clangd/index/FileIndex.h
@@ -107,7 +107,7 @@ class FileSymbols {
/// FIXME: Expose an interface to remove files that are closed.
class FileIndex : public MergedIndex {
public:
- FileIndex(bool UseDex = true, bool CollectMainFileRefs = false);
+ FileIndex();
/// Update preamble symbols of file \p Path with all declarations in \p AST
/// and macros in \p PP.
@@ -122,9 +122,6 @@ class FileIndex : public MergedIndex {
void profile(MemoryTree &MT) const;
private:
- bool UseDex; // FIXME: this should be always on.
- bool CollectMainFileRefs;
-
// Contains information from each file's preamble only. Symbols and relations
// are sharded per declaration file to deduplicate multiple symbols and reduce
// memory usage.
@@ -158,7 +155,7 @@ using SlabTuple = std::tuple<SymbolSlab, RefSlab, RelationSlab>;
/// Retrieves symbols and refs of local top level decls in \p AST (i.e.
/// `AST.getLocalTopLevelDecls()`).
/// Exposed to assist in unit tests.
-SlabTuple indexMainDecls(ParsedAST &AST, bool CollectMainFileRefs = false);
+SlabTuple indexMainDecls(ParsedAST &AST);
/// Index declarations from \p AST and macros from \p PP that are declared in
/// included headers.
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index fe69079bfe67..6a61f0db5294 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -281,6 +281,11 @@ opt<bool> IncludeIneligibleResults{
};
RetiredFlag<bool> EnableIndex("index");
+RetiredFlag<bool> SuggestMissingIncludes("suggest-missing-includes");
+RetiredFlag<bool> RecoveryAST("recovery-ast");
+RetiredFlag<bool> RecoveryASTType("recovery-ast-type");
+RetiredFlag<bool> AsyncPreamble("async-preamble");
+RetiredFlag<bool> CollectMainFileRefs("collect-main-file-refs");
opt<int> LimitResults{
"limit-results",
@@ -290,7 +295,6 @@ opt<int> LimitResults{
init(100),
};
-RetiredFlag<bool> SuggestMissingIncludes("suggest-missing-includes");
list<std::string> TweakList{
"tweaks",
@@ -307,9 +311,6 @@ opt<bool> CrossFileRename{
init(true),
};
-RetiredFlag<bool> RecoveryAST("recovery-ast");
-RetiredFlag<bool> RecoveryASTType("recovery-ast-type");
-
opt<bool> FoldingRanges{
"folding-ranges",
cat(Features),
@@ -450,16 +451,6 @@ opt<bool> PrettyPrint{
init(false),
};
-// FIXME: retire this flag in llvm 13 release cycle.
-opt<bool> AsyncPreamble{
- "async-preamble",
- cat(Misc),
- desc("Reuse even stale preambles, and rebuild them in the background. This "
- "improves latency at the cost of accuracy."),
- init(ClangdServer::Options().AsyncPreambleBuilds),
- Hidden,
-};
-
opt<bool> EnableConfig{
"enable-config",
cat(Misc),
@@ -474,15 +465,6 @@ opt<bool> EnableConfig{
init(true),
};
-// FIXME: retire this flag in llvm 13 release cycle.
-opt<bool> CollectMainFileRefs{
- "collect-main-file-refs",
- cat(Misc),
- desc("Store references to main-file-only symbols in the index"),
- init(ClangdServer::Options().CollectMainFileRefs),
- Hidden,
-};
-
#if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
opt<bool> EnableMallocTrim{
"malloc-trim",
@@ -760,7 +742,6 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
if (!ResourceDir.empty())
Opts.ResourceDir = ResourceDir;
Opts.BuildDynamicSymbolIndex = true;
- Opts.CollectMainFileRefs = CollectMainFileRefs;
std::vector<std::unique_ptr<SymbolIndex>> IdxStack;
std::unique_ptr<SymbolIndex> StaticIdx;
std::future<void> AsyncIndexLoad; // Block exit while loading the index.
@@ -860,7 +841,6 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
ClangTidyOptProvider = combine(std::move(Providers));
Opts.ClangTidyProvider = ClangTidyOptProvider;
}
- Opts.AsyncPreambleBuilds = AsyncPreamble;
Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
Opts.TweakFilter = [&](const Tweak &T) {
if (T.hidden() && !HiddenFeatures)
diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
index bcbedd2500ce..4cff911ebd9d 100644
--- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -190,7 +190,6 @@ TEST_F(BackgroundIndexTest, IndexTwoFiles) {
MemoryShardStorage MSS(Storage, CacheHits);
OverlayCDB CDB(/*Base=*/nullptr);
BackgroundIndex::Options Opts;
- Opts.CollectMainFileRefs = true;
BackgroundIndex Idx(
FS, CDB, [&](llvm::StringRef) { return &MSS; }, Opts);
@@ -241,52 +240,25 @@ TEST_F(BackgroundIndexTest, MainFileRefs) {
FS.Files[testPath("root/A.cc")] =
"#include \"A.h\"\nstatic void main_sym() { (void)header_sym; }";
- // Check the behaviour with CollectMainFileRefs = false (the default
- // at the SymbolCollector level).
- {
- llvm::StringMap<std::string> Storage;
- size_t CacheHits = 0;
- MemoryShardStorage MSS(Storage, CacheHits);
- OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
- /*Opts=*/{});
-
- tooling::CompileCommand Cmd;
- Cmd.Filename = testPath("root/A.cc");
- Cmd.Directory = testPath("root");
- Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
- CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
-
- ASSERT_TRUE(Idx.blockUntilIdleForTest());
- EXPECT_THAT(
- runFuzzyFind(Idx, ""),
- UnorderedElementsAre(AllOf(Named("header_sym"), NumReferences(1U)),
- AllOf(Named("main_sym"), NumReferences(0U))));
- }
-
- // Check the behaviour with CollectMainFileRefs = true.
- {
- llvm::StringMap<std::string> Storage;
- size_t CacheHits = 0;
- MemoryShardStorage MSS(Storage, CacheHits);
- OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex::Options Opts;
- Opts.CollectMainFileRefs = true;
- BackgroundIndex Idx(
- FS, CDB, [&](llvm::StringRef) { return &MSS; }, Opts);
+ llvm::StringMap<std::string> Storage;
+ size_t CacheHits = 0;
+ MemoryShardStorage MSS(Storage, CacheHits);
+ OverlayCDB CDB(/*Base=*/nullptr);
+ BackgroundIndex::Options Opts;
+ BackgroundIndex Idx(
+ FS, CDB, [&](llvm::StringRef) { return &MSS; }, Opts);
- tooling::CompileCommand Cmd;
- Cmd.Filename = testPath("root/A.cc");
- Cmd.Directory = testPath("root");
- Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
- CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+ tooling::CompileCommand Cmd;
+ Cmd.Filename = testPath("root/A.cc");
+ Cmd.Directory = testPath("root");
+ Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+ CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
- ASSERT_TRUE(Idx.blockUntilIdleForTest());
- EXPECT_THAT(
- runFuzzyFind(Idx, ""),
- UnorderedElementsAre(AllOf(Named("header_sym"), NumReferences(1U)),
- AllOf(Named("main_sym"), NumReferences(1U))));
- }
+ ASSERT_TRUE(Idx.blockUntilIdleForTest());
+ EXPECT_THAT(
+ runFuzzyFind(Idx, ""),
+ UnorderedElementsAre(AllOf(Named("header_sym"), NumReferences(1U)),
+ AllOf(Named("main_sym"), NumReferences(1U))));
}
TEST_F(BackgroundIndexTest, ShardStorageTest) {
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp
index a956b6b73b19..8d336b3f4e19 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -154,8 +154,7 @@ RefSlab TestTU::headerRefs() const {
std::unique_ptr<SymbolIndex> TestTU::index() const {
auto AST = build();
- auto Idx = std::make_unique<FileIndex>(/*UseDex=*/true,
- /*CollectMainFileRefs=*/true);
+ auto Idx = std::make_unique<FileIndex>();
Idx->updatePreamble(testPath(Filename), /*Version=*/"null",
AST.getASTContext(), AST.getPreprocessorPtr(),
AST.getCanonicalIncludes());
More information about the cfe-commits
mailing list