[clang-tools-extra] 59c1139 - [clangd] Expose more dependent-name detail via semanticTokens
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 9 11:41:08 PST 2021
Author: Sam McCall
Date: 2021-02-09T20:40:59+01:00
New Revision: 59c1139d3ee127a2049de5c711f81d46d8ec4e41
URL: https://github.com/llvm/llvm-project/commit/59c1139d3ee127a2049de5c711f81d46d8ec4e41
DIFF: https://github.com/llvm/llvm-project/commit/59c1139d3ee127a2049de5c711f81d46d8ec4e41.diff
LOG: [clangd] Expose more dependent-name detail via semanticTokens
This change makes dependentName a modifier, rather than a token type.
It can be combined with:
- type (new, standard) - this combination replaces dependentType like T::typename Foo
- unknown (new, nonstandard) - for general dependent names
- Field, etc - when the name is dependent but we heuristically resolve it
While here, fix cases where template-template-parameter cases were
incorrectly flagged as type-dependent.
And the merging of modifiers when resolving conflicts accidentally
happens to work around a bug that showed up in a test.
The behavior observed through the pre-standard protocol should be mostly
unchanged (it'll see the bugfixes only). This is done in a somehat
fragile way but it's not expected to live long.
Differential Revision: https://reviews.llvm.org/D95706
Added:
Modified:
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/test/semantic-tokens.test
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 29afae3746f8..0e50fc133e4d 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -213,21 +213,31 @@ SourceLocation getHighlightableSpellingToken(SourceLocation L,
return getHighlightableSpellingToken(SM.getImmediateSpellingLoc(L), SM);
}
-unsigned evaluateHighlightPriority(HighlightingKind Kind) {
+unsigned evaluateHighlightPriority(const HighlightingToken &Tok) {
enum HighlightPriority { Dependent = 0, Resolved = 1 };
- return Kind == HighlightingKind::DependentType ||
- Kind == HighlightingKind::DependentName
+ return (Tok.Modifiers & (1 << uint32_t(HighlightingModifier::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.
+// Sometimes we get multiple tokens at the same location:
+//
+// - findExplicitReferences() returns a heuristic result for a dependent name
+// (e.g. Method) and CollectExtraHighlighting returning a fallback dependent
+// highlighting (e.g. Unknown+Dependent).
+// - macro arguments are expanded multiple times and have
diff erent roles
+// - broken code recovery produces several AST nodes at the same location
+//
+// We should either resolve these to a single token, or drop them all.
+// Our heuristics are:
+//
+// - token kinds that come with "dependent-name" modifiers are less reliable
+// (these tend to be vague, like Type or Unknown)
+// - if we have multiple equally reliable kinds, drop token rather than guess
+// - take the union of modifiers from all tokens
+//
+// In particular, heuristically resolved dependent names get their heuristic
+// kind, plus the dependent modifier.
llvm::Optional<HighlightingToken>
resolveConflict(ArrayRef<HighlightingToken> Tokens) {
if (Tokens.size() == 1)
@@ -236,11 +246,13 @@ resolveConflict(ArrayRef<HighlightingToken> Tokens) {
if (Tokens.size() != 2)
return llvm::None;
- unsigned Priority1 = evaluateHighlightPriority(Tokens[0].Kind);
- unsigned Priority2 = evaluateHighlightPriority(Tokens[1].Kind);
- if (Priority1 == Priority2)
+ unsigned Priority1 = evaluateHighlightPriority(Tokens[0]);
+ unsigned Priority2 = evaluateHighlightPriority(Tokens[1]);
+ if (Priority1 == Priority2 && Tokens[0].Kind != Tokens[1].Kind)
return llvm::None;
- return Priority1 > Priority2 ? Tokens[0] : Tokens[1];
+ auto Result = Priority1 > Priority2 ? Tokens[0] : Tokens[1];
+ Result.Modifiers = Tokens[0].Modifiers | Tokens[1].Modifiers;
+ return Result;
}
/// Consumes source locations and maps them to text ranges for highlightings.
@@ -430,7 +442,8 @@ class CollectExtraHighlightings
bool VisitOverloadExpr(OverloadExpr *E) {
if (!E->decls().empty())
return true; // handled by findExplicitReferences.
- auto &Tok = H.addToken(E->getNameLoc(), HighlightingKind::DependentName);
+ auto &Tok = H.addToken(E->getNameLoc(), HighlightingKind::Unknown)
+ .addModifier(HighlightingModifier::DependentName);
if (llvm::isa<UnresolvedMemberExpr>(E))
Tok.addModifier(HighlightingModifier::ClassScope);
// other case is UnresolvedLookupExpr, scope is unknown.
@@ -438,38 +451,58 @@ class CollectExtraHighlightings
}
bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
- H.addToken(E->getMemberNameInfo().getLoc(), HighlightingKind::DependentName)
+ H.addToken(E->getMemberNameInfo().getLoc(), HighlightingKind::Unknown)
+ .addModifier(HighlightingModifier::DependentName)
.addModifier(HighlightingModifier::ClassScope);
return true;
}
bool VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) {
- H.addToken(E->getNameInfo().getLoc(), HighlightingKind::DependentName)
+ H.addToken(E->getNameInfo().getLoc(), HighlightingKind::Unknown)
+ .addModifier(HighlightingModifier::DependentName)
.addModifier(HighlightingModifier::ClassScope);
return true;
}
bool VisitDependentNameTypeLoc(DependentNameTypeLoc L) {
- H.addToken(L.getNameLoc(), HighlightingKind::DependentType)
+ H.addToken(L.getNameLoc(), HighlightingKind::Type)
+ .addModifier(HighlightingModifier::DependentName)
.addModifier(HighlightingModifier::ClassScope);
return true;
}
bool VisitDependentTemplateSpecializationTypeLoc(
DependentTemplateSpecializationTypeLoc L) {
- H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType)
+ H.addToken(L.getTemplateNameLoc(), HighlightingKind::Type)
+ .addModifier(HighlightingModifier::DependentName)
.addModifier(HighlightingModifier::ClassScope);
return true;
}
bool TraverseTemplateArgumentLoc(TemplateArgumentLoc L) {
- switch (L.getArgument().getKind()) {
- case TemplateArgument::Template:
- case TemplateArgument::TemplateExpansion:
- // FIXME: this isn't *always* a dependent template name.
- H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+ // Handle template template arguments only (other arguments are handled by
+ // their Expr, TypeLoc etc values).
+ if (L.getArgument().getKind() != TemplateArgument::Template &&
+ L.getArgument().getKind() != TemplateArgument::TemplateExpansion)
+ return RecursiveASTVisitor::TraverseTemplateArgumentLoc(L);
+
+ TemplateName N = L.getArgument().getAsTemplateOrTemplatePattern();
+ switch (N.getKind()) {
+ case TemplateName::OverloadedTemplate:
+ // Template template params must always be class templates.
+ // Don't bother to try to work out the scope here.
+ H.addToken(L.getTemplateNameLoc(), HighlightingKind::Class);
+ break;
+ case TemplateName::DependentTemplate:
+ case TemplateName::AssumedTemplate:
+ H.addToken(L.getTemplateNameLoc(), HighlightingKind::Class)
+ .addModifier(HighlightingModifier::DependentName);
break;
- default:
+ case TemplateName::Template:
+ case TemplateName::QualifiedTemplate:
+ case TemplateName::SubstTemplateTemplateParm:
+ case TemplateName::SubstTemplateTemplateParmPack:
+ // Names that could be resolved to a TemplateDecl are handled elsewhere.
break;
}
return RecursiveASTVisitor::TraverseTemplateArgumentLoc(L);
@@ -483,7 +516,8 @@ class CollectExtraHighlightings
bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc Q) {
if (NestedNameSpecifier *NNS = Q.getNestedNameSpecifier()) {
if (NNS->getKind() == NestedNameSpecifier::Identifier)
- H.addToken(Q.getLocalBeginLoc(), HighlightingKind::DependentType)
+ H.addToken(Q.getLocalBeginLoc(), HighlightingKind::Type)
+ .addModifier(HighlightingModifier::DependentName)
.addModifier(HighlightingModifier::ClassScope);
}
return RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(Q);
@@ -594,10 +628,10 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K) {
return OS << "EnumConstant";
case HighlightingKind::Typedef:
return OS << "Typedef";
- case HighlightingKind::DependentType:
- return OS << "DependentType";
- case HighlightingKind::DependentName:
- return OS << "DependentName";
+ case HighlightingKind::Type:
+ return OS << "Type";
+ case HighlightingKind::Unknown:
+ return OS << "Unknown";
case HighlightingKind::Namespace:
return OS << "Namespace";
case HighlightingKind::TemplateParameter:
@@ -747,10 +781,10 @@ llvm::StringRef toSemanticTokenType(HighlightingKind Kind) {
case HighlightingKind::EnumConstant:
return "enumMember";
case HighlightingKind::Typedef:
+ case HighlightingKind::Type:
return "type";
- case HighlightingKind::DependentType:
- case HighlightingKind::DependentName:
- return "dependent"; // nonstandard
+ case HighlightingKind::Unknown:
+ return "unknown"; // nonstandard
case HighlightingKind::Namespace:
return "namespace";
case HighlightingKind::TemplateParameter:
@@ -781,6 +815,8 @@ llvm::StringRef toSemanticTokenModifier(HighlightingModifier Modifier) {
return "deduced"; // nonstandard
case HighlightingModifier::Abstract:
return "abstract";
+ case HighlightingModifier::DependentName:
+ return "dependentName"; // nonstandard
case HighlightingModifier::FunctionScope:
return "functionScope"; // nonstandard
case HighlightingModifier::ClassScope:
@@ -850,9 +886,13 @@ llvm::StringRef toTextMateScope(HighlightingKind Kind) {
return "variable.other.enummember.cpp";
case HighlightingKind::Typedef:
return "entity.name.type.typedef.cpp";
- case HighlightingKind::DependentType:
+ case HighlightingKind::Type:
+ // Fragile: all paths emitting `Type` are dependent names for now.
+ // But toTextMateScope is going away soon.
return "entity.name.type.dependent.cpp";
- case HighlightingKind::DependentName:
+ case HighlightingKind::Unknown:
+ // Fragile: all paths emitting `Unknown` are dependent names for now.
+ // But toTextMateScope is going away soon.
return "entity.name.other.dependent.cpp";
case HighlightingKind::Namespace:
return "entity.name.namespace.cpp";
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h
index e88835385399..a34806d4a822 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.h
+++ b/clang-tools-extra/clangd/SemanticHighlighting.h
@@ -50,8 +50,8 @@ enum class HighlightingKind {
Enum,
EnumConstant,
Typedef,
- DependentType,
- DependentName,
+ Type,
+ Unknown,
Namespace,
TemplateParameter,
Concept,
@@ -75,6 +75,7 @@ enum class HighlightingModifier {
Readonly,
Static,
Abstract,
+ DependentName,
FunctionScope,
ClassScope,
diff --git a/clang-tools-extra/clangd/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test
index 605584187c44..4b40b17b2d64 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -87,7 +87,8 @@
# CHECK-NEXT: "deduced",
# CHECK-NEXT: "readonly",
# CHECK-NEXT: "static",
-# CHECK-NEXT: "abstract"
+# CHECK-NEXT: "abstract",
+# CHECK-NEXT: "dependentName",
# CHECK-NEXT: "functionScope",
# CHECK-NEXT: "classScope",
# CHECK-NEXT: "fileScope",
diff --git a/clang-tools-extra/clangd/test/semantic-tokens.test b/clang-tools-extra/clangd/test/semantic-tokens.test
index 50897c3c1208..dc79c79b6e76 100644
--- a/clang-tools-extra/clangd/test/semantic-tokens.test
+++ b/clang-tools-extra/clangd/test/semantic-tokens.test
@@ -23,7 +23,7 @@
# CHECK-NEXT: 4,
# CHECK-NEXT: 1,
# CHECK-NEXT: 0,
-# CHECK-NEXT: 513
+# CHECK-NEXT: 1025
# CHECK-NEXT: ],
# CHECK-NEXT: "resultId": "1"
# CHECK-NEXT: }
@@ -49,7 +49,7 @@
# CHECK-NEXT: 4,
# CHECK-NEXT: 1,
# CHECK-NEXT: 0,
-# CHECK-NEXT: 513
+# CHECK-NEXT: 1025
# CHECK-NEXT: ],
# Inserted at position 1
# CHECK-NEXT: "deleteCount": 0,
@@ -72,12 +72,12 @@
# CHECK-NEXT: 4,
# CHECK-NEXT: 1,
# CHECK-NEXT: 0,
-# CHECK-NEXT: 513,
+# CHECK-NEXT: 1025,
# CHECK-NEXT: 1,
# CHECK-NEXT: 4,
# CHECK-NEXT: 1,
# CHECK-NEXT: 0,
-# CHECK-NEXT: 513
+# CHECK-NEXT: 1025
# CHECK-NEXT: ],
# CHECK-NEXT: "resultId": "3"
# CHECK-NEXT: }
diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index f2afd5634681..577b01d3eae8 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -58,8 +58,8 @@ std::vector<HighlightingToken> getExpectedTokens(Annotations &Test) {
{HighlightingKind::Method, "Method"},
{HighlightingKind::StaticMethod, "StaticMethod"},
{HighlightingKind::Typedef, "Typedef"},
- {HighlightingKind::DependentType, "DependentType"},
- {HighlightingKind::DependentName, "DependentName"},
+ {HighlightingKind::Type, "Type"},
+ {HighlightingKind::Unknown, "Unknown"},
{HighlightingKind::TemplateParameter, "TemplateParameter"},
{HighlightingKind::Concept, "Concept"},
{HighlightingKind::Primitive, "Primitive"},
@@ -208,7 +208,7 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
}
template<typename $TemplateParameter_decl[[T]]>
struct $Class_decl[[C]] : $Namespace[[abc]]::$Class[[A]]<$TemplateParameter[[T]]> {
- typename $TemplateParameter[[T]]::$DependentType[[A]]* $Field_decl[[D]];
+ typename $TemplateParameter[[T]]::$Type_dependentName[[A]]* $Field_decl[[D]];
};
$Namespace[[abc]]::$Class[[A]]<int> $Variable_decl[[AA]];
typedef $Namespace[[abc]]::$Class[[A]]<int> $Class_decl[[AAA]];
@@ -342,11 +342,8 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
R"cpp(
template <class $TemplateParameter_decl[[T]]>
struct $Class_decl[[Tmpl]] {$TemplateParameter[[T]] $Field_decl[[x]] = 0;};
- // FIXME: highlights dropped due to conflicts.
- // explicitReferenceTargets reports ClassTemplateSpecializationDecl twice
- // at its location, calling it a declaration/non-declaration once each.
- extern template struct Tmpl<float>;
- template struct Tmpl<double>;
+ extern template struct $Class_decl[[Tmpl]]<float>;
+ template struct $Class_decl[[Tmpl]]<double>;
)cpp",
// This test is to guard against highlightings disappearing when using
// conversion operators as their behaviour in the clang AST
diff er from
@@ -569,7 +566,7 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
template <class $TemplateParameter_decl[[T]]>
void $Function_decl[[foo]]($TemplateParameter[[T]] $Parameter_decl[[P]]) {
$Function[[phase1]]($Parameter[[P]]);
- $DependentName[[phase2]]($Parameter[[P]]);
+ $Unknown_dependentName[[phase2]]($Parameter[[P]]);
}
)cpp",
R"cpp(
@@ -598,20 +595,20 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
)cpp",
R"cpp(
template <class $TemplateParameter_decl[[T]]>
- void $Function_decl[[foo]](typename $TemplateParameter[[T]]::$DependentType[[Type]]
- = $TemplateParameter[[T]]::$DependentName[[val]]);
+ void $Function_decl[[foo]](typename $TemplateParameter[[T]]::$Type_dependentName[[Type]]
+ = $TemplateParameter[[T]]::$Unknown_dependentName[[val]]);
)cpp",
R"cpp(
template <class $TemplateParameter_decl[[T]]>
void $Function_decl[[foo]]($TemplateParameter[[T]] $Parameter_decl[[P]]) {
- $Parameter[[P]].$DependentName[[Field]];
+ $Parameter[[P]].$Unknown_dependentName[[Field]];
}
)cpp",
R"cpp(
template <class $TemplateParameter_decl[[T]]>
class $Class_decl[[A]] {
int $Method_decl[[foo]]() {
- return $TemplateParameter[[T]]::$DependentName[[Field]];
+ return $TemplateParameter[[T]]::$Unknown_dependentName[[Field]];
}
};
)cpp",
@@ -674,15 +671,15 @@ sizeof...($TemplateParameter[[Elements]]);
template <typename $TemplateParameter_decl[[T]]>
struct $Class_decl[[Waldo]] {
using $Typedef_decl[[Location1]] = typename $TemplateParameter[[T]]
- ::$DependentType[[Resolver]]::$DependentType[[Location]];
+ ::$Type_dependentName[[Resolver]]::$Type_dependentName[[Location]];
using $Typedef_decl[[Location2]] = typename $TemplateParameter[[T]]
- ::template $DependentType[[Resolver]]<$TemplateParameter[[T]]>
- ::$DependentType[[Location]];
+ ::template $Type_dependentName[[Resolver]]<$TemplateParameter[[T]]>
+ ::$Type_dependentName[[Location]];
using $Typedef_decl[[Location3]] = typename $TemplateParameter[[T]]
- ::$DependentType[[Resolver]]
- ::template $DependentType[[Location]]<$TemplateParameter[[T]]>;
+ ::$Type_dependentName[[Resolver]]
+ ::template $Type_dependentName[[Location]]<$TemplateParameter[[T]]>;
static const int $StaticField_decl_readonly_static[[Value]] = $TemplateParameter[[T]]
- ::$DependentType[[Resolver]]::$DependentName[[Value]];
+ ::$Type_dependentName[[Resolver]]::$Unknown_dependentName[[Value]];
};
)cpp",
// Dependent name with heuristic target
@@ -691,11 +688,11 @@ sizeof...($TemplateParameter[[Elements]]);
struct $Class_decl[[Foo]] {
int $Field_decl[[Waldo]];
void $Method_decl[[bar]]() {
- $Class[[Foo]]().$Field[[Waldo]];
+ $Class[[Foo]]().$Field_dependentName[[Waldo]];
}
template <typename $TemplateParameter_decl[[U]]>
void $Method_decl[[bar1]]() {
- $Class[[Foo]]<$TemplateParameter[[U]]>().$Field[[Waldo]];
+ $Class[[Foo]]<$TemplateParameter[[U]]>().$Field_dependentName[[Waldo]];
}
};
)cpp",
@@ -704,12 +701,12 @@ sizeof...($TemplateParameter[[Elements]]);
template <typename $TemplateParameter_decl[[T]]>
concept $Concept_decl[[Fooable]] =
requires($TemplateParameter[[T]] $Parameter_decl[[F]]) {
- $Parameter[[F]].$DependentName[[foo]]();
+ $Parameter[[F]].$Unknown_dependentName[[foo]]();
};
template <typename $TemplateParameter_decl[[T]]>
requires $Concept[[Fooable]]<$TemplateParameter[[T]]>
void $Function_decl[[bar]]($TemplateParameter[[T]] $Parameter_decl[[F]]) {
- $Parameter[[F]].$DependentName[[foo]]();
+ $Parameter[[F]].$Unknown_dependentName[[foo]]();
}
)cpp",
// Dependent template name
@@ -717,7 +714,7 @@ sizeof...($TemplateParameter[[Elements]]);
template <template <typename> class> struct $Class_decl[[A]] {};
template <typename $TemplateParameter_decl[[T]]>
using $Typedef_decl[[W]] = $Class[[A]]<
- $TemplateParameter[[T]]::template $DependentType[[Waldo]]
+ $TemplateParameter[[T]]::template $Class_dependentName[[Waldo]]
>;
)cpp",
R"cpp(
@@ -800,7 +797,7 @@ TEST(SemanticHighlighting, ScopeModifiers) {
// No useful scope for template parameters of variable templates.
template <typename $TemplateParameter[[A]]>
unsigned $Variable_globalScope[[X]] =
- $TemplateParameter[[A]]::$DependentName_classScope[[x]];
+ $TemplateParameter[[A]]::$Unknown_classScope[[x]];
)cpp",
R"cpp(
#define $Macro_globalScope[[X]] 1
More information about the cfe-commits
mailing list