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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
