nridge updated this revision to Diff 257596.
nridge added a comment.
Address final review comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76896/new/
https://reviews.llvm.org/D76896
Files:
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
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -659,6 +659,20 @@
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(
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -848,11 +848,11 @@
"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*); }
@@ -866,7 +866,7 @@
)cpp",
"0: targets = {ns1::func, ns2::func}\n"
"1: targets = {t}\n"},
- #endif
+#endif
// Handle UnresolvedMemberExpr.
{R"cpp(
struct X {
@@ -882,6 +882,35 @@
"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>
@@ -1153,7 +1182,7 @@
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",
@@ -1163,7 +1192,8 @@
"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(
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -143,6 +143,36 @@
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 @@
// 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());
Index: clang-tools-extra/clangd/FindTarget.cpp
===================================================================
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -646,6 +646,7 @@
/*IsDecl=*/false,
{E->getNamedConcept()}});
}
+
void VisitDeclRefExpr(const DeclRefExpr *E) {
Refs.push_back(ReferenceLoc{E->getQualifierLoc(),
E->getNameInfo().getLoc(),
@@ -653,6 +654,12 @@
{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.
@@ -664,6 +671,14 @@
{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(),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
