dgoldman added a comment.
In D97617#2592424 <https://reviews.llvm.org/D97617#2592424>, @sammccall wrote:
> (not really sure why this suddenly seemed important to me, but if you don't
> have semantic highlighting on, you're missing out!)
Thanks!
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:657
+ void VisitObjCInterfaceDecl(const ObjCInterfaceDecl *OID) {
+ if (OID->hasDefinition())
+ visitProtocolList(OID->protocols(), OID->protocol_locs());
----------------
Should this be isThisDeclarationADefinition()? Is it even needed (will the
protocol list just be empty otherwise)?
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:659
+ visitProtocolList(OID->protocols(), OID->protocol_locs());
+ Base::VisitObjCInterfaceDecl(OID); // Visit the interface itself.
+ }
----------------
Hmm, for what? might be better to say why/what it does
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:668
+ /*IsDecl=*/false,
+ {OCD->getClassInterface()}});
+ Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
----------------
IIRC I think it's possible that these could be NULL if the code was invalid, is
this NULL safe?
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:687-689
+ if (OPD->hasDefinition())
+ visitProtocolList(OPD->protocols(), OPD->protocol_locs());
+ Base::VisitObjCProtocolDecl(OPD); // Visit the protocol itself.
----------------
Same comments here as the interface code above
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:774
+ Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+ E->getSelectorStartLoc(),
+ /*IsDecl=*/false, Targets});
----------------
What is this location used for? I guess it's not possible to have multiple Locs
here like we have for a selector and you specifically handle for semantic
highlighting below?
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:927-930
+ // Possible it was something else referencing an ObjCMethodDecl.
+ // If the first token doesn't match, assume our guess was wrong.
+ if (!Locs.empty() && Locs.front() != Loc)
+ Locs.clear();
----------------
Hmm, is this intentional? Not sure what else could reference it besides maybe a
property?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97617/new/
https://reviews.llvm.org/D97617
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
