sammccall added inline comments.
Herald added a subscriber: cfe-commits.
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:309-313
+ // FIXME: visiting this here allows us to hover on UIColor in
+ // `UIColor.blackColor` but then `blackColor` no longer refers to the
+ // method.
+ // if (OPRE->isClassReceiver())
+ // Outer.add(OPRE->getClassReceiver(), Flags);
----------------
dgoldman wrote:
> Is there a good way to handle this case where the expression can refer to
> multiple decls?
TL;DR: yes, fix the AST :-)
The design assumption is that an AST node (specifically: the tokens owned by
exactly that AST node and not a descendant of it) refers to one thing. I don't
see an easy way to hack around this.
Expressions like this are supposed to be split into multiple AST nodes. But
ObjC seems to be missing AST nodes in a bunch of places (Protocol lists is
another).
e.g. in C++ equivalent DeclRefExpr `UIColor::blackColor` there'd be both a
NestedNameSpecifier for `UIColor::` and a TagTypeLoc for `UIColor`. And objc
`[Test foo]` has an `ObjCInterfaceTypeLoc` for `Test`.
---
I'm not sure this is the right place for this comment, the problem isn't what
VisitObjCPropertyRefExpr is doing, the issue is that the caller is passing the
wrong kind of node to findTarget() because there's no right one.
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:768
void VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *E) {
+ if (E->isClassReceiver()) {
+ Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
----------------
comment here:
// There's no contained TypeLoc node for the receiver type.
================
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:1040
- {
- // Simple expressions.
- {R"cpp(
----------------
As in the other patch, we should keep these large unrelated reformattings out
of substantive patches.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99975/new/
https://reviews.llvm.org/D99975
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
