[clangd] Color dependent names based on their heuristic target if they have one

Summary: Fixes https://github.com/clangd/clangd/issues/297

Subscribers: ilya-biryukov, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

clang-tools-extra/clangd/FindTarget.cpp

@@ -654,6 +654,7 @@ llvm::SmallVector<ReferenceLoc, 2> refInExpr(const Expr *E) {
                                   /*IsDecl=*/false,
                                   {E->getNamedConcept()}});
     }
+
     void VisitDeclRefExpr(const DeclRefExpr *E) {
       Refs.push_back(ReferenceLoc{E->getQualifierLoc(),
                                   E->getNameInfo().getLoc(),
@@ -661,6 +662,12 @@ llvm::SmallVector<ReferenceLoc, 2> refInExpr(const Expr *E) {
                                   {E->getFoundDecl()}});
     }
 
+    void VisitDependentScopeDeclRefExpr(const DependentScopeDeclRefExpr *E) {
+      Refs.push_back(ReferenceLoc{
+          E->getQualifierLoc(), E->getNameInfo().getLoc(), /*IsDecl=*/false,
+          explicitReferenceTargets(DynTypedNode::create(*E), {})});
+    }
+
     void VisitMemberExpr(const MemberExpr *E) {
       // Skip destructor calls to avoid duplication: TypeLoc within will be
       // visited separately.
@@ -672,6 +679,14 @@ llvm::SmallVector<ReferenceLoc, 2> refInExpr(const Expr *E) {
                                   {E->getFoundDecl()}});
     }
 
+    void
+    VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
+      Refs.push_back(
+          ReferenceLoc{E->getQualifierLoc(), E->getMemberNameInfo().getLoc(),
+                       /*IsDecl=*/false,
+                       explicitReferenceTargets(DynTypedNode::create(*E), {})});
+    }
+
     void VisitOverloadExpr(const OverloadExpr *E) {
       Refs.push_back(ReferenceLoc{E->getQualifierLoc(),
                                   E->getNameInfo().getLoc(),

clang-tools-extra/clangd/SemanticHighlighting.cpp

@@ -143,6 +143,36 @@ SourceLocation getHighlightableSpellingToken(SourceLocation L,
   return getHighlightableSpellingToken(SM.getImmediateSpellingLoc(L), SM);
 }
 
+unsigned evaluateHighlightPriority(HighlightingKind Kind) {
+  enum HighlightPriority { Dependent = 0, Resolved = 1 };
+  return Kind == HighlightingKind::DependentType ||
+                 Kind == HighlightingKind::DependentName
+             ? Dependent
+             : Resolved;
+}
+
+// Sometimes we get conflicts between findExplicitReferences() returning
+// a heuristic result for a dependent name (e.g. Method) and
+// CollectExtraHighlighting returning a fallback dependent highlighting (e.g.
+// DependentName). In such cases, resolve the conflict in favour of the
+// resolved (non-dependent) highlighting.
+// With macros we can get other conflicts (if a spelled token has multiple
+// expansions with different token types) which we can't usefully resolve.
+llvm::Optional<HighlightingToken>
+resolveConflict(ArrayRef<HighlightingToken> Tokens) {
+  if (Tokens.size() == 1)
+    return Tokens[0];
+
+  if (Tokens.size() != 2)
+    return llvm::None;
+
+  unsigned Priority1 = evaluateHighlightPriority(Tokens[0].Kind);
+  unsigned Priority2 = evaluateHighlightPriority(Tokens[1].Kind);
+  if (Priority1 == Priority2)
+    return llvm::None;
+  return Priority1 > Priority2 ? Tokens[0] : Tokens[1];
+}
+
 /// Consumes source locations and maps them to text ranges for highlightings.
 class HighlightingsBuilder {
 public:
@@ -183,10 +213,8 @@ public:
             // this predicate would never fire.
             return T.R == TokRef.front().R;
           });
-      // If there is exactly one token with this range it's non conflicting and
-      // should be in the highlightings.
-      if (Conflicting.size() == 1)
-        NonConflicting.push_back(TokRef.front());
+      if (auto Resolved = resolveConflict(Conflicting))
+        NonConflicting.push_back(*Resolved);
       // TokRef[Conflicting.size()] is the next token with a different range (or
       // the end of the Tokens).
       TokRef = TokRef.drop_front(Conflicting.size());

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

@@ -864,11 +864,11 @@ TEST_F(FindExplicitReferencesTest, All) {
         "0: targets = {x}, decl\n"
         "1: targets = {vector}\n"
         "2: targets = {x}\n"},
-       // Handle UnresolvedLookupExpr.
-       // FIXME
-       // This case fails when expensive checks are enabled.
-       // Seems like the order of ns1::func and ns2::func isn't defined.
-       #ifndef EXPENSIVE_CHECKS
+// Handle UnresolvedLookupExpr.
+// FIXME
+// This case fails when expensive checks are enabled.
+// Seems like the order of ns1::func and ns2::func isn't defined.
+#ifndef EXPENSIVE_CHECKS
        {R"cpp(
             namespace ns1 { void func(char*); }
             namespace ns2 { void func(int*); }
@@ -882,7 +882,7 @@ TEST_F(FindExplicitReferencesTest, All) {
         )cpp",
         "0: targets = {ns1::func, ns2::func}\n"
         "1: targets = {t}\n"},
-        #endif
+#endif
        // Handle UnresolvedMemberExpr.
        {R"cpp(
             struct X {
@@ -898,6 +898,35 @@ TEST_F(FindExplicitReferencesTest, All) {
         "0: targets = {x}\n"
         "1: targets = {X::func, X::func}\n"
         "2: targets = {t}\n"},
+       // Handle DependentScopeDeclRefExpr.
+       {R"cpp(
+            template <class T>
+            struct S {
+              static int value;
+            };
+
+            template <class T>
+            void foo() {
+              $0^S<$1^T>::$2^value;
+            }
+       )cpp",
+        "0: targets = {S}\n"
+        "1: targets = {T}\n"
+        "2: targets = {S::value}, qualifier = 'S<T>::'\n"},
+       // Handle CXXDependentScopeMemberExpr.
+       {R"cpp(
+            template <class T>
+            struct S {
+              int value;
+            };
+
+            template <class T>
+            void foo(S<T> t) {
+              $0^t.$1^value;
+            }
+       )cpp",
+        "0: targets = {t}\n"
+        "1: targets = {S::value}\n"},
        // Type template parameters.
        {R"cpp(
             template <class T>
@@ -1169,7 +1198,7 @@ TEST_F(FindExplicitReferencesTest, All) {
               namespace foo {
                 template <typename $0^T> requires $1^Drawable<$2^T>
                 void $3^bar($4^T $5^t) {
-                  $6^t.draw();
+                  $6^t.$7^draw();
                 }
               }
           )cpp",
@@ -1179,7 +1208,8 @@ TEST_F(FindExplicitReferencesTest, All) {
            "3: targets = {foo::bar}, decl\n"
            "4: targets = {T}\n"
            "5: targets = {t}, decl\n"
-           "6: targets = {t}\n"},
+           "6: targets = {t}\n"
+           "7: targets = {}\n"},
        // Objective-C: properties
        {
            R"cpp(

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

@@ -659,6 +659,20 @@ sizeof...($TemplateParameter[[Elements]]);
         static const int $StaticField[[Value]] = $TemplateParameter[[T]]
             ::$DependentType[[Resolver]]::$DependentName[[Value]];
       };
+    )cpp",
+      // Dependent name with heuristic target
+      R"cpp(
+      template <typename>
+      struct $Class[[Foo]] {
+        int $Field[[Waldo]];
+        void $Method[[bar]]() {
+          $Class[[Foo]]().$Field[[Waldo]];
+        }
+        template <typename $TemplateParameter[[U]]>
+        void $Method[[bar1]]() {
+          $Class[[Foo]]<$TemplateParameter[[U]]>().$Field[[Waldo]];
+        }
+      };
     )cpp",
       // Concepts
       R"cpp(