[clang-tools-extra] [clangd] Store documentation when indexing standard library (PR #133681)
Nathan Ridge via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 27 17:28:17 PDT 2025
https://github.com/HighCommander4 updated https://github.com/llvm/llvm-project/pull/133681
>From aab6aef62b7691290f02b0dd29b0071f428502c9 Mon Sep 17 00:00:00 2001
From: Nathan Ridge <zeratul976 at hotmail.com>
Date: Mon, 31 Mar 2025 02:25:45 -0400
Subject: [PATCH] [clangd] Store documentation when indexing standard library
Fixes https://github.com/clangd/clangd/issues/2344
---
clang-tools-extra/clangd/index/FileIndex.cpp | 21 ++++++-----
clang-tools-extra/clangd/index/FileIndex.h | 3 +-
clang-tools-extra/clangd/index/StdLib.cpp | 34 ++++++++---------
.../clangd/unittests/StdLibTests.cpp | 37 +++++++++++++++++++
.../clangd/unittests/SyncAPI.cpp | 7 ++++
clang-tools-extra/clangd/unittests/SyncAPI.h | 3 ++
clang-tools-extra/clangd/unittests/TestTU.cpp | 3 +-
7 files changed, 77 insertions(+), 31 deletions(-)
diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp b/clang-tools-extra/clangd/index/FileIndex.cpp
index aa573e312a756..de88edd0c7a0b 100644
--- a/clang-tools-extra/clangd/index/FileIndex.cpp
+++ b/clang-tools-extra/clangd/index/FileIndex.cpp
@@ -48,13 +48,12 @@ SlabTuple indexSymbols(ASTContext &AST, Preprocessor &PP,
const MainFileMacros *MacroRefsToIndex,
const include_cleaner::PragmaIncludes &PI,
bool IsIndexMainAST, llvm::StringRef Version,
- bool CollectMainFileRefs) {
+ bool CollectMainFileRefs, SymbolOrigin Origin) {
SymbolCollector::Options CollectorOpts;
CollectorOpts.CollectIncludePath = true;
CollectorOpts.PragmaIncludes = &PI;
CollectorOpts.CountReferences = false;
- CollectorOpts.Origin =
- IsIndexMainAST ? SymbolOrigin::Open : SymbolOrigin::Preamble;
+ CollectorOpts.Origin = Origin;
CollectorOpts.CollectMainFileRefs = CollectMainFileRefs;
// We want stdlib implementation details in the index only if we've opened the
// file in question. This does means xrefs won't work, though.
@@ -221,22 +220,24 @@ FileShardedIndex::getShard(llvm::StringRef Uri) const {
}
SlabTuple indexMainDecls(ParsedAST &AST) {
- return indexSymbols(
- AST.getASTContext(), AST.getPreprocessor(), AST.getLocalTopLevelDecls(),
- &AST.getMacros(), AST.getPragmaIncludes(),
- /*IsIndexMainAST=*/true, AST.version(), /*CollectMainFileRefs=*/true);
+ return indexSymbols(AST.getASTContext(), AST.getPreprocessor(),
+ AST.getLocalTopLevelDecls(), &AST.getMacros(),
+ AST.getPragmaIncludes(),
+ /*IsIndexMainAST=*/true, AST.version(),
+ /*CollectMainFileRefs=*/true, SymbolOrigin::Open);
}
SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST,
Preprocessor &PP,
- const include_cleaner::PragmaIncludes &PI) {
+ const include_cleaner::PragmaIncludes &PI,
+ SymbolOrigin Origin) {
std::vector<Decl *> DeclsToIndex(
AST.getTranslationUnitDecl()->decls().begin(),
AST.getTranslationUnitDecl()->decls().end());
return indexSymbols(AST, PP, DeclsToIndex,
/*MainFileMacros=*/nullptr, PI,
/*IsIndexMainAST=*/false, Version,
- /*CollectMainFileRefs=*/false);
+ /*CollectMainFileRefs=*/false, Origin);
}
FileSymbols::FileSymbols(IndexContents IdxContents, bool SupportContainedRefs)
@@ -463,7 +464,7 @@ void FileIndex::updatePreamble(PathRef Path, llvm::StringRef Version,
const include_cleaner::PragmaIncludes &PI) {
IndexFileIn IF;
std::tie(IF.Symbols, std::ignore, IF.Relations) =
- indexHeaderSymbols(Version, AST, PP, PI);
+ indexHeaderSymbols(Version, AST, PP, PI, SymbolOrigin::Preamble);
updatePreamble(std::move(IF));
}
diff --git a/clang-tools-extra/clangd/index/FileIndex.h b/clang-tools-extra/clangd/index/FileIndex.h
index 8e88dc9712996..86af5ee3723f6 100644
--- a/clang-tools-extra/clangd/index/FileIndex.h
+++ b/clang-tools-extra/clangd/index/FileIndex.h
@@ -164,7 +164,8 @@ SlabTuple indexMainDecls(ParsedAST &AST);
/// included headers.
SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST,
Preprocessor &PP,
- const include_cleaner::PragmaIncludes &PI);
+ const include_cleaner::PragmaIncludes &PI,
+ SymbolOrigin Origin);
/// Takes slabs coming from a TU (multiple files) and shards them per
/// declaration location.
diff --git a/clang-tools-extra/clangd/index/StdLib.cpp b/clang-tools-extra/clangd/index/StdLib.cpp
index d34838a45048d..7087b661a2c6b 100644
--- a/clang-tools-extra/clangd/index/StdLib.cpp
+++ b/clang-tools-extra/clangd/index/StdLib.cpp
@@ -15,12 +15,15 @@
#include "Compiler.h"
#include "Config.h"
#include "SymbolCollector.h"
+#include "clang-include-cleaner/Record.h"
+#include "index/FileIndex.h"
#include "index/IndexAction.h"
#include "support/Logger.h"
#include "support/ThreadsafeFS.h"
#include "support/Trace.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
#include "clang/Lex/PreprocessorOptions.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
@@ -223,33 +226,26 @@ SymbolSlab indexStandardLibrary(llvm::StringRef HeaderSources,
return Symbols;
}
- SymbolCollector::Options IndexOpts;
- IndexOpts.Origin = SymbolOrigin::StdLib;
- IndexOpts.CollectMainFileSymbols = false;
- IndexOpts.CollectMainFileRefs = false;
- IndexOpts.CollectMacro = true;
- IndexOpts.StoreAllDocumentation = true;
- // Sadly we can't use IndexOpts.FileFilter to restrict indexing scope.
- // Files from outside the StdLibLocation may define true std symbols anyway.
- // We end up "blessing" such headers, and can only do that by indexing
- // everything first.
-
- // Refs, relations, include graph in the stdlib mostly aren't useful.
- auto Action = createStaticIndexingAction(
- IndexOpts, [&](SymbolSlab S) { Symbols = std::move(S); }, nullptr,
- nullptr, nullptr);
-
- if (!Action->BeginSourceFile(*Clang, Input)) {
+ SyntaxOnlyAction Action;
+
+ if (!Action.BeginSourceFile(*Clang, Input)) {
elog("Standard Library Index: BeginSourceFile() failed");
return Symbols;
}
- if (llvm::Error Err = Action->Execute()) {
+ if (llvm::Error Err = Action.Execute()) {
elog("Standard Library Index: Execute failed: {0}", std::move(Err));
return Symbols;
}
- Action->EndSourceFile();
+ // We don't care about include graph for stdlib headers, so provide a no-op
+ // PI.
+ include_cleaner::PragmaIncludes PI;
+ auto Slabs =
+ indexHeaderSymbols("", Clang->getASTContext(), Clang->getPreprocessor(),
+ PI, SymbolOrigin::StdLib);
+ Symbols = std::move(std::get<0>(Slabs));
+ Action.EndSourceFile();
unsigned SymbolsBeforeFilter = Symbols.size();
Symbols = filter(std::move(Symbols), Loc);
diff --git a/clang-tools-extra/clangd/unittests/StdLibTests.cpp b/clang-tools-extra/clangd/unittests/StdLibTests.cpp
index a7a33f78303d3..00c6d629e1c25 100644
--- a/clang-tools-extra/clangd/unittests/StdLibTests.cpp
+++ b/clang-tools-extra/clangd/unittests/StdLibTests.cpp
@@ -158,6 +158,43 @@ TEST(StdLibTests, EndToEnd) {
UnorderedElementsAre(StdlibSymbol("list"), StdlibSymbol("vector")));
}
+TEST(StdLibTests, StdLibDocComments) {
+ Config Cfg;
+ Cfg.Index.StandardLibrary = true;
+ WithContextValue Enabled(Config::Key, std::move(Cfg));
+
+ MockFS FS;
+ FS.Files["stdlib/vector"] = R"cpp(
+ namespace std {
+ template <typename T>
+ class vector {
+ public:
+ /**doc comment*/
+ unsigned int size() const;
+ };
+ }
+ )cpp";
+ MockCompilationDatabase CDB;
+ CDB.ExtraClangFlags.push_back("-isystem" + testPath("stdlib"));
+ ClangdServer::Options Opts = ClangdServer::optsForTest();
+ Opts.BuildDynamicSymbolIndex = true; // also used for stdlib index
+ ClangdServer Server(CDB, FS, Opts);
+
+ Annotations A(R"cpp(
+ #include <vector>
+ void foo() {
+ std::vector<int> v;
+ v.si^ze();
+ }
+ )cpp");
+
+ Server.addDocument(testPath("foo.cc"), A.code());
+ ASSERT_TRUE(Server.blockUntilIdleForTest());
+ auto HI = cantFail(runHover(Server, testPath("foo.cc"), A.point()));
+ EXPECT_TRUE(HI.has_value());
+ EXPECT_EQ(HI->Documentation, "doc comment");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.cpp b/clang-tools-extra/clangd/unittests/SyncAPI.cpp
index d48622eba5378..00bec7afd1a98 100644
--- a/clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ b/clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -68,6 +68,13 @@ template <typename T> CaptureProxy<T> capture(std::optional<T> &Target) {
}
} // namespace
+llvm::Expected<std::optional<HoverInfo>> runHover(ClangdServer &Server,
+ PathRef File, Position Pos) {
+ std::optional<llvm::Expected<std::optional<HoverInfo>>> HI;
+ Server.findHover(File, Pos, capture(HI));
+ return std::move(*HI);
+}
+
llvm::Expected<CodeCompleteResult>
runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
clangd::CodeCompleteOptions Opts) {
diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.h b/clang-tools-extra/clangd/unittests/SyncAPI.h
index cf3de4f742e84..e0c7c4d72e73e 100644
--- a/clang-tools-extra/clangd/unittests/SyncAPI.h
+++ b/clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -29,6 +29,9 @@ void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents,
WantDiagnostics WantDiags = WantDiagnostics::Auto,
bool ForceRebuild = false);
+llvm::Expected<std::optional<HoverInfo>> runHover(ClangdServer &Server,
+ PathRef File, Position Pos);
+
llvm::Expected<CodeCompleteResult>
runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
clangd::CodeCompleteOptions Opts);
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp
index 3f8990c86f714..a733fac932bf4 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -12,6 +12,7 @@
#include "Diagnostics.h"
#include "TestFS.h"
#include "index/FileIndex.h"
+#include "index/SymbolOrigin.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Frontend/CompilerInvocation.h"
@@ -164,7 +165,7 @@ SymbolSlab TestTU::headerSymbols() const {
auto AST = build();
return std::get<0>(indexHeaderSymbols(
/*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(),
- AST.getPragmaIncludes()));
+ AST.getPragmaIncludes(), SymbolOrigin::Preamble));
}
RefSlab TestTU::headerRefs() const {
More information about the cfe-commits
mailing list