Skip to content

Commit aab6aef

Browse files
[clangd] Store documentation when indexing standard library
Fixes clangd/clangd#2344
1 parent fff8f03 commit aab6aef

File tree

7 files changed

+77
-31
lines changed

7 files changed

+77
-31
lines changed

clang-tools-extra/clangd/index/FileIndex.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,12 @@ SlabTuple indexSymbols(ASTContext &AST, Preprocessor &PP,
4848
const MainFileMacros *MacroRefsToIndex,
4949
const include_cleaner::PragmaIncludes &PI,
5050
bool IsIndexMainAST, llvm::StringRef Version,
51-
bool CollectMainFileRefs) {
51+
bool CollectMainFileRefs, SymbolOrigin Origin) {
5252
SymbolCollector::Options CollectorOpts;
5353
CollectorOpts.CollectIncludePath = true;
5454
CollectorOpts.PragmaIncludes = Π
5555
CollectorOpts.CountReferences = false;
56-
CollectorOpts.Origin =
57-
IsIndexMainAST ? SymbolOrigin::Open : SymbolOrigin::Preamble;
56+
CollectorOpts.Origin = Origin;
5857
CollectorOpts.CollectMainFileRefs = CollectMainFileRefs;
5958
// We want stdlib implementation details in the index only if we've opened the
6059
// file in question. This does means xrefs won't work, though.
@@ -221,22 +220,24 @@ FileShardedIndex::getShard(llvm::StringRef Uri) const {
221220
}
222221

223222
SlabTuple indexMainDecls(ParsedAST &AST) {
224-
return indexSymbols(
225-
AST.getASTContext(), AST.getPreprocessor(), AST.getLocalTopLevelDecls(),
226-
&AST.getMacros(), AST.getPragmaIncludes(),
227-
/*IsIndexMainAST=*/true, AST.version(), /*CollectMainFileRefs=*/true);
223+
return indexSymbols(AST.getASTContext(), AST.getPreprocessor(),
224+
AST.getLocalTopLevelDecls(), &AST.getMacros(),
225+
AST.getPragmaIncludes(),
226+
/*IsIndexMainAST=*/true, AST.version(),
227+
/*CollectMainFileRefs=*/true, SymbolOrigin::Open);
228228
}
229229

230230
SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST,
231231
Preprocessor &PP,
232-
const include_cleaner::PragmaIncludes &PI) {
232+
const include_cleaner::PragmaIncludes &PI,
233+
SymbolOrigin Origin) {
233234
std::vector<Decl *> DeclsToIndex(
234235
AST.getTranslationUnitDecl()->decls().begin(),
235236
AST.getTranslationUnitDecl()->decls().end());
236237
return indexSymbols(AST, PP, DeclsToIndex,
237238
/*MainFileMacros=*/nullptr, PI,
238239
/*IsIndexMainAST=*/false, Version,
239-
/*CollectMainFileRefs=*/false);
240+
/*CollectMainFileRefs=*/false, Origin);
240241
}
241242

242243
FileSymbols::FileSymbols(IndexContents IdxContents, bool SupportContainedRefs)
@@ -463,7 +464,7 @@ void FileIndex::updatePreamble(PathRef Path, llvm::StringRef Version,
463464
const include_cleaner::PragmaIncludes &PI) {
464465
IndexFileIn IF;
465466
std::tie(IF.Symbols, std::ignore, IF.Relations) =
466-
indexHeaderSymbols(Version, AST, PP, PI);
467+
indexHeaderSymbols(Version, AST, PP, PI, SymbolOrigin::Preamble);
467468
updatePreamble(std::move(IF));
468469
}
469470

clang-tools-extra/clangd/index/FileIndex.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ SlabTuple indexMainDecls(ParsedAST &AST);
164164
/// included headers.
165165
SlabTuple indexHeaderSymbols(llvm::StringRef Version, ASTContext &AST,
166166
Preprocessor &PP,
167-
const include_cleaner::PragmaIncludes &PI);
167+
const include_cleaner::PragmaIncludes &PI,
168+
SymbolOrigin Origin);
168169

169170
/// Takes slabs coming from a TU (multiple files) and shards them per
170171
/// declaration location.

clang-tools-extra/clangd/index/StdLib.cpp

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@
1515
#include "Compiler.h"
1616
#include "Config.h"
1717
#include "SymbolCollector.h"
18+
#include "clang-include-cleaner/Record.h"
19+
#include "index/FileIndex.h"
1820
#include "index/IndexAction.h"
1921
#include "support/Logger.h"
2022
#include "support/ThreadsafeFS.h"
2123
#include "support/Trace.h"
2224
#include "clang/Basic/LangOptions.h"
2325
#include "clang/Frontend/CompilerInvocation.h"
26+
#include "clang/Frontend/FrontendActions.h"
2427
#include "clang/Lex/PreprocessorOptions.h"
2528
#include "clang/Tooling/Inclusions/StandardLibrary.h"
2629
#include "llvm/ADT/IntrusiveRefCntPtr.h"
@@ -223,33 +226,26 @@ SymbolSlab indexStandardLibrary(llvm::StringRef HeaderSources,
223226
return Symbols;
224227
}
225228

226-
SymbolCollector::Options IndexOpts;
227-
IndexOpts.Origin = SymbolOrigin::StdLib;
228-
IndexOpts.CollectMainFileSymbols = false;
229-
IndexOpts.CollectMainFileRefs = false;
230-
IndexOpts.CollectMacro = true;
231-
IndexOpts.StoreAllDocumentation = true;
232-
// Sadly we can't use IndexOpts.FileFilter to restrict indexing scope.
233-
// Files from outside the StdLibLocation may define true std symbols anyway.
234-
// We end up "blessing" such headers, and can only do that by indexing
235-
// everything first.
236-
237-
// Refs, relations, include graph in the stdlib mostly aren't useful.
238-
auto Action = createStaticIndexingAction(
239-
IndexOpts, [&](SymbolSlab S) { Symbols = std::move(S); }, nullptr,
240-
nullptr, nullptr);
241-
242-
if (!Action->BeginSourceFile(*Clang, Input)) {
229+
SyntaxOnlyAction Action;
230+
231+
if (!Action.BeginSourceFile(*Clang, Input)) {
243232
elog("Standard Library Index: BeginSourceFile() failed");
244233
return Symbols;
245234
}
246235

247-
if (llvm::Error Err = Action->Execute()) {
236+
if (llvm::Error Err = Action.Execute()) {
248237
elog("Standard Library Index: Execute failed: {0}", std::move(Err));
249238
return Symbols;
250239
}
251240

252-
Action->EndSourceFile();
241+
// We don't care about include graph for stdlib headers, so provide a no-op
242+
// PI.
243+
include_cleaner::PragmaIncludes PI;
244+
auto Slabs =
245+
indexHeaderSymbols("", Clang->getASTContext(), Clang->getPreprocessor(),
246+
PI, SymbolOrigin::StdLib);
247+
Symbols = std::move(std::get<0>(Slabs));
248+
Action.EndSourceFile();
253249

254250
unsigned SymbolsBeforeFilter = Symbols.size();
255251
Symbols = filter(std::move(Symbols), Loc);

clang-tools-extra/clangd/unittests/StdLibTests.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,43 @@ TEST(StdLibTests, EndToEnd) {
158158
UnorderedElementsAre(StdlibSymbol("list"), StdlibSymbol("vector")));
159159
}
160160

161+
TEST(StdLibTests, StdLibDocComments) {
162+
Config Cfg;
163+
Cfg.Index.StandardLibrary = true;
164+
WithContextValue Enabled(Config::Key, std::move(Cfg));
165+
166+
MockFS FS;
167+
FS.Files["stdlib/vector"] = R"cpp(
168+
namespace std {
169+
template <typename T>
170+
class vector {
171+
public:
172+
/**doc comment*/
173+
unsigned int size() const;
174+
};
175+
}
176+
)cpp";
177+
MockCompilationDatabase CDB;
178+
CDB.ExtraClangFlags.push_back("-isystem" + testPath("stdlib"));
179+
ClangdServer::Options Opts = ClangdServer::optsForTest();
180+
Opts.BuildDynamicSymbolIndex = true; // also used for stdlib index
181+
ClangdServer Server(CDB, FS, Opts);
182+
183+
Annotations A(R"cpp(
184+
#include <vector>
185+
void foo() {
186+
std::vector<int> v;
187+
v.si^ze();
188+
}
189+
)cpp");
190+
191+
Server.addDocument(testPath("foo.cc"), A.code());
192+
ASSERT_TRUE(Server.blockUntilIdleForTest());
193+
auto HI = cantFail(runHover(Server, testPath("foo.cc"), A.point()));
194+
EXPECT_TRUE(HI.has_value());
195+
EXPECT_EQ(HI->Documentation, "doc comment");
196+
}
197+
161198
} // namespace
162199
} // namespace clangd
163200
} // namespace clang

clang-tools-extra/clangd/unittests/SyncAPI.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,13 @@ template <typename T> CaptureProxy<T> capture(std::optional<T> &Target) {
6868
}
6969
} // namespace
7070

71+
llvm::Expected<std::optional<HoverInfo>> runHover(ClangdServer &Server,
72+
PathRef File, Position Pos) {
73+
std::optional<llvm::Expected<std::optional<HoverInfo>>> HI;
74+
Server.findHover(File, Pos, capture(HI));
75+
return std::move(*HI);
76+
}
77+
7178
llvm::Expected<CodeCompleteResult>
7279
runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
7380
clangd::CodeCompleteOptions Opts) {

clang-tools-extra/clangd/unittests/SyncAPI.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ void runAddDocument(ClangdServer &Server, PathRef File, StringRef Contents,
2929
WantDiagnostics WantDiags = WantDiagnostics::Auto,
3030
bool ForceRebuild = false);
3131

32+
llvm::Expected<std::optional<HoverInfo>> runHover(ClangdServer &Server,
33+
PathRef File, Position Pos);
34+
3235
llvm::Expected<CodeCompleteResult>
3336
runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
3437
clangd::CodeCompleteOptions Opts);

clang-tools-extra/clangd/unittests/TestTU.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "Diagnostics.h"
1313
#include "TestFS.h"
1414
#include "index/FileIndex.h"
15+
#include "index/SymbolOrigin.h"
1516
#include "clang/AST/RecursiveASTVisitor.h"
1617
#include "clang/Basic/Diagnostic.h"
1718
#include "clang/Frontend/CompilerInvocation.h"
@@ -164,7 +165,7 @@ SymbolSlab TestTU::headerSymbols() const {
164165
auto AST = build();
165166
return std::get<0>(indexHeaderSymbols(
166167
/*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(),
167-
AST.getPragmaIncludes()));
168+
AST.getPragmaIncludes(), SymbolOrigin::Preamble));
168169
}
169170

170171
RefSlab TestTU::headerRefs() const {

0 commit comments

Comments
 (0)