diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h --- a/clang-tools-extra/clangd/SemanticHighlighting.h +++ b/clang-tools-extra/clangd/SemanticHighlighting.h @@ -50,8 +50,8 @@ Enum, EnumConstant, Typedef, - DependentType, - DependentName, + Type, + Unknown, Namespace, TemplateParameter, Concept, @@ -75,6 +75,7 @@ Readonly, Static, Abstract, + DependentName, FunctionScope, ClassScope, diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -213,21 +213,31 @@ 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 different 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 different 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 resolveConflict(ArrayRef Tokens) { if (Tokens.size() == 1) @@ -236,11 +246,13 @@ 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 @@ 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(E)) Tok.addModifier(HighlightingModifier::ClassScope); // other case is UnresolvedLookupExpr, scope is unknown. @@ -438,38 +451,58 @@ } 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test --- 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 --- 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 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -58,8 +58,8 @@ {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 @@ } template 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]] $Variable_decl[[AA]]; typedef $Namespace[[abc]]::$Class[[A]] $Class_decl[[AAA]]; @@ -342,11 +342,8 @@ R"cpp( template 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; - template struct Tmpl; + extern template struct $Class_decl[[Tmpl]]; + template struct $Class_decl[[Tmpl]]; )cpp", // This test is to guard against highlightings disappearing when using // conversion operators as their behaviour in the clang AST differ from @@ -569,7 +566,7 @@ template 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 @@ )cpp", R"cpp( template - 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 void $Function_decl[[foo]]($TemplateParameter[[T]] $Parameter_decl[[P]]) { - $Parameter[[P]].$DependentName[[Field]]; + $Parameter[[P]].$Unknown_dependentName[[Field]]; } )cpp", R"cpp( template class $Class_decl[[A]] { int $Method_decl[[foo]]() { - return $TemplateParameter[[T]]::$DependentName[[Field]]; + return $TemplateParameter[[T]]::$Unknown_dependentName[[Field]]; } }; )cpp", @@ -674,15 +671,15 @@ template 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 @@ struct $Class_decl[[Foo]] { int $Field_decl[[Waldo]]; void $Method_decl[[bar]]() { - $Class[[Foo]]().$Field[[Waldo]]; + $Class[[Foo]]().$Field_dependentName[[Waldo]]; } template void $Method_decl[[bar1]]() { - $Class[[Foo]]<$TemplateParameter[[U]]>().$Field[[Waldo]]; + $Class[[Foo]]<$TemplateParameter[[U]]>().$Field_dependentName[[Waldo]]; } }; )cpp", @@ -704,12 +701,12 @@ template concept $Concept_decl[[Fooable]] = requires($TemplateParameter[[T]] $Parameter_decl[[F]]) { - $Parameter[[F]].$DependentName[[foo]](); + $Parameter[[F]].$Unknown_dependentName[[foo]](); }; template 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 @@ template