nridge updated this revision to Diff 284262.
nridge added a comment.
Add the requested flag
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83536/new/
https://reviews.llvm.org/D83536
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.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/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolCollector.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -613,6 +613,7 @@
)");
CollectorOpts.RefFilter = RefKind::All;
CollectorOpts.CollectMacro = true;
+ CollectorOpts.CollectMainFileRefs = true;
runSymbolCollector(Header.code(),
(Main.code() + SymbolsOnlyInMainCode.code()).str());
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
@@ -624,15 +625,13 @@
EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _))));
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID,
HaveRanges(Main.ranges("macro")))));
- // Symbols *only* in the main file:
- // - (a, b) externally visible and should have refs.
- // - (c, FUNC) externally invisible and had no refs collected.
- auto MainSymbols =
- TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
- EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "a").ID, _)));
- EXPECT_THAT(Refs, Contains(Pair(findSymbol(MainSymbols, "b").ID, _)));
- EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _))));
- EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "FUNC").ID, _))));
+ // Symbols *only* in the main file (a, b, c) should have refs collected
+ // as well.
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "a").ID, _)));
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "b").ID, _)));
+ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "c").ID, _)));
+ // However, references to main-file macros are not collected.
+ EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "FUNC").ID, _))));
}
TEST_F(SymbolCollectorTest, MacroRefInHeader) {
@@ -708,12 +707,12 @@
llvm::StringRef Main;
llvm::StringRef TargetSymbolName;
} TestCases[] = {
- {
- R"cpp(
+ {
+ R"cpp(
struct Foo;
#define MACRO Foo
)cpp",
- R"cpp(
+ R"cpp(
struct $spelled[[Foo]] {
$spelled[[Foo]]();
~$spelled[[Foo]]();
@@ -721,24 +720,24 @@
$spelled[[Foo]] Variable1;
$implicit[[MACRO]] Variable2;
)cpp",
- "Foo",
- },
- {
- R"cpp(
+ "Foo",
+ },
+ {
+ R"cpp(
class Foo {
public:
Foo() = default;
};
)cpp",
- R"cpp(
+ R"cpp(
void f() { Foo $implicit[[f]]; f = $spelled[[Foo]]();}
)cpp",
- "Foo::Foo" /// constructor.
- },
+ "Foo::Foo" /// constructor.
+ },
};
CollectorOpts.RefFilter = RefKind::All;
CollectorOpts.RefsInHeaders = false;
- for (const auto& T : TestCases) {
+ for (const auto &T : TestCases) {
Annotations Header(T.Header);
Annotations Main(T.Main);
// Reset the file system.
@@ -818,8 +817,9 @@
$Foo[[Foo]] fo;
}
)");
- // The main file is normal .cpp file, we should collect the refs
- // for externally visible symbols.
+ // We should collect refs to main-file symbols in all cases:
+
+ // 1. The main file is normal .cpp file.
TestFileName = testPath("foo.cpp");
runSymbolCollector("", Header.code());
EXPECT_THAT(Refs,
@@ -828,7 +828,7 @@
Pair(findSymbol(Symbols, "Func").ID,
HaveRanges(Header.ranges("Func")))));
- // Run the .h file as main file, we should collect the refs.
+ // 2. Run the .h file as main file.
TestFileName = testPath("foo.h");
runSymbolCollector("", Header.code(),
/*ExtraArgs=*/{"-xobjective-c++-header"});
@@ -839,8 +839,7 @@
Pair(findSymbol(Symbols, "Func").ID,
HaveRanges(Header.ranges("Func")))));
- // Run the .hh file as main file (without "-x c++-header"), we should collect
- // the refs as well.
+ // 3. Run the .hh file as main file (without "-x c++-header").
TestFileName = testPath("foo.hh");
runSymbolCollector("", Header.code());
EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Func")));
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -199,7 +199,7 @@
EXPECT_THAT(runFuzzyFind(Idx, ""),
UnorderedElementsAre(AllOf(Named("common"), NumReferences(1U)),
AllOf(Named("A_CC"), NumReferences(0U)),
- AllOf(Named("g"), NumReferences(0U)),
+ AllOf(Named("g"), NumReferences(1U)),
AllOf(Named("f_b"), Declared(),
Not(Defined()), NumReferences(0U))));
@@ -212,7 +212,7 @@
EXPECT_THAT(runFuzzyFind(Idx, ""),
UnorderedElementsAre(AllOf(Named("common"), NumReferences(5U)),
AllOf(Named("A_CC"), NumReferences(0U)),
- AllOf(Named("g"), NumReferences(0U)),
+ AllOf(Named("g"), NumReferences(1U)),
AllOf(Named("f_b"), Declared(), Defined(),
NumReferences(1U))));
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -450,6 +450,13 @@
init(true),
};
+opt<bool> CollectMainFileRefs{
+ "collect-main-file-refs",
+ cat(Misc),
+ desc("Store references to main-file-only symbols in the index"),
+ init(false),
+};
+
#if CLANGD_ENABLE_REMOTE
opt<std::string> RemoteIndexAddress{
"remote-index-address",
@@ -683,6 +690,7 @@
Opts.ResourceDir = ResourceDir;
Opts.BuildDynamicSymbolIndex = EnableIndex;
Opts.BackgroundIndex = EnableBackgroundIndex;
+ Opts.CollectMainFileRefs = CollectMainFileRefs;
std::unique_ptr<SymbolIndex> StaticIdx;
std::future<void> AsyncIndexLoad; // Block exit while loading the index.
if (EnableIndex && !IndexFile.empty()) {
Index: clang-tools-extra/clangd/index/SymbolCollector.h
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -78,6 +78,8 @@
/// Collect symbols local to main-files, such as static functions
/// and symbols inside an anonymous namespace.
bool CollectMainFileSymbols = true;
+ /// Collect references to main-file symbols.
+ bool CollectMainFileRefs = false;
/// If set to true, SymbolCollector will collect doc for all symbols.
/// Note that documents of symbols being indexed for completion will always
/// be collected regardless of this option.
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -309,12 +309,13 @@
if (IsOnlyRef && !CollectRef)
return true;
- // Do not store references to main-file symbols.
// Unlike other fields, e.g. Symbols (which use spelling locations), we use
// file locations for references (as it aligns the behavior of clangd's
// AST-based xref).
// FIXME: we should try to use the file locations for other fields.
- if (CollectRef && (!IsMainFileOnly || ND->isExternallyVisible()) &&
+ if (CollectRef &&
+ (!IsMainFileOnly || Opts.CollectMainFileRefs ||
+ ND->isExternallyVisible()) &&
!isa<NamespaceDecl>(ND) &&
(Opts.RefsInHeaders ||
SM.getFileID(SM.getFileLoc(Loc)) == SM.getMainFileID()))
Index: clang-tools-extra/clangd/index/FileIndex.h
===================================================================
--- clang-tools-extra/clangd/index/FileIndex.h
+++ clang-tools-extra/clangd/index/FileIndex.h
@@ -110,11 +110,13 @@
/// and macros in \p PP.
void updatePreamble(PathRef Path, llvm::StringRef Version, ASTContext &AST,
std::shared_ptr<Preprocessor> PP,
- const CanonicalIncludes &Includes);
+ const CanonicalIncludes &Includes,
+ bool CollectMainFileRefs = true);
/// Update symbols and references from main file \p Path with
/// `indexMainDecls`.
- void updateMain(PathRef Path, ParsedAST &AST);
+ void updateMain(PathRef Path, ParsedAST &AST,
+ bool CollectMainFileRefs = true);
private:
bool UseDex; // FIXME: this should be always on.
@@ -152,13 +154,14 @@
/// 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);
+SlabTuple indexMainDecls(ParsedAST &AST, bool CollectMainFileRefs = true);
/// Index declarations from \p AST and macros from \p PP that are declared in
/// included headers.
SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST,
std::shared_ptr<Preprocessor> PP,
- const CanonicalIncludes &Includes);
+ const CanonicalIncludes &Includes,
+ bool CollectMainFileRefs = true);
/// Takes slabs coming from a TU (multiple files) and shards them per
/// declaration location.
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -47,12 +47,13 @@
llvm::ArrayRef<Decl *> DeclsToIndex,
const MainFileMacros *MacroRefsToIndex,
const CanonicalIncludes &Includes, bool IsIndexMainAST,
- llvm::StringRef Version) {
+ llvm::StringRef Version, bool CollectMainFileRefs) {
SymbolCollector::Options CollectorOpts;
CollectorOpts.CollectIncludePath = true;
CollectorOpts.Includes = &Includes;
CollectorOpts.CountReferences = false;
CollectorOpts.Origin = SymbolOrigin::Dynamic;
+ CollectorOpts.CollectMainFileRefs = CollectMainFileRefs;
index::IndexingOptions IndexOpts;
// We only need declarations, because we don't count references.
@@ -205,22 +206,23 @@
return std::move(IF);
}
-SlabTuple indexMainDecls(ParsedAST &AST) {
- return indexSymbols(AST.getASTContext(), AST.getPreprocessorPtr(),
- AST.getLocalTopLevelDecls(), &AST.getMacros(),
- AST.getCanonicalIncludes(),
- /*IsIndexMainAST=*/true, AST.version());
+SlabTuple indexMainDecls(ParsedAST &AST, bool CollectMainFileRefs) {
+ return indexSymbols(
+ AST.getASTContext(), AST.getPreprocessorPtr(),
+ AST.getLocalTopLevelDecls(), &AST.getMacros(), AST.getCanonicalIncludes(),
+ /*IsIndexMainAST=*/true, AST.version(), CollectMainFileRefs);
}
SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST,
std::shared_ptr<Preprocessor> PP,
- const CanonicalIncludes &Includes) {
+ const CanonicalIncludes &Includes,
+ bool CollectMainFileRefs) {
std::vector<Decl *> DeclsToIndex(
AST.getTranslationUnitDecl()->decls().begin(),
AST.getTranslationUnitDecl()->decls().end());
return indexSymbols(AST, std::move(PP), DeclsToIndex,
/*MainFileMacros=*/nullptr, Includes,
- /*IsIndexMainAST=*/false, Version);
+ /*IsIndexMainAST=*/false, Version, CollectMainFileRefs);
}
void FileSymbols::update(llvm::StringRef Key,
@@ -379,10 +381,11 @@
void FileIndex::updatePreamble(PathRef Path, llvm::StringRef Version,
ASTContext &AST,
std::shared_ptr<Preprocessor> PP,
- const CanonicalIncludes &Includes) {
+ const CanonicalIncludes &Includes,
+ bool CollectMainFileRefs) {
IndexFileIn IF;
- std::tie(IF.Symbols, std::ignore, IF.Relations) =
- indexHeaderSymbols(Version, AST, std::move(PP), Includes);
+ std::tie(IF.Symbols, std::ignore, IF.Relations) = indexHeaderSymbols(
+ Version, AST, std::move(PP), Includes, CollectMainFileRefs);
FileShardedIndex ShardedIndex(std::move(IF));
for (auto Uri : ShardedIndex.getAllSources()) {
auto IF = ShardedIndex.getShard(Uri);
@@ -414,8 +417,9 @@
}
}
-void FileIndex::updateMain(PathRef Path, ParsedAST &AST) {
- auto Contents = indexMainDecls(AST);
+void FileIndex::updateMain(PathRef Path, ParsedAST &AST,
+ bool CollectMainFileRefs) {
+ auto Contents = indexMainDecls(AST, CollectMainFileRefs);
MainFileSymbols.update(
Path, std::make_unique<SymbolSlab>(std::move(std::get<0>(Contents))),
std::make_unique<RefSlab>(std::move(std::get<1>(Contents))),
Index: clang-tools-extra/clangd/index/Background.h
===================================================================
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -139,7 +139,8 @@
// In production an explicit value is passed.
size_t ThreadPoolSize = 4,
std::function<void(BackgroundQueue::Stats)> OnProgress = nullptr,
- std::function<Context(PathRef)> ContextProvider = nullptr);
+ std::function<Context(PathRef)> ContextProvider = nullptr,
+ bool CollectMainFileRefs = true);
~BackgroundIndex(); // Blocks while the current task finishes.
// Enqueue translation units for indexing.
@@ -185,6 +186,7 @@
const GlobalCompilationDatabase &CDB;
Context BackgroundContext;
std::function<Context(PathRef)> ContextProvider;
+ bool CollectMainFileRefs;
llvm::Error index(tooling::CompileCommand);
Index: clang-tools-extra/clangd/index/Background.cpp
===================================================================
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -95,10 +95,11 @@
const GlobalCompilationDatabase &CDB,
BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize,
std::function<void(BackgroundQueue::Stats)> OnProgress,
- std::function<Context(PathRef)> ContextProvider)
+ std::function<Context(PathRef)> ContextProvider, bool CollectMainFileRefs)
: SwapIndex(std::make_unique<MemIndex>()), TFS(TFS), CDB(CDB),
BackgroundContext(std::move(BackgroundContext)),
ContextProvider(std::move(ContextProvider)),
+ CollectMainFileRefs(CollectMainFileRefs),
Rebuilder(this, &IndexedSymbols, ThreadPoolSize),
IndexStorageFactory(std::move(IndexStorageFactory)),
Queue(std::move(OnProgress)),
@@ -304,6 +305,7 @@
return false; // Skip files that haven't changed, without errors.
return true;
};
+ IndexOpts.CollectMainFileRefs = CollectMainFileRefs;
IndexFileIn Index;
auto Action = createStaticIndexingAction(
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -111,6 +111,9 @@
/// 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 = false;
+
/// If set, use this index to augment code completion results.
SymbolIndex *StaticIndex = nullptr;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -62,20 +62,22 @@
struct UpdateIndexCallbacks : public ParsingCallbacks {
UpdateIndexCallbacks(FileIndex *FIndex,
ClangdServer::Callbacks *ServerCallbacks,
- bool TheiaSemanticHighlighting)
+ bool TheiaSemanticHighlighting, bool CollectMainFileRefs)
: FIndex(FIndex), ServerCallbacks(ServerCallbacks),
- TheiaSemanticHighlighting(TheiaSemanticHighlighting) {}
+ TheiaSemanticHighlighting(TheiaSemanticHighlighting),
+ CollectMainFileRefs(CollectMainFileRefs) {}
void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext &Ctx,
std::shared_ptr<clang::Preprocessor> PP,
const CanonicalIncludes &CanonIncludes) override {
if (FIndex)
- FIndex->updatePreamble(Path, Version, Ctx, std::move(PP), CanonIncludes);
+ FIndex->updatePreamble(Path, Version, Ctx, std::move(PP), CanonIncludes,
+ CollectMainFileRefs);
}
void onMainAST(PathRef Path, ParsedAST &AST, PublishFn Publish) override {
if (FIndex)
- FIndex->updateMain(Path, AST);
+ FIndex->updateMain(Path, AST, CollectMainFileRefs);
std::vector<Diag> Diagnostics = AST.getDiagnostics();
std::vector<HighlightingToken> Highlightings;
@@ -108,6 +110,7 @@
FileIndex *FIndex;
ClangdServer::Callbacks *ServerCallbacks;
bool TheiaSemanticHighlighting;
+ bool CollectMainFileRefs;
};
} // namespace
@@ -157,8 +160,9 @@
};
return O;
}(),
- std::make_unique<UpdateIndexCallbacks>(
- DynamicIdx.get(), Callbacks, Opts.TheiaSemanticHighlighting)) {
+ std::make_unique<UpdateIndexCallbacks>(DynamicIdx.get(), Callbacks,
+ Opts.TheiaSemanticHighlighting,
+ Opts.CollectMainFileRefs)) {
// Adds an index to the stack, at higher priority than existing indexes.
auto AddIndex = [&](SymbolIndex *Idx) {
if (this->Index != nullptr) {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
