From c911803d5df0f8a781b56849180b4b93a61306a7 Mon Sep 17 00:00:00 2001 From: Aleksandr Platonov Date: Mon, 20 Jul 2020 13:46:09 +0200 Subject: [PATCH] [clangd] Remove TokenBuffer usage in TypeHierarchy Summary: This patch mostly reverts D74850. We could not use `AST.getTokens()` here, because it does not have tokens from the preamble. Reviewers: sammccall, kadircet Reviewed By: kadircet Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, kbobyrev, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D84144 --- clang-tools-extra/clangd/XRefs.cpp | 40 ++++++++++------------ .../clangd/unittests/TypeHierarchyTests.cpp | 27 +++++++++++++++ 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index c208e95..aeff7eb 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1183,23 +1183,24 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const LocatedSymbol &S) { // FIXME(nridge): Reduce duplication between this function and declToSym(). static llvm::Optional -declToTypeHierarchyItem(ASTContext &Ctx, const NamedDecl &ND, - const syntax::TokenBuffer &TB) { +declToTypeHierarchyItem(ASTContext &Ctx, const NamedDecl &ND) { auto &SM = Ctx.getSourceManager(); SourceLocation NameLoc = nameLocation(ND, Ctx.getSourceManager()); + SourceLocation BeginLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getBeginLoc())); + SourceLocation EndLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getEndLoc())); + const auto DeclRange = + toHalfOpenFileRange(SM, Ctx.getLangOpts(), {BeginLoc, EndLoc}); + if (!DeclRange) + return llvm::None; auto FilePath = getCanonicalPath(SM.getFileEntryForID(SM.getFileID(NameLoc)), SM); auto TUPath = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM); if (!FilePath || !TUPath) return llvm::None; // Not useful without a uri. - auto DeclToks = TB.spelledForExpanded(TB.expandedTokens(ND.getSourceRange())); - if (!DeclToks || DeclToks->empty()) - return llvm::None; - - auto NameToks = TB.spelledForExpanded(TB.expandedTokens(NameLoc)); - if (!NameToks || NameToks->empty()) - return llvm::None; + Position NameBegin = sourceLocToPosition(SM, NameLoc); + Position NameEnd = sourceLocToPosition( + SM, Lexer::getLocForEndOfToken(NameLoc, 0, SM, Ctx.getLangOpts())); index::SymbolInfo SymInfo = index::getSymbolInfo(&ND); // FIXME: this is not classifying constructors, destructors and operators @@ -1210,12 +1211,9 @@ declToTypeHierarchyItem(ASTContext &Ctx, const NamedDecl &ND, THI.name = printName(Ctx, ND); THI.kind = SK; THI.deprecated = ND.isDeprecated(); - THI.range = halfOpenToRange( - SM, syntax::Token::range(SM, DeclToks->front(), DeclToks->back()) - .toCharRange(SM)); - THI.selectionRange = halfOpenToRange( - SM, syntax::Token::range(SM, NameToks->front(), NameToks->back()) - .toCharRange(SM)); + THI.range = Range{sourceLocToPosition(SM, DeclRange->getBegin()), + sourceLocToPosition(SM, DeclRange->getEnd())}; + THI.selectionRange = Range{NameBegin, NameEnd}; if (!THI.range.contains(THI.selectionRange)) { // 'selectionRange' must be contained in 'range', so in cases where clang // reports unrelated ranges we need to reconcile somehow. @@ -1282,8 +1280,7 @@ using RecursionProtectionSet = llvm::SmallSet; static void fillSuperTypes(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx, std::vector &SuperTypes, - RecursionProtectionSet &RPSet, - const syntax::TokenBuffer &TB) { + RecursionProtectionSet &RPSet) { // typeParents() will replace dependent template specializations // with their class template, so to avoid infinite recursion for // certain types of hierarchies, keep the templates encountered @@ -1298,9 +1295,9 @@ static void fillSuperTypes(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx, for (const CXXRecordDecl *ParentDecl : typeParents(&CXXRD)) { if (Optional ParentSym = - declToTypeHierarchyItem(ASTCtx, *ParentDecl, TB)) { + declToTypeHierarchyItem(ASTCtx, *ParentDecl)) { ParentSym->parents.emplace(); - fillSuperTypes(*ParentDecl, ASTCtx, *ParentSym->parents, RPSet, TB); + fillSuperTypes(*ParentDecl, ASTCtx, *ParentSym->parents, RPSet); SuperTypes.emplace_back(std::move(*ParentSym)); } } @@ -1404,7 +1401,7 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels, return llvm::None; Optional Result = - declToTypeHierarchyItem(AST.getASTContext(), *CXXRD, AST.getTokens()); + declToTypeHierarchyItem(AST.getASTContext(), *CXXRD); if (!Result) return Result; @@ -1413,8 +1410,7 @@ getTypeHierarchy(ParsedAST &AST, Position Pos, int ResolveLevels, Result->parents.emplace(); RecursionProtectionSet RPSet; - fillSuperTypes(*CXXRD, AST.getASTContext(), *Result->parents, RPSet, - AST.getTokens()); + fillSuperTypes(*CXXRD, AST.getASTContext(), *Result->parents, RPSet); } if ((Direction == TypeHierarchyDirection::Children || diff --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp index 73e124d..8e615e2 100644 --- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp @@ -523,6 +523,33 @@ TEST(TypeHierarchy, DeriveFromTemplate) { WithKind(SymbolKind::Struct), Children())))); } +TEST(TypeHierarchy, Preamble) { + Annotations SourceAnnotations(R"cpp( +struct Ch^ild : Parent { + int b; +};)cpp"); + + Annotations HeaderInPreambleAnnotations(R"cpp( +struct [[Parent]] { + int a; +};)cpp"); + + TestTU TU = TestTU::withCode(SourceAnnotations.code()); + TU.HeaderCode = HeaderInPreambleAnnotations.code().str(); + auto AST = TU.build(); + + llvm::Optional Result = getTypeHierarchy( + AST, SourceAnnotations.point(), 1, TypeHierarchyDirection::Parents); + + ASSERT_TRUE(Result); + EXPECT_THAT( + *Result, + AllOf(WithName("Child"), + Parents(AllOf(WithName("Parent"), + SelectionRangeIs(HeaderInPreambleAnnotations.range()), + Parents())))); +} + SymbolID findSymbolIDByName(SymbolIndex *Index, llvm::StringRef Name, llvm::StringRef TemplateArgs = "") { SymbolID Result; -- 2.7.4