[clang-tools-extra] 7d2fba8 - [clangd] ObjC fixes for semantic highlighting and xref highlights
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 3 11:16:16 PST 2021
Author: Sam McCall
Date: 2021-03-03T20:16:08+01:00
New Revision: 7d2fba8ddb90cf018d9cfc852b68e4584b15678e
URL: https://github.com/llvm/llvm-project/commit/7d2fba8ddb90cf018d9cfc852b68e4584b15678e
DIFF: https://github.com/llvm/llvm-project/commit/7d2fba8ddb90cf018d9cfc852b68e4584b15678e.diff
LOG: [clangd] ObjC fixes for semantic highlighting and xref highlights
- highlight references to protocols in class/protocol/extension decls
- support multi-token selector highlights in semantic + xref highlights
(method calls and declarations only)
- In `@interface I(C)`, I now references the interface and C the category
- highlight uses of interfaces as types
- added semantic highlightings of protocol names (as "interface") and
category names (as "namespace").
These are both standard kinds, maybe "extension" will be standardized...
- highlight `auto` as "class" when it resolves to an ObjC pointer
- don't highlight `self` as a variable even though the AST models it as one
Not fixed: uses of protocols in type names (needs some refactoring of
unrelated code first)
Differential Revision: https://reviews.llvm.org/D97617
Added:
Modified:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp
index 64053797867d..16433151d902 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -12,6 +12,7 @@
#include "support/Logger.h"
#include "clang/AST/ASTTypeTraits.h"
#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/DeclVisitor.h"
@@ -633,6 +634,61 @@ llvm::SmallVector<ReferenceLoc> refInDecl(const Decl *D,
/*IsDecl=*/false,
{DG->getDeducedTemplate()}});
}
+
+ void VisitObjCMethodDecl(const ObjCMethodDecl *OMD) {
+ // The name may have several tokens, we can only report the first.
+ Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+ OMD->getSelectorStartLoc(),
+ /*IsDecl=*/true,
+ {OMD}});
+ }
+
+ void visitProtocolList(
+ llvm::iterator_range<ObjCProtocolList::iterator> Protocols,
+ llvm::iterator_range<const SourceLocation *> Locations) {
+ for (const auto &P : llvm::zip(Protocols, Locations)) {
+ Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+ std::get<1>(P),
+ /*IsDecl=*/false,
+ {std::get<0>(P)}});
+ }
+ }
+
+ void VisitObjCInterfaceDecl(const ObjCInterfaceDecl *OID) {
+ if (OID->isThisDeclarationADefinition())
+ visitProtocolList(OID->protocols(), OID->protocol_locs());
+ Base::VisitObjCInterfaceDecl(OID); // Visit the interface's name.
+ }
+
+ void VisitObjCCategoryDecl(const ObjCCategoryDecl *OCD) {
+ visitProtocolList(OCD->protocols(), OCD->protocol_locs());
+ // getLocation is the extended class's location, not the category's.
+ Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+ OCD->getLocation(),
+ /*IsDecl=*/false,
+ {OCD->getClassInterface()}});
+ Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+ OCD->getCategoryNameLoc(),
+ /*IsDecl=*/true,
+ {OCD}});
+ }
+
+ void VisitObjCCategoryImplDecl(const ObjCCategoryImplDecl *OCID) {
+ Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+ OCID->getLocation(),
+ /*IsDecl=*/false,
+ {OCID->getClassInterface()}});
+ Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+ OCID->getCategoryNameLoc(),
+ /*IsDecl=*/true,
+ {OCID->getCategoryDecl()}});
+ }
+
+ void VisitObjCProtocolDecl(const ObjCProtocolDecl *OPD) {
+ if (OPD->isThisDeclarationADefinition())
+ visitProtocolList(OPD->protocols(), OPD->protocol_locs());
+ Base::VisitObjCProtocolDecl(OPD); // Visit the protocol's name.
+ }
};
Visitor V{Resolver};
@@ -711,6 +767,14 @@ llvm::SmallVector<ReferenceLoc> refInStmt(const Stmt *S,
explicitReferenceTargets(DynTypedNode::create(*E), {}, Resolver)});
}
+ void VisitObjCMessageExpr(const ObjCMessageExpr *E) {
+ // The name may have several tokens, we can only report the first.
+ Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+ E->getSelectorStartLoc(),
+ /*IsDecl=*/false,
+ {E->getMethodDecl()}});
+ }
+
void VisitDesignatedInitExpr(const DesignatedInitExpr *DIE) {
for (const DesignatedInitExpr::Designator &D : DIE->designators()) {
if (!D.isFieldDesignator())
@@ -824,6 +888,16 @@ refInTypeLoc(TypeLoc L, const HeuristicResolver *Resolver) {
/*IsDecl=*/false,
{L.getTypedefNameDecl()}};
}
+
+ void VisitObjCInterfaceTypeLoc(ObjCInterfaceTypeLoc L) {
+ Ref = ReferenceLoc{NestedNameSpecifierLoc(),
+ L.getNameLoc(),
+ /*IsDecl=*/false,
+ {L.getIFaceDecl()}};
+ }
+
+ // FIXME: add references to protocols in ObjCObjectTypeLoc and maybe
+ // ObjCObjectPointerTypeLoc.
};
Visitor V{Resolver};
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index da8ee7e41ebd..9e24b92b0f5c 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -40,11 +40,27 @@ namespace {
/// Some names are not written in the source code and cannot be highlighted,
/// e.g. anonymous classes. This function detects those cases.
bool canHighlightName(DeclarationName Name) {
- if (Name.getNameKind() == DeclarationName::CXXConstructorName ||
- Name.getNameKind() == DeclarationName::CXXUsingDirective)
+ switch (Name.getNameKind()) {
+ case DeclarationName::Identifier: {
+ auto *II = Name.getAsIdentifierInfo();
+ return II && !II->getName().empty();
+ }
+ case DeclarationName::CXXConstructorName:
+ case DeclarationName::CXXDestructorName:
return true;
- auto *II = Name.getAsIdentifierInfo();
- return II && !II->getName().empty();
+ case DeclarationName::ObjCZeroArgSelector:
+ case DeclarationName::ObjCOneArgSelector:
+ case DeclarationName::ObjCMultiArgSelector:
+ // Multi-arg selectors need special handling, and we handle 0/1 arg
+ // selectors there too.
+ return false;
+ case DeclarationName::CXXConversionFunctionName:
+ case DeclarationName::CXXOperatorName:
+ case DeclarationName::CXXDeductionGuideName:
+ case DeclarationName::CXXLiteralOperatorName:
+ case DeclarationName::CXXUsingDirective:
+ return false;
+ }
}
llvm::Optional<HighlightingKind> kindForType(const Type *TP);
@@ -73,13 +89,20 @@ llvm::Optional<HighlightingKind> kindForDecl(const NamedDecl *D) {
return llvm::None;
return HighlightingKind::Class;
}
- if (isa<ClassTemplateDecl>(D) || isa<RecordDecl>(D) ||
- isa<CXXConstructorDecl>(D))
+ if (isa<ClassTemplateDecl, RecordDecl, CXXConstructorDecl, ObjCInterfaceDecl,
+ ObjCImplementationDecl>(D))
return HighlightingKind::Class;
+ if (isa<ObjCProtocolDecl>(D))
+ return HighlightingKind::Interface;
+ if (isa<ObjCCategoryDecl>(D))
+ return HighlightingKind::Namespace;
if (auto *MD = dyn_cast<CXXMethodDecl>(D))
return MD->isStatic() ? HighlightingKind::StaticMethod
: HighlightingKind::Method;
- if (isa<FieldDecl>(D))
+ if (auto *OMD = dyn_cast<ObjCMethodDecl>(D))
+ return OMD->isClassMethod() ? HighlightingKind::StaticMethod
+ : HighlightingKind::Method;
+ if (isa<FieldDecl, ObjCPropertyDecl>(D))
return HighlightingKind::Field;
if (isa<EnumDecl>(D))
return HighlightingKind::Enum;
@@ -87,11 +110,14 @@ llvm::Optional<HighlightingKind> kindForDecl(const NamedDecl *D) {
return HighlightingKind::EnumConstant;
if (isa<ParmVarDecl>(D))
return HighlightingKind::Parameter;
- if (auto *VD = dyn_cast<VarDecl>(D))
+ if (auto *VD = dyn_cast<VarDecl>(D)) {
+ if (isa<ImplicitParamDecl>(VD)) // e.g. ObjC Self
+ return llvm::None;
return VD->isStaticDataMember()
? HighlightingKind::StaticField
: VD->isLocalVarDecl() ? HighlightingKind::LocalVariable
: HighlightingKind::Variable;
+ }
if (const auto *BD = dyn_cast<BindingDecl>(D))
return BD->getDeclContext()->isFunctionOrMethod()
? HighlightingKind::LocalVariable
@@ -115,6 +141,8 @@ llvm::Optional<HighlightingKind> kindForType(const Type *TP) {
return HighlightingKind::Primitive;
if (auto *TD = dyn_cast<TemplateTypeParmType>(TP))
return kindForDecl(TD->getDecl());
+ if (isa<ObjCObjectPointerType>(TP))
+ return HighlightingKind::Class;
if (auto *TD = TP->getAsTagDecl())
return kindForDecl(TD);
return llvm::None;
@@ -439,6 +467,36 @@ class CollectExtraHighlightings
return true;
}
+ // We handle objective-C selectors specially, because one reference can
+ // cover several non-contiguous tokens.
+ void highlightObjCSelector(const ArrayRef<SourceLocation> &Locs, bool Decl,
+ bool Class) {
+ HighlightingKind Kind =
+ Class ? HighlightingKind::StaticMethod : HighlightingKind::Method;
+ for (SourceLocation Part : Locs) {
+ auto &Tok =
+ H.addToken(Part, Kind).addModifier(HighlightingModifier::ClassScope);
+ if (Decl)
+ Tok.addModifier(HighlightingModifier::Declaration);
+ if (Class)
+ Tok.addModifier(HighlightingModifier::Static);
+ }
+ }
+
+ bool VisitObjCMethodDecl(ObjCMethodDecl *OMD) {
+ llvm::SmallVector<SourceLocation> Locs;
+ OMD->getSelectorLocs(Locs);
+ highlightObjCSelector(Locs, /*Decl=*/true, OMD->isClassMethod());
+ return true;
+ }
+
+ bool VisitObjCMessageExpr(ObjCMessageExpr *OME) {
+ llvm::SmallVector<SourceLocation> Locs;
+ OME->getSelectorLocs(Locs);
+ highlightObjCSelector(Locs, /*Decl=*/false, OME->isClassMessage());
+ return true;
+ }
+
bool VisitOverloadExpr(OverloadExpr *E) {
if (!E->decls().empty())
return true; // handled by findExplicitReferences.
@@ -602,6 +660,8 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingKind K) {
return OS << "StaticField";
case HighlightingKind::Class:
return OS << "Class";
+ case HighlightingKind::Interface:
+ return OS << "Interface";
case HighlightingKind::Enum:
return OS << "Enum";
case HighlightingKind::EnumConstant:
@@ -695,6 +755,8 @@ llvm::StringRef toSemanticTokenType(HighlightingKind Kind) {
return "property";
case HighlightingKind::Class:
return "class";
+ case HighlightingKind::Interface:
+ return "interface";
case HighlightingKind::Enum:
return "enum";
case HighlightingKind::EnumConstant:
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h
index e4c36ab5261e..40b315c679a4 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.h
+++ b/clang-tools-extra/clangd/SemanticHighlighting.h
@@ -37,6 +37,7 @@ enum class HighlightingKind {
Field,
StaticField,
Class,
+ Interface,
Enum,
EnumConstant,
Typedef,
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index ff3b8d73b64e..1f821f8edd1e 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -882,8 +882,8 @@ class ReferenceFinder : public index::IndexDataConsumer {
};
ReferenceFinder(const ParsedAST &AST,
- const llvm::DenseSet<SymbolID> &TargetIDs)
- : AST(AST), TargetIDs(TargetIDs) {}
+ const llvm::DenseSet<SymbolID> &TargetIDs, bool PerToken)
+ : PerToken(PerToken), AST(AST), TargetIDs(TargetIDs) {}
std::vector<Reference> take() && {
llvm::sort(References, [](const Reference &L, const Reference &R) {
@@ -915,21 +915,43 @@ class ReferenceFinder : public index::IndexDataConsumer {
if (!TargetIDs.contains(ID))
return true;
const auto &TB = AST.getTokens();
- Loc = SM.getFileLoc(Loc);
- if (const auto *Tok = TB.spelledTokenAt(Loc))
- References.push_back({*Tok, Roles, ID});
+
+ llvm::SmallVector<SourceLocation, 1> Locs;
+ if (PerToken) {
+ // Check whether this is one of the few constructs where the reference
+ // can be split over several tokens.
+ if (auto *OME = llvm::dyn_cast_or_null<ObjCMessageExpr>(ASTNode.OrigE)) {
+ OME->getSelectorLocs(Locs);
+ } else if (auto *OMD =
+ llvm::dyn_cast_or_null<ObjCMethodDecl>(ASTNode.OrigD)) {
+ OMD->getSelectorLocs(Locs);
+ }
+ // Sanity check: we expect the *first* token to match the reported loc.
+ // Otherwise, maybe it was e.g. some other kind of reference to a Decl.
+ if (!Locs.empty() && Locs.front() != Loc)
+ Locs.clear(); // First token doesn't match, assume our guess was wrong.
+ }
+ if (Locs.empty())
+ Locs.push_back(Loc);
+
+ for (SourceLocation L : Locs) {
+ L = SM.getFileLoc(L);
+ if (const auto *Tok = TB.spelledTokenAt(L))
+ References.push_back({*Tok, Roles, ID});
+ }
return true;
}
private:
+ bool PerToken; // If true, report 3 references for split ObjC selector names.
std::vector<Reference> References;
const ParsedAST &AST;
const llvm::DenseSet<SymbolID> &TargetIDs;
};
std::vector<ReferenceFinder::Reference>
-findRefs(const llvm::DenseSet<SymbolID> &IDs, ParsedAST &AST) {
- ReferenceFinder RefFinder(AST, IDs);
+findRefs(const llvm::DenseSet<SymbolID> &IDs, ParsedAST &AST, bool PerToken) {
+ ReferenceFinder RefFinder(AST, IDs, PerToken);
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -1224,7 +1246,7 @@ std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
for (const NamedDecl *ND : Decls)
if (auto ID = getSymbolID(ND))
Targets.insert(ID);
- for (const auto &Ref : findRefs(Targets, AST))
+ for (const auto &Ref : findRefs(Targets, AST, /*PerToken=*/true))
Result.push_back(toHighlight(Ref, SM));
return true;
}
@@ -1376,7 +1398,7 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
}
// We traverse the AST to find references in the main file.
- auto MainFileRefs = findRefs(Targets, AST);
+ auto MainFileRefs = findRefs(Targets, AST, /*PerToken=*/false);
// We may get multiple refs with the same location and
diff erent Roles, as
// cross-reference is only interested in locations, we deduplicate them
// by the location to avoid emitting duplicated locations.
diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index c9f035e7e971..7a25293bf2b9 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1593,6 +1593,35 @@ TEST_F(FindExplicitReferencesTest, All) {
"0: targets = {f}\n"
"1: targets = {I::x}\n"
"2: targets = {I::setY:}\n"},
+ {
+ // Objective-C: methods
+ R"cpp(
+ @interface I
+ -(void) a:(int)x b:(int)y;
+ @end
+ void foo(I *i) {
+ [$0^i $1^a:1 b:2];
+ }
+ )cpp",
+ "0: targets = {i}\n"
+ "1: targets = {I::a:b:}\n"
+ },
+ {
+ // Objective-C: protocols
+ R"cpp(
+ @interface I
+ @end
+ @protocol P
+ @end
+ void foo() {
+ // FIXME: should reference P
+ $0^I<P> *$1^x;
+ }
+ )cpp",
+ "0: targets = {I}\n"
+ "1: targets = {x}, decl\n"
+ },
+
// Designated initializers.
{R"cpp(
void foo() {
diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index eb848dddad20..d8dc3b061df5 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -75,6 +75,7 @@ void checkHighlightings(llvm::StringRef Code,
TU.Code = std::string(Test.code());
TU.ExtraArgs.push_back("-std=c++20");
+ TU.ExtraArgs.push_back("-xobjective-c++");
for (auto File : AdditionalFiles)
TU.AdditionalFiles.insert({File.first, std::string(File.second)});
@@ -645,6 +646,48 @@ sizeof...($TemplateParameter[[Elements]]);
R"cpp(
<:[deprecated]:> int $Variable_decl_deprecated[[x]];
)cpp",
+ R"cpp(
+ // ObjC: Classes and methods
+ @class $Class_decl[[Forward]];
+
+ @interface $Class_decl[[Foo]]
+ @end
+ @interface $Class_decl[[Bar]] : $Class[[Foo]]
+ -($Class[[id]]) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]];
+ +(void) $StaticMethod_decl_static[[explode]];
+ @end
+ @implementation $Class_decl[[Bar]]
+ -($Class[[id]]) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]] {
+ return self;
+ }
+ +(void) $StaticMethod_decl_static[[explode]] {}
+ @end
+
+ void $Function_decl[[m]]($Class[[Bar]] *$Parameter_decl[[b]]) {
+ [$Parameter[[b]] $Method[[x]]:1 $Method[[y]]:2];
+ [$Class[[Bar]] $StaticMethod_static[[explode]]];
+ }
+ )cpp",
+ R"cpp(
+ // ObjC: Protocols
+ @protocol $Interface_decl[[Protocol]]
+ @end
+ @protocol $Interface_decl[[Protocol2]] <$Interface[[Protocol]]>
+ @end
+ @interface $Class_decl[[Klass]] <$Interface[[Protocol]]>
+ @end
+ // FIXME: protocol list in ObjCObjectType should be highlighted.
+ id<Protocol> $Variable_decl[[x]];
+ )cpp",
+ R"cpp(
+ // ObjC: Categories
+ @interface $Class_decl[[Foo]]
+ @end
+ @interface $Class[[Foo]]($Namespace_decl[[Bar]])
+ @end
+ @implementation $Class[[Foo]]($Namespace_decl[[Bar]])
+ @end
+ )cpp",
};
for (const auto &TestCase : TestCases)
// Mask off scope modifiers to keep the tests manageable.
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 9d77e0fb291a..2fbf1f98db8a 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -115,10 +115,23 @@ TEST(HighlightsTest, All) {
f.[[^~]]Foo();
}
)cpp",
+ R"cpp(// ObjC methods with split selectors.
+ @interface Foo
+ +(void) [[x]]:(int)a [[y]]:(int)b;
+ @end
+ @implementation Foo
+ +(void) [[x]]:(int)a [[y]]:(int)b {}
+ @end
+ void go() {
+ [Foo [[x]]:2 [[^y]]:4];
+ }
+ )cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
- auto AST = TestTU::withCode(T.code()).build();
+ auto TU = TestTU::withCode(T.code());
+ TU.ExtraArgs.push_back("-xobjective-c++");
+ auto AST = TU.build();
EXPECT_THAT(findDocumentHighlights(AST, T.point()), HighlightsFrom(T))
<< Test;
}
More information about the cfe-commits
mailing list