[PATCH] D84144: [clangd] Remove TokenBuffer usage in TypeHierarchy

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX updated this revision to Diff 279168.
ArcsinX added a comment.

NameRange => DeclRange
Simplify TypeHierarchy.Preamble test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84144/new/

https://reviews.llvm.org/D84144

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp


Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -523,6 +523,31 @@
                                    WithKind(SymbolKind::Struct), Children()))));
 }
 
+TEST(TypeHierarchy, Preamble) {
+  Annotations SourceAnnotations(R"cpp(
+#include "header_in_preamble.h"
+struct Ch^ild : Parent {
+  int b;
+};)cpp");
+
+  Annotations HeaderInPreambleAnnotations(R"cpp(
+struct Parent {
+  int a;
+};)cpp");
+
+  TestTU TU = TestTU::withCode(SourceAnnotations.code());
+  TU.AdditionalFiles["header_in_preamble.h"] =
+      HeaderInPreambleAnnotations.code().str();
+  auto AST = TU.build();
+
+  llvm::Optional<TypeHierarchyItem> Result = getTypeHierarchy(
+      AST, SourceAnnotations.point(), 1, TypeHierarchyDirection::Parents);
+
+  ASSERT_TRUE(Result);
+  EXPECT_THAT(*Result, AllOf(WithName("Child"),
+                             Parents(AllOf(WithName("Parent"), Parents()))));
+}
+
 SymbolID findSymbolIDByName(SymbolIndex *Index, llvm::StringRef Name,
                             llvm::StringRef TemplateArgs = "") {
   SymbolID Result;
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1183,23 +1183,24 @@
 
 // FIXME(nridge): Reduce duplication between this function and declToSym().
 static llvm::Optional<TypeHierarchyItem>
-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 @@
   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 @@
 
 static void fillSuperTypes(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx,
                            std::vector<TypeHierarchyItem> &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 @@
 
   for (const CXXRecordDecl *ParentDecl : typeParents(&CXXRD)) {
     if (Optional<TypeHierarchyItem> 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 @@
     return llvm::None;
 
   Optional<TypeHierarchyItem> Result =
-      declToTypeHierarchyItem(AST.getASTContext(), *CXXRD, AST.getTokens());
+      declToTypeHierarchyItem(AST.getASTContext(), *CXXRD);
   if (!Result)
     return Result;
 
@@ -1413,8 +1410,7 @@
     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 ||

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits