[clang-tools-extra] 87e0cb4 - [clangd] Implement semantic highlightings via findExplicitReferences

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 5 10:34:56 PST 2019


Author: Ilya Biryukov
Date: 2019-11-05T19:15:24+01:00
New Revision: 87e0cb4f1ad299c87c3e26676a9b31b3caf58921

URL: https://github.com/llvm/llvm-project/commit/87e0cb4f1ad299c87c3e26676a9b31b3caf58921
DIFF: https://github.com/llvm/llvm-project/commit/87e0cb4f1ad299c87c3e26676a9b31b3caf58921.diff

LOG: [clangd] Implement semantic highlightings via findExplicitReferences

Summary:
To keep the logic of finding locations of interesting AST nodes in one
place.

The advantage is better coverage of various AST nodes, both now and in
the future: as new nodes get added to `findExplicitReferences`, semantic
highlighting will automatically pick them up.

The drawback of this change is that we have to traverse declarations
inside our file twice in order to highlight dependent names, 'auto'
and 'decltype'. Hopefully, this should not affect the actual latency
too much, most time should be spent in building the AST and not
traversing it.

Reviewers: hokein

Reviewed By: hokein

Subscribers: nridge, merge_guards_bot, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/FindTarget.cpp
    clang-tools-extra/clangd/FindTarget.h
    clang-tools-extra/clangd/SemanticHighlighting.cpp
    clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp
index 4e61d22dd7fb..c536cbf75e5c 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -732,6 +732,10 @@ void findExplicitReferences(const Decl *D,
   assert(D);
   ExplicitReferenceColletor(Out).TraverseDecl(const_cast<Decl *>(D));
 }
+void findExplicitReferences(const ASTContext &AST,
+                            llvm::function_ref<void(ReferenceLoc)> Out) {
+  ExplicitReferenceColletor(Out).TraverseAST(const_cast<ASTContext &>(AST));
+}
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, DeclRelation R) {
   switch (R) {

diff  --git a/clang-tools-extra/clangd/FindTarget.h b/clang-tools-extra/clangd/FindTarget.h
index 9ddd24adb895..bd5d7690ceed 100644
--- a/clang-tools-extra/clangd/FindTarget.h
+++ b/clang-tools-extra/clangd/FindTarget.h
@@ -22,6 +22,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_FINDTARGET_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_FINDTARGET_H
 
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/Stmt.h"
@@ -107,6 +108,8 @@ void findExplicitReferences(const Stmt *S,
                             llvm::function_ref<void(ReferenceLoc)> Out);
 void findExplicitReferences(const Decl *D,
                             llvm::function_ref<void(ReferenceLoc)> Out);
+void findExplicitReferences(const ASTContext &AST,
+                            llvm::function_ref<void(ReferenceLoc)> Out);
 
 /// Similar to targetDecl(), however instead of applying a filter, all possible
 /// decls are returned along with their DeclRelationSets.

diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 62d3a164b5b8..9838600584c6 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "SemanticHighlighting.h"
+#include "FindTarget.h"
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
@@ -15,10 +16,16 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclarationName.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/Casting.h"
 #include <algorithm>
 
 namespace clang {
@@ -103,12 +110,12 @@ llvm::Optional<HighlightingKind> kindForType(const Type *TP) {
     return kindForDecl(TD);
   return llvm::None;
 }
-// Given a set of candidate declarations, if the declarations all have the same
-// highlighting kind, return that highlighting kind, otherwise return None.
-template <typename IteratorRange>
-llvm::Optional<HighlightingKind> kindForCandidateDecls(IteratorRange Decls) {
+
+llvm::Optional<HighlightingKind> kindForReference(const ReferenceLoc &R) {
   llvm::Optional<HighlightingKind> Result;
-  for (NamedDecl *Decl : Decls) {
+  for (const NamedDecl *Decl : R.Targets) {
+    if (!canHighlightName(Decl->getDeclName()))
+      return llvm::None;
     auto Kind = kindForDecl(Decl);
     if (!Kind || (Result && Kind != Result))
       return llvm::None;
@@ -117,27 +124,49 @@ llvm::Optional<HighlightingKind> kindForCandidateDecls(IteratorRange Decls) {
   return Result;
 }
 
-// Collects all semantic tokens in an ASTContext.
-class HighlightingTokenCollector
-    : public RecursiveASTVisitor<HighlightingTokenCollector> {
-  std::vector<HighlightingToken> Tokens;
-  ParsedAST &AST;
-
+/// Consumes source locations and maps them to text ranges for highlightings.
+class HighlightingsBuilder {
 public:
-  HighlightingTokenCollector(ParsedAST &AST) : AST(AST) {}
-
-  std::vector<HighlightingToken> collectTokens() {
-    Tokens.clear();
-    TraverseAST(AST.getASTContext());
-    // Add highlightings for macro expansions as they are not traversed by the
-    // visitor.
-    for (const auto &M : AST.getMacros().Ranges)
-      Tokens.push_back({HighlightingKind::Macro, M});
+  HighlightingsBuilder(const SourceManager &SourceMgr,
+                       const LangOptions &LangOpts)
+      : SourceMgr(SourceMgr), LangOpts(LangOpts) {}
+
+  void addToken(HighlightingToken T) { Tokens.push_back(T); }
+
+  void addToken(SourceLocation Loc, HighlightingKind Kind) {
+    if (Loc.isInvalid())
+      return;
+    if (Loc.isMacroID()) {
+      // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
+      if (!SourceMgr.isMacroArgExpansion(Loc))
+        return;
+      Loc = SourceMgr.getSpellingLoc(Loc);
+    }
+
+    // Non top level decls that are included from a header are not filtered by
+    // topLevelDecls. (example: method declarations being included from
+    // another file for a class from another file).
+    // There are also cases with macros where the spelling loc will not be in
+    // the main file and the highlighting would be incorrect.
+    if (!isInsideMainFile(Loc, SourceMgr))
+      return;
+
+    auto Range = getTokenRange(SourceMgr, LangOpts, Loc);
+    if (!Range) {
+      // R should always have a value, if it doesn't something is very wrong.
+      elog("Tried to add semantic token with an invalid range");
+      return;
+    }
+    Tokens.push_back(HighlightingToken{Kind, *Range});
+  }
+
+  std::vector<HighlightingToken> collect() && {
     // Initializer lists can give duplicates of tokens, therefore all tokens
     // must be deduplicated.
     llvm::sort(Tokens);
     auto Last = std::unique(Tokens.begin(), Tokens.end());
     Tokens.erase(Last, Tokens.end());
+
     // Macros can give tokens that have the same source range but conflicting
     // kinds. In this case all tokens sharing this source range should be
     // removed.
@@ -161,155 +190,59 @@ class HighlightingTokenCollector
     return NonConflicting;
   }
 
-  bool VisitNamespaceAliasDecl(NamespaceAliasDecl *NAD) {
-    // The target namespace of an alias can not be found in any other way.
-    addToken(NAD->getTargetNameLoc(), NAD->getAliasedNamespace());
-    return true;
-  }
-
-  bool VisitMemberExpr(MemberExpr *ME) {
-    if (canHighlightName(ME->getMemberNameInfo().getName()))
-      addToken(ME->getMemberLoc(), ME->getMemberDecl());
-    return true;
-  }
-
-  bool VisitOverloadExpr(OverloadExpr *E) {
-    if (canHighlightName(E->getName()))
-      addToken(E->getNameLoc(),
-               kindForCandidateDecls(E->decls())
-                   .getValueOr(HighlightingKind::DependentName));
-    return true;
-  }
-
-  bool VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) {
-    if (canHighlightName(E->getDeclName()))
-      addToken(E->getLocation(), HighlightingKind::DependentName);
-    return true;
-  }
-
-  bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
-    if (canHighlightName(E->getMember()))
-      addToken(E->getMemberLoc(), HighlightingKind::DependentName);
-    return true;
-  }
-
-  bool VisitNamedDecl(NamedDecl *ND) {
-    if (canHighlightName(ND->getDeclName()))
-      addToken(ND->getLocation(), ND);
-    return true;
-  }
+private:
+  const SourceManager &SourceMgr;
+  const LangOptions &LangOpts;
+  std::vector<HighlightingToken> Tokens;
+};
 
-  bool VisitUsingDecl(UsingDecl *UD) {
-    if (auto K = kindForCandidateDecls(UD->shadows()))
-      addToken(UD->getLocation(), *K);
-    return true;
-  }
+/// Produces highlightings, which are not captured by findExplicitReferences,
+/// e.g. highlights dependent names and 'auto' as the underlying type.
+class CollectExtraHighlightings
+    : public RecursiveASTVisitor<CollectExtraHighlightings> {
+public:
+  CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {}
 
-  bool VisitDeclRefExpr(DeclRefExpr *Ref) {
-    if (canHighlightName(Ref->getNameInfo().getName()))
-      addToken(Ref->getLocation(), Ref->getDecl());
+  bool VisitDecltypeTypeLoc(DecltypeTypeLoc L) {
+    if (auto K = kindForType(L.getTypePtr()))
+      H.addToken(L.getBeginLoc(), *K);
     return true;
   }
 
-  bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
-    addToken(TL.getBeginLoc(), TL.getTypedefNameDecl());
+  bool VisitDeclaratorDecl(DeclaratorDecl *D) {
+    auto *AT = D->getType()->getContainedAutoType();
+    if (!AT)
+      return true;
+    if (auto K = kindForType(AT->getDeducedType().getTypePtrOrNull()))
+      H.addToken(D->getTypeSpecStartLoc(), *K);
     return true;
   }
 
-  bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
-    if (const TemplateDecl *TD =
-            TL.getTypePtr()->getTemplateName().getAsTemplateDecl())
-      addToken(TL.getBeginLoc(), TD);
+  bool VisitOverloadExpr(OverloadExpr *E) {
+    if (!E->decls().empty())
+      return true; // handled by findExplicitReferences.
+    H.addToken(E->getNameLoc(), HighlightingKind::DependentName);
     return true;
   }
 
-  bool VisitTagTypeLoc(TagTypeLoc L) {
-    if (L.isDefinition())
-      return true; // Definition will be highligthed by VisitNamedDecl.
-    if (auto K = kindForType(L.getTypePtr()))
-      addToken(L.getBeginLoc(), *K);
+  bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+    H.addToken(E->getMemberNameInfo().getLoc(),
+               HighlightingKind::DependentName);
     return true;
   }
 
-  bool VisitDecltypeTypeLoc(DecltypeTypeLoc L) {
-    if (auto K = kindForType(L.getTypePtr()))
-      addToken(L.getBeginLoc(), *K);
+  bool VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) {
+    H.addToken(E->getNameInfo().getLoc(), HighlightingKind::DependentName);
     return true;
   }
 
   bool VisitDependentNameTypeLoc(DependentNameTypeLoc L) {
-    addToken(L.getNameLoc(), HighlightingKind::DependentType);
-    return true;
-  }
-
-  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TL) {
-    addToken(TL.getBeginLoc(), HighlightingKind::TemplateParameter);
-    return true;
-  }
-
-  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) {
-    if (auto *NNS = NNSLoc.getNestedNameSpecifier()) {
-      if (NNS->getKind() == NestedNameSpecifier::Namespace ||
-          NNS->getKind() == NestedNameSpecifier::NamespaceAlias)
-        addToken(NNSLoc.getLocalBeginLoc(), HighlightingKind::Namespace);
-    }
-    return RecursiveASTVisitor<
-        HighlightingTokenCollector>::TraverseNestedNameSpecifierLoc(NNSLoc);
-  }
-
-  bool TraverseConstructorInitializer(CXXCtorInitializer *CI) {
-    if (const FieldDecl *FD = CI->getMember())
-      addToken(CI->getSourceLocation(), FD);
-    return RecursiveASTVisitor<
-        HighlightingTokenCollector>::TraverseConstructorInitializer(CI);
-  }
-
-  bool VisitDeclaratorDecl(DeclaratorDecl *D) {
-    // Highlight 'auto' with its underlying type.
-    auto *AT = D->getType()->getContainedAutoType();
-    if (!AT)
-      return true;
-    auto K = kindForType(AT->getDeducedType().getTypePtrOrNull());
-    if (!K)
-      return true;
-    addToken(D->getTypeSpecStartLoc(), *K);
+    H.addToken(L.getNameLoc(), HighlightingKind::DependentType);
     return true;
   }
 
 private:
-  void addToken(SourceLocation Loc, HighlightingKind Kind) {
-    if (Loc.isInvalid())
-      return;
-    const auto &SM = AST.getSourceManager();
-    if (Loc.isMacroID()) {
-      // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
-      if (!SM.isMacroArgExpansion(Loc))
-        return;
-      Loc = SM.getSpellingLoc(Loc);
-    }
-
-    // Non top level decls that are included from a header are not filtered by
-    // topLevelDecls. (example: method declarations being included from
-    // another file for a class from another file).
-    // There are also cases with macros where the spelling loc will not be in
-    // the main file and the highlighting would be incorrect.
-    if (!isInsideMainFile(Loc, SM))
-      return;
-
-    auto R = getTokenRange(SM, AST.getASTContext().getLangOpts(), Loc);
-    if (!R) {
-      // R should always have a value, if it doesn't something is very wrong.
-      elog("Tried to add semantic token with an invalid range");
-      return;
-    }
-
-    Tokens.push_back({Kind, R.getValue()});
-  }
-
-  void addToken(SourceLocation Loc, const NamedDecl *D) {
-    if (auto K = kindForDecl(D))
-      addToken(Loc, *K);
-  }
+  HighlightingsBuilder &H;
 };
 
 // Encode binary data into base64.
@@ -367,6 +300,23 @@ takeLine(ArrayRef<HighlightingToken> AllTokens,
 }
 } // namespace
 
+std::vector<HighlightingToken> getSemanticHighlightings(ParsedAST &AST) {
+  auto &C = AST.getASTContext();
+  // Add highlightings for AST nodes.
+  HighlightingsBuilder Builder(AST.getSourceManager(), C.getLangOpts());
+  // Highlight 'decltype' and 'auto' as their underlying types.
+  CollectExtraHighlightings(Builder).TraverseAST(C);
+  // Highlight all decls and references coming from the AST.
+  findExplicitReferences(C, [&](ReferenceLoc R) {
+    if (auto Kind = kindForReference(R))
+      Builder.addToken(R.NameLoc, *Kind);
+  });
+  // Add highlightings for macro expansions.
+  for (const auto &M : AST.getMacros().Ranges)
+    Builder.addToken({HighlightingKind::Macro, M});
+  return std::move(Builder).collect();
+}
+
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K) {
   switch (K) {
   case HighlightingKind::Variable:
@@ -466,10 +416,6 @@ bool operator==(const LineHighlightings &L, const LineHighlightings &R) {
   return std::tie(L.Line, L.Tokens) == std::tie(R.Line, R.Tokens);
 }
 
-std::vector<HighlightingToken> getSemanticHighlightings(ParsedAST &AST) {
-  return HighlightingTokenCollector(AST).collectTokens();
-}
-
 std::vector<SemanticHighlightingInformation>
 toSemanticHighlightingInformation(llvm::ArrayRef<LineHighlightings> Tokens) {
   if (Tokens.size() == 0)

diff  --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 139773b616ea..36237e94dfe0 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -589,6 +589,15 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
       R"cpp(
       void $Function[[foo]]();
       using ::$Function[[foo]];
+    )cpp",
+      // Highlighting of template template arguments.
+      R"cpp(
+      template <template <class> class $TemplateParameter[[TT]],
+                template <class> class ...$TemplateParameter[[TTs]]>
+      struct $Class[[Foo]] {
+        $Class[[Foo]]<$TemplateParameter[[TT]], $TemplateParameter[[TTs]]...>
+          *$Field[[t]];
+      }
     )cpp"};
   for (const auto &TestCase : TestCases) {
     checkHighlightings(TestCase);


        


More information about the cfe-commits mailing list