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 <zeratul...@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 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
