[clangd] Get rid of getTokenRange helper
authorKadir Cetinkaya <kadircet@google.com>
Sun, 1 Mar 2020 15:05:12 +0000 (16:05 +0100)
committerKadir Cetinkaya <kadircet@google.com>
Tue, 3 Mar 2020 13:30:42 +0000 (14:30 +0100)
Summary:

Reviewers: sammccall

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D75474

clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/CollectMacros.cpp [new file with mode: 0644]
clang-tools-extra/clangd/CollectMacros.h
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/XRefs.cpp

index fc5a07e69e9d628ff82ece2c92514e7c31f29e27..5db345ecc63f093772eb0cdf7b23d0024be2a397 100644 (file)
@@ -41,6 +41,7 @@ add_clang_library(clangDaemon
   ClangdServer.cpp
   CodeComplete.cpp
   CodeCompletionStrings.cpp
+  CollectMacros.cpp
   CompileCommands.cpp
   Compiler.cpp
   Context.cpp
diff --git a/clang-tools-extra/clangd/CollectMacros.cpp b/clang-tools-extra/clangd/CollectMacros.cpp
new file mode 100644 (file)
index 0000000..ea7dd18
--- /dev/null
@@ -0,0 +1,34 @@
+//===--- CollectMacros.cpp ---------------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CollectMacros.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Lexer.h"
+
+namespace clang {
+namespace clangd {
+
+void CollectMainFileMacros::add(const Token &MacroNameTok,
+                                const MacroInfo *MI) {
+  if (!InMainFile)
+    return;
+  auto Loc = MacroNameTok.getLocation();
+  if (Loc.isInvalid() || Loc.isMacroID())
+    return;
+
+  auto Name = MacroNameTok.getIdentifierInfo()->getName();
+  Out.Names.insert(Name);
+  auto Range = halfOpenToRange(
+      SM, CharSourceRange::getCharRange(Loc, MacroNameTok.getEndLoc()));
+  if (auto SID = getSymbolID(Name, MI, SM))
+    Out.MacroRefs[*SID].push_back(Range);
+  else
+    Out.UnknownMacros.push_back(Range);
+}
+} // namespace clangd
+} // namespace clang
index 5c3fca10ad4a5caed3b511cd7cea2d32664e7cbd..eecea0455be2702fd837a84cb0f8dc28f878ff47 100644 (file)
@@ -40,10 +40,8 @@ struct MainFileMacros {
 ///  - collect macros after the preamble of the main file (in ParsedAST.cpp)
 class CollectMainFileMacros : public PPCallbacks {
 public:
-  explicit CollectMainFileMacros(const SourceManager &SM,
-                                 const LangOptions &LangOpts,
-                                 MainFileMacros &Out)
-      : SM(SM), LangOpts(LangOpts), Out(Out) {}
+  explicit CollectMainFileMacros(const SourceManager &SM, MainFileMacros &Out)
+      : SM(SM), Out(Out) {}
 
   void FileChanged(SourceLocation Loc, FileChangeReason,
                    SrcMgr::CharacteristicKind, FileID) override {
@@ -89,24 +87,8 @@ public:
   }
 
 private:
-  void add(const Token &MacroNameTok, const MacroInfo *MI) {
-    if (!InMainFile)
-      return;
-    auto Loc = MacroNameTok.getLocation();
-    if (Loc.isMacroID())
-      return;
-
-    if (auto Range = getTokenRange(SM, LangOpts, Loc)) {
-      auto Name = MacroNameTok.getIdentifierInfo()->getName();
-      Out.Names.insert(Name);
-      if (auto SID = getSymbolID(Name, MI, SM))
-        Out.MacroRefs[*SID].push_back(*Range);
-      else
-        Out.UnknownMacros.push_back(*Range);
-    }
-  }
+  void add(const Token &MacroNameTok, const MacroInfo *MI);
   const SourceManager &SM;
-  const LangOptions &LangOpts;
   bool InMainFile = true;
   MainFileMacros &Out;
 };
index 5796657a5800be889ad6396204c61971011ababd..5c1288c14b5860bd23401411d3864801f090d7f4 100644 (file)
@@ -26,6 +26,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Index/IndexSymbol.h"
@@ -530,32 +531,33 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
     llvm::consumeError(CurLoc.takeError());
     return llvm::None;
   }
-  auto TokensTouchingCursor =
-      syntax::spelledTokensTouching(*CurLoc, AST.getTokens());
+  const auto &TB = AST.getTokens();
+  auto TokensTouchingCursor = syntax::spelledTokensTouching(*CurLoc, TB);
   // Early exit if there were no tokens around the cursor.
   if (TokensTouchingCursor.empty())
     return llvm::None;
 
-  // To be used as a backup for highlighting the selected token.
-  SourceLocation IdentLoc;
+  // To be used as a backup for highlighting the selected token, we use back as
+  // it aligns better with biases elsewhere (editors tend to send the position
+  // for the left of the hovered token).
+  CharSourceRange HighlightRange =
+      TokensTouchingCursor.back().range(SM).toCharRange(SM);
   llvm::Optional<HoverInfo> HI;
   // Macros and deducedtype only works on identifiers and auto/decltype keywords
   // respectively. Therefore they are only trggered on whichever works for them,
   // similar to SelectionTree::create().
   for (const auto &Tok : TokensTouchingCursor) {
     if (Tok.kind() == tok::identifier) {
-      IdentLoc = Tok.location();
+      // Prefer the identifier token as a fallback highlighting range.
+      HighlightRange = Tok.range(SM).toCharRange(SM);
       if (auto M = locateMacroAt(Tok, AST.getPreprocessor())) {
         HI = getHoverContents(*M, AST);
-        HI->SymRange = getTokenRange(AST.getSourceManager(), AST.getLangOpts(),
-                                     Tok.location());
         break;
       }
     } else if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype) {
       if (auto Deduced = getDeducedType(AST.getASTContext(), Tok.location())) {
         HI = getHoverContents(*Deduced, AST.getASTContext(), Index);
-        HI->SymRange = getTokenRange(AST.getSourceManager(), AST.getLangOpts(),
-                                     Tok.location());
+        HighlightRange = Tok.range(SM).toCharRange(SM);
         break;
       }
     }
@@ -566,10 +568,11 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
     auto Offset = SM.getFileOffset(*CurLoc);
     // Editors send the position on the left of the hovered character.
     // So our selection tree should be biased right. (Tested with VSCode).
-    SelectionTree ST = SelectionTree::createRight(
-        AST.getASTContext(), AST.getTokens(), Offset, Offset);
+    SelectionTree ST =
+        SelectionTree::createRight(AST.getASTContext(), TB, Offset, Offset);
     std::vector<const Decl *> Result;
     if (const SelectionTree::Node *N = ST.commonAncestor()) {
+      // FIXME: Fill in HighlightRange with range coming from N->ASTNode.
       auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias);
       if (!Decls.empty()) {
         HI = getHoverContents(Decls.front(), Index);
@@ -592,14 +595,7 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
   if (auto Formatted =
           tooling::applyAllReplacements(HI->Definition, Replacements))
     HI->Definition = *Formatted;
-  // FIXME: We should rather fill this with info coming from SelectionTree node.
-  if (!HI->SymRange) {
-    SourceLocation ToHighlight = TokensTouchingCursor.front().location();
-    if (IdentLoc.isValid())
-      ToHighlight = IdentLoc;
-    HI->SymRange =
-        getTokenRange(AST.getSourceManager(), AST.getLangOpts(), ToHighlight);
-  }
+  HI->SymRange = halfOpenToRange(SM, HighlightRange);
 
   return HI;
 }
index 36a9c47f7a9d2e9841d41ae53bd85122f08a640b..e43c2ce662616cb26bb7c5c78fb3bb3b9e174ce4 100644 (file)
@@ -350,7 +350,7 @@ ParsedAST::build(std::unique_ptr<clang::CompilerInvocation> CI,
     Macros = Preamble->Macros;
   Clang->getPreprocessor().addPPCallbacks(
       std::make_unique<CollectMainFileMacros>(Clang->getSourceManager(),
-                                              Clang->getLangOpts(), Macros));
+                                              Macros));
 
   // Copy over the includes from the preamble, then combine with the
   // non-preamble includes below.
index eca545fd09e4b39a90601d75e938c4b5e51896f7..f2b6b017f10f58f7faac5b142fdc1acf0c5f4a28 100644 (file)
@@ -54,7 +54,7 @@ public:
 
     return std::make_unique<PPChainedCallbacks>(
         collectIncludeStructureCallback(*SourceMgr, &Includes),
-        std::make_unique<CollectMainFileMacros>(*SourceMgr, *LangOpts, Macros));
+        std::make_unique<CollectMainFileMacros>(*SourceMgr, Macros));
   }
 
   CommentHandler *getCommentHandler() override {
index 79d027def4bc35402fe2245281ea468b410f47e8..d18daa910d18eed46c6e8ab7f222aeb87d179eda 100644 (file)
@@ -225,17 +225,6 @@ bool isSpelledInSource(SourceLocation Loc, const SourceManager &SM) {
   return true;
 }
 
-llvm::Optional<Range> getTokenRange(const SourceManager &SM,
-                                    const LangOptions &LangOpts,
-                                    SourceLocation TokLoc) {
-  if (!TokLoc.isValid())
-    return llvm::None;
-  SourceLocation End = Lexer::getLocForEndOfToken(TokLoc, 0, SM, LangOpts);
-  if (!End.isValid())
-    return llvm::None;
-  return halfOpenToRange(SM, CharSourceRange::getCharRange(TokLoc, End));
-}
-
 bool isValidFileRange(const SourceManager &Mgr, SourceRange R) {
   if (!R.getBegin().isValid() || !R.getEnd().isValid())
     return false;
@@ -645,8 +634,7 @@ std::vector<Range> collectIdentifierRanges(llvm::StringRef Identifier,
       [&](const syntax::Token &Tok, const SourceManager &SM) {
         if (Tok.kind() != tok::identifier || Tok.text(SM) != Identifier)
           return;
-        if (auto Range = getTokenRange(SM, LangOpts, Tok.location()))
-          Ranges.push_back(*Range);
+        Ranges.push_back(halfOpenToRange(SM, Tok.range(SM).toCharRange(SM)));
       });
   return Ranges;
 }
index c601cc89df28095b689b9f352a5aa753c90f7961..383c57371b0059eef08577ade4009ee65f5bfcff 100644 (file)
@@ -69,11 +69,6 @@ Position offsetToPosition(llvm::StringRef Code, size_t Offset);
 /// FIXME: This should return an error if the location is invalid.
 Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc);
 
-/// Returns the taken range at \p TokLoc.
-llvm::Optional<Range> getTokenRange(const SourceManager &SM,
-                                    const LangOptions &LangOpts,
-                                    SourceLocation TokLoc);
-
 /// Return the file location, corresponding to \p P. Note that one should take
 /// care to avoid comparing the result with expansion locations.
 llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM,
index 560e39e13550397ab9bc082c2b8ec8fb6b8a395c..67f7bda6a5e65bd51dee8347208e5810ce36427e 100644 (file)
@@ -29,6 +29,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexDataConsumer.h"
@@ -149,25 +150,27 @@ std::vector<const NamedDecl *> getDeclAtPosition(ParsedAST &AST,
   return Result;
 }
 
-llvm::Optional<Location> makeLocation(ASTContext &AST, SourceLocation TokLoc,
+// Expects Loc to be a SpellingLocation, will bail out otherwise as it can't
+// figure out a filename.
+llvm::Optional<Location> makeLocation(const ASTContext &AST, SourceLocation Loc,
                                       llvm::StringRef TUPath) {
-  const SourceManager &SourceMgr = AST.getSourceManager();
-  const FileEntry *F = SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc));
+  const auto &SM = AST.getSourceManager();
+  const FileEntry *F = SM.getFileEntryForID(SM.getFileID(Loc));
   if (!F)
     return None;
-  auto FilePath = getCanonicalPath(F, SourceMgr);
+  auto FilePath = getCanonicalPath(F, SM);
   if (!FilePath) {
     log("failed to get path!");
     return None;
   }
-  if (auto Range =
-          getTokenRange(AST.getSourceManager(), AST.getLangOpts(), TokLoc)) {
-    Location L;
-    L.uri = URIForFile::canonicalize(*FilePath, TUPath);
-    L.range = *Range;
-    return L;
-  }
-  return None;
+  Location L;
+  L.uri = URIForFile::canonicalize(*FilePath, TUPath);
+  // We call MeasureTokenLength here as TokenBuffer doesn't store spelled tokens
+  // outside the main file.
+  auto TokLen = Lexer::MeasureTokenLength(Loc, SM, AST.getLangOpts());
+  L.range = halfOpenToRange(
+      SM, CharSourceRange::getCharRange(Loc, Loc.getLocWithOffset(TokLen)));
+  return L;
 }
 
 } // namespace
@@ -373,11 +376,15 @@ namespace {
 class ReferenceFinder : public index::IndexDataConsumer {
 public:
   struct Reference {
-    SourceLocation Loc;
+    syntax::Token SpelledTok;
     index::SymbolRoleSet Role;
+
+    Range range(const SourceManager &SM) const {
+      return halfOpenToRange(SM, SpelledTok.range(SM).toCharRange(SM));
+    }
   };
 
-  ReferenceFinder(ASTContext &AST, Preprocessor &PP,
+  ReferenceFinder(const ParsedAST &AST,
                   const std::vector<const NamedDecl *> &TargetDecls)
       : AST(AST) {
     for (const NamedDecl *D : TargetDecls)
@@ -386,13 +393,17 @@ public:
 
   std::vector<Reference> take() && {
     llvm::sort(References, [](const Reference &L, const Reference &R) {
-      return std::tie(L.Loc, L.Role) < std::tie(R.Loc, R.Role);
+      auto LTok = L.SpelledTok.location();
+      auto RTok = R.SpelledTok.location();
+      return std::tie(LTok, L.Role) < std::tie(RTok, R.Role);
     });
     // We sometimes see duplicates when parts of the AST get traversed twice.
     References.erase(std::unique(References.begin(), References.end(),
                                  [](const Reference &L, const Reference &R) {
-                                   return std::tie(L.Loc, L.Role) ==
-                                          std::tie(R.Loc, R.Role);
+                                   auto LTok = L.SpelledTok.location();
+                                   auto RTok = R.SpelledTok.location();
+                                   return std::tie(LTok, L.Role) ==
+                                          std::tie(RTok, R.Role);
                                  }),
                      References.end());
     return std::move(References);
@@ -404,22 +415,27 @@ public:
                        SourceLocation Loc,
                        index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
     assert(D->isCanonicalDecl() && "expect D to be a canonical declaration");
+    if (!CanonicalTargets.count(D))
+      return true;
+    const auto &TB = AST.getTokens();
     const SourceManager &SM = AST.getSourceManager();
     Loc = SM.getFileLoc(Loc);
-    if (isInsideMainFile(Loc, SM) && CanonicalTargets.count(D))
-      References.push_back({Loc, Roles});
+    // We are only traversing decls *inside* the main file, so this should hold.
+    assert(isInsideMainFile(Loc, SM));
+    if (const auto *Tok = TB.spelledTokenAt(Loc))
+      References.push_back({*Tok, Roles});
     return true;
   }
 
 private:
   llvm::SmallSet<const Decl *, 4> CanonicalTargets;
   std::vector<Reference> References;
-  const ASTContext &AST;
+  const ParsedAST &AST;
 };
 
 std::vector<ReferenceFinder::Reference>
 findRefs(const std::vector<const NamedDecl *> &Decls, ParsedAST &AST) {
-  ReferenceFinder RefFinder(AST.getASTContext(), AST.getPreprocessor(), Decls);
+  ReferenceFinder RefFinder(AST, Decls);
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -450,18 +466,15 @@ std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
   // different kinds, deduplicate them.
   std::vector<DocumentHighlight> Result;
   for (const auto &Ref : References) {
-    if (auto Range =
-            getTokenRange(AST.getSourceManager(), AST.getLangOpts(), Ref.Loc)) {
-      DocumentHighlight DH;
-      DH.range = *Range;
-      if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
-        DH.kind = DocumentHighlightKind::Write;
-      else if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Read))
-        DH.kind = DocumentHighlightKind::Read;
-      else
-        DH.kind = DocumentHighlightKind::Text;
-      Result.push_back(std::move(DH));
-    }
+    DocumentHighlight DH;
+    DH.range = Ref.range(SM);
+    if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
+      DH.kind = DocumentHighlightKind::Write;
+    else if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Read))
+      DH.kind = DocumentHighlightKind::Read;
+    else
+      DH.kind = DocumentHighlightKind::Text;
+    Result.push_back(std::move(DH));
   }
   return Result;
 }
@@ -524,16 +537,15 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
     MainFileRefs.erase(std::unique(MainFileRefs.begin(), MainFileRefs.end(),
                                    [](const ReferenceFinder::Reference &L,
                                       const ReferenceFinder::Reference &R) {
-                                     return L.Loc == R.Loc;
+                                     return L.SpelledTok.location() ==
+                                            R.SpelledTok.location();
                                    }),
                        MainFileRefs.end());
     for (const auto &Ref : MainFileRefs) {
-      if (auto Range = getTokenRange(SM, AST.getLangOpts(), Ref.Loc)) {
-        Location Result;
-        Result.range = *Range;
-        Result.uri = URIMainFile;
-        Results.References.push_back(std::move(Result));
-      }
+      Location Result;
+      Result.range = Ref.range(SM);
+      Result.uri = URIMainFile;
+      Results.References.push_back(std::move(Result));
     }
     if (Index && Results.References.size() <= Limit) {
       for (const Decl *D : Decls) {