[clang-tools-extra] d83541d - [clangd] Color dependent names based on their heuristic target if they have one
Nathan Ridge via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 14 21:57:26 PDT 2020
Author: Nathan Ridge
Date: 2020-04-15T00:57:08-04:00
New Revision: d83541d1b8f75ff44083c8ba1dba1ca7bac330cf
URL: https://github.com/llvm/llvm-project/commit/d83541d1b8f75ff44083c8ba1dba1ca7bac330cf
DIFF: https://github.com/llvm/llvm-project/commit/d83541d1b8f75ff44083c8ba1dba1ca7bac330cf.diff
LOG: [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
Added:
Modified:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.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 ff50323980a7..27f6532bf5ac 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/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(),
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 35eabfe00dd2..baf13755ecf7 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/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
diff erent 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 @@ class HighlightingsBuilder {
// 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
diff erent range (or
// the end of the Tokens).
TokRef = TokRef.drop_front(Conflicting.size());
diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index cdea98c299e8..beb81a7c8054 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/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(
diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 1cca429a4ea6..2b4c306c5163 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/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(
More information about the cfe-commits
mailing list