sammccall created this revision. sammccall added a reviewer: nridge. Herald added subscribers: usaxena95, kadircet, arphaman. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang.
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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D95706 Files: clang-tools-extra/clangd/SemanticHighlighting.cpp clang-tools-extra/clangd/SemanticHighlighting.h clang-tools-extra/clangd/test/initialize-params.test 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 @@ -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"}, @@ -198,7 +198,7 @@ } 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]]; @@ -332,11 +332,8 @@ 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 differ from @@ -559,7 +556,7 @@ 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( @@ -588,20 +585,20 @@ )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", @@ -664,15 +661,15 @@ 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 @@ -681,11 +678,11 @@ 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", @@ -694,12 +691,12 @@ 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 @@ -707,7 +704,7 @@ 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( Index: clang-tools-extra/clangd/test/initialize-params.test =================================================================== --- clang-tools-extra/clangd/test/initialize-params.test +++ 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: ], # CHECK-NEXT: "tokenTypes": [ # CHECK-NEXT: "variable", Index: clang-tools-extra/clangd/SemanticHighlighting.h =================================================================== --- clang-tools-extra/clangd/SemanticHighlighting.h +++ clang-tools-extra/clangd/SemanticHighlighting.h @@ -50,8 +50,8 @@ Enum, EnumConstant, Typedef, - DependentType, - DependentName, + Type, + Unknown, Namespace, TemplateParameter, Concept, @@ -75,8 +75,9 @@ Readonly, Static, Abstract, + DependentName, - LastModifier = Abstract + LastModifier = DependentName }; static_assert(static_cast<unsigned>(HighlightingModifier::LastModifier) < 32, "Increase width of modifiers bitfield!"); Index: clang-tools-extra/clangd/SemanticHighlighting.cpp =================================================================== --- clang-tools-extra/clangd/SemanticHighlighting.cpp +++ clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -209,10 +209,9 @@ 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; } @@ -232,11 +231,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. @@ -380,39 +381,59 @@ bool VisitOverloadExpr(OverloadExpr *E) { if (!E->decls().empty()) return true; // handled by findExplicitReferences. - H.addToken(E->getNameLoc(), HighlightingKind::DependentName); + H.addToken(E->getNameLoc(), HighlightingKind::Unknown) + .addModifier(HighlightingModifier::DependentName); return true; } bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) { - H.addToken(E->getMemberNameInfo().getLoc(), - HighlightingKind::DependentName); + H.addToken(E->getMemberNameInfo().getLoc(), HighlightingKind::Unknown) + .addModifier(HighlightingModifier::DependentName); return true; } bool VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) { - H.addToken(E->getNameInfo().getLoc(), HighlightingKind::DependentName); + H.addToken(E->getNameInfo().getLoc(), HighlightingKind::Unknown) + .addModifier(HighlightingModifier::DependentName); return true; } bool VisitDependentNameTypeLoc(DependentNameTypeLoc L) { - H.addToken(L.getNameLoc(), HighlightingKind::DependentType); + H.addToken(L.getNameLoc(), HighlightingKind::Type) + .addModifier(HighlightingModifier::DependentName); return true; } bool VisitDependentTemplateSpecializationTypeLoc( DependentTemplateSpecializationTypeLoc L) { - H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType); + H.addToken(L.getTemplateNameLoc(), HighlightingKind::Type) + .addModifier(HighlightingModifier::DependentName); return true; } bool TraverseTemplateArgumentLoc(TemplateArgumentLoc L) { - switch (L.getArgument().getKind()) { - case TemplateArgument::Template: - case TemplateArgument::TemplateExpansion: - 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. + 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); @@ -426,7 +447,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); } return RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(Q); } @@ -536,10 +558,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: @@ -689,10 +711,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: @@ -723,6 +745,8 @@ return "deduced"; // nonstandard case HighlightingModifier::Abstract: return "abstract"; + case HighlightingModifier::DependentName: + return "dependentName"; } } @@ -783,9 +807,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";
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits