diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp index bf441e8bc1bfe4..cf517717a27adf 100644 --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -289,21 +289,103 @@ llvm::Optional declToSym(ASTContext &Ctx, const NamedDecl &ND) { /// - visiting decls is actually simple, so we don't hit the complicated /// cases that RAV mostly helps with (types, expressions, etc.) class DocumentOutline { + // A DocumentSymbol we're constructing. + // We use this instead of DocumentSymbol directly so that we can keep track + // of the nodes we insert for macros. + class SymBuilder { + std::vector Children; + DocumentSymbol Symbol; // Symbol.children is empty, use Children instead. + // Macro expansions that this node or its parents are associated with. + // (Thus we will never create further children for these expansions). + llvm::SmallVector EnclosingMacroLoc; + + public: + DocumentSymbol build() && { + for (SymBuilder &C : Children) { + Symbol.children.push_back(std::move(C).build()); + // Expand range to ensure children nest properly, which editors expect. + // This can fix some edge-cases in the AST, but is vital for macros. + // A macro expansion "contains" AST node if it covers the node's primary + // location, but it may not span the node's whole range. + Symbol.range.start = + std::min(Symbol.range.start, Symbol.children.back().range.start); + Symbol.range.end = + std::max(Symbol.range.end, Symbol.children.back().range.end); + } + return std::move(Symbol); + } + + // Add a symbol as a child of the current one. + SymBuilder &addChild(DocumentSymbol S) { + Children.emplace_back(); + Children.back().EnclosingMacroLoc = EnclosingMacroLoc; + Children.back().Symbol = std::move(S); + return Children.back(); + } + + // Get an appropriate container for children of this symbol that were + // expanded from a macro (whose spelled name is Tok). + // + // This may return: + // - a macro symbol child of this (either new or previously created) + // - this scope itself, if it *is* the macro symbol or is nested within it + SymBuilder &inMacro(const syntax::Token &Tok, const SourceManager &SM, + llvm::Optional Exp) { + if (llvm::is_contained(EnclosingMacroLoc, Tok.location())) + return *this; + // If there's an existing child for this macro, we expect it to be last. + if (!Children.empty() && !Children.back().EnclosingMacroLoc.empty() && + Children.back().EnclosingMacroLoc.back() == Tok.location()) + return Children.back(); + + DocumentSymbol Sym; + Sym.name = Tok.text(SM).str(); + Sym.kind = SymbolKind::Null; // There's no suitable kind! + Sym.range = Sym.selectionRange = + halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)); + + // FIXME: Exp is currently unavailable for nested expansions. + if (Exp) { + // Full range covers the macro args. + Sym.range = halfOpenToRange(SM, CharSourceRange::getCharRange( + Exp->Spelled.front().location(), + Exp->Spelled.back().endLocation())); + // Show macro args as detail. + llvm::raw_string_ostream OS(Sym.detail); + const syntax::Token *Prev = nullptr; + for (const auto &Tok : Exp->Spelled.drop_front()) { + // Don't dump arbitrarily long macro args. + if (OS.tell() > 80) { + OS << " ...)"; + break; + } + if (Prev && Prev->endLocation() != Tok.location()) + OS << ' '; + OS << Tok.text(SM); + Prev = &Tok; + } + } + SymBuilder &Child = addChild(std::move(Sym)); + Child.EnclosingMacroLoc.push_back(Tok.location()); + return Child; + } + }; + public: DocumentOutline(ParsedAST &AST) : AST(AST) {} /// Builds the document outline for the generated AST. std::vector build() { - std::vector Results; + SymBuilder DummyRoot; for (auto &TopLevel : AST.getLocalTopLevelDecls()) - traverseDecl(TopLevel, Results); - return Results; + traverseDecl(TopLevel, DummyRoot); + return std::move(std::move(DummyRoot).build().children); } private: enum class VisitKind { No, OnlyDecl, OnlyChildren, DeclAndChildren }; - void traverseDecl(Decl *D, std::vector &Results) { + void traverseDecl(Decl *D, SymBuilder &Parent) { // Skip symbols which do not originate from the main file. if (!isInsideMainFile(D->getLocation(), AST.getSourceManager())) return; @@ -319,27 +401,76 @@ class DocumentOutline { return; if (Visit == VisitKind::OnlyChildren) - return traverseChildren(D, Results); + return traverseChildren(D, Parent); auto *ND = llvm::cast(D); auto Sym = declToSym(AST.getASTContext(), *ND); if (!Sym) return; - Results.push_back(std::move(*Sym)); + SymBuilder &MacroParent = possibleMacroContainer(D->getLocation(), Parent); + SymBuilder &Child = MacroParent.addChild(std::move(*Sym)); if (Visit == VisitKind::OnlyDecl) return; assert(Visit == VisitKind::DeclAndChildren && "Unexpected VisitKind"); - traverseChildren(ND, Results.back().children); + traverseChildren(ND, Child); + } + + // Determines where a decl should appear in the DocumentSymbol hierarchy. + // + // This is usually a direct child of the relevant AST parent. + // But we may also insert nodes for macros. Given: + // #define DECLARE_INT(V) int v; + // namespace a { DECLARE_INT(x) } + // We produce: + // Namespace a + // Macro DECLARE_INT(x) + // Variable x + // + // In the absence of macros, this method simply returns Parent. + // Otherwise it may return a macro expansion node instead. + // Each macro only has at most one node in the hierarchy, even if it expands + // to multiple decls. + SymBuilder &possibleMacroContainer(SourceLocation TargetLoc, + SymBuilder &Parent) { + const auto &SM = AST.getSourceManager(); + // Look at the path of macro-callers from the token to the main file. + // Note that along these paths we see the "outer" macro calls first. + SymBuilder *CurParent = &Parent; + for (SourceLocation Loc = TargetLoc; Loc.isMacroID(); + Loc = SM.getImmediateMacroCallerLoc(Loc)) { + // Find the virtual macro body that our token is being substituted into. + FileID MacroBody; + if (SM.isMacroArgExpansion(Loc)) { + // Loc is part of a macro arg being substituted into a macro body. + MacroBody = SM.getFileID(SM.getImmediateExpansionRange(Loc).getBegin()); + } else { + // Loc is already in the macro body. + MacroBody = SM.getFileID(Loc); + } + // The macro body is being substituted for a macro expansion, whose + // first token is the name of the macro. + SourceLocation MacroName = + SM.getSLocEntry(MacroBody).getExpansion().getExpansionLocStart(); + // Only include the macro expansion in the outline if it was written + // directly in the main file, rather than expanded from another macro. + if (!MacroName.isValid() || !MacroName.isFileID()) + continue; + // All conditions satisfied, add the macro. + if (auto *Tok = AST.getTokens().spelledTokenAt(MacroName)) + CurParent = &CurParent->inMacro( + *Tok, SM, AST.getTokens().expansionStartingAt(Tok)); + } + return *CurParent; } - void traverseChildren(Decl *D, std::vector &Results) { + void traverseChildren(Decl *D, SymBuilder &Builder) { auto *Scope = llvm::dyn_cast(D); if (!Scope) return; for (auto *C : Scope->decls()) - traverseDecl(C, Results); + traverseDecl(C, Builder); } VisitKind shouldVisit(Decl *D) { diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp index 8cae9d60615974..fa9af8637304cc 100644 --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -662,7 +662,76 @@ TEST(DocumentSymbols, Enums) { WithDetail("(unnamed)")))))))); } -TEST(DocumentSymbols, FromMacro) { +TEST(DocumentSymbols, Macro) { + struct Test { + const char *Code; + testing::Matcher Matcher; + } Tests[] = { + { + R"cpp( + // Basic macro that generates symbols. + #define DEFINE_FLAG(X) bool FLAGS_##X; bool FLAGS_no##X + DEFINE_FLAG(pretty); + )cpp", + AllOf(WithName("DEFINE_FLAG"), WithDetail("(pretty)"), + Children(WithName("FLAGS_pretty"), WithName("FLAGS_nopretty"))), + }, + { + R"cpp( + // Hierarchy is determined by primary (name) location. + #define ID(X) X + namespace ID(ns) { int ID(y); } + )cpp", + AllOf(WithName("ID"), WithDetail("(ns)"), + Children(AllOf(WithName("ns"), + Children(AllOf(WithName("ID"), WithDetail("(y)"), + Children(WithName("y"))))))), + }, + { + R"cpp( + // More typical example where macro only generates part of a decl. + #define TEST(A, B) class A##_##B { void go(); }; void A##_##B::go() + TEST(DocumentSymbols, Macro) { } + )cpp", + AllOf(WithName("TEST"), WithDetail("(DocumentSymbols, Macro)"), + Children(AllOf(WithName("DocumentSymbols_Macro"), + Children(WithName("go"))), + WithName("DocumentSymbols_Macro::go"))), + }, + { + R"cpp( + // Nested macros. + #define NAMESPACE(NS, BODY) namespace NS { BODY } + NAMESPACE(a, NAMESPACE(b, int x;)) + )cpp", + AllOf( + WithName("NAMESPACE"), WithDetail("(a, NAMESPACE(b, int x;))"), + Children(AllOf( + WithName("a"), + Children(AllOf(WithName("NAMESPACE"), + // FIXME: nested expansions not in TokenBuffer + WithDetail(""), + Children(AllOf(WithName("b"), + Children(WithName("x"))))))))), + }, + { + R"cpp( + // Macro invoked from body is not exposed. + #define INNER(X) int X + #define OUTER(X) INNER(X) + OUTER(foo); + )cpp", + AllOf(WithName("OUTER"), WithDetail("(foo)"), + Children(WithName("foo"))), + }, + }; + for (const Test &T : Tests) { + auto TU = TestTU::withCode(T.Code); + EXPECT_THAT(getSymbols(TU.build()), ElementsAre(T.Matcher)) << T.Code; + } +} + +TEST(DocumentSymbols, RangeFromMacro) { TestTU TU; Annotations Main(R"( #define FF(name) \ @@ -671,9 +740,9 @@ TEST(DocumentSymbols, FromMacro) { $expansion1[[FF]](abc); #define FF2() \ - class Test {}; + class Test {} - $expansion2[[FF2]](); + $expansion2parens[[$expansion2[[FF2]]()]]; #define FF3() \ void waldo() @@ -683,13 +752,21 @@ TEST(DocumentSymbols, FromMacro) { }]] )"); TU.Code = Main.code().str(); - EXPECT_THAT(getSymbols(TU.build()), - ElementsAre(AllOf(WithName("abc_Test"), WithDetail("class"), - SymNameRange(Main.range("expansion1"))), - AllOf(WithName("Test"), WithDetail("class"), - SymNameRange(Main.range("expansion2"))), - AllOf(WithName("waldo"), WithDetail("void ()"), - SymRange(Main.range("fullDef"))))); + EXPECT_THAT( + getSymbols(TU.build()), + ElementsAre( + AllOf(WithName("FF"), WithDetail("(abc)"), + Children(AllOf(WithName("abc_Test"), WithDetail("class"), + SymNameRange(Main.range("expansion1"))))), + AllOf(WithName("FF2"), WithDetail("()"), + SymNameRange(Main.range("expansion2")), + SymRange(Main.range("expansion2parens")), + Children(AllOf(WithName("Test"), WithDetail("class"), + SymNameRange(Main.range("expansion2"))))), + AllOf(WithName("FF3"), WithDetail("()"), + SymRange(Main.range("fullDef")), + Children(AllOf(WithName("waldo"), WithDetail("void ()"), + SymRange(Main.range("fullDef"))))))); } TEST(DocumentSymbols, FuncTemplates) {