Page MenuHomePhabricator

[clangd] Fix targetDecl() on certain usage of ObjC properties.
ClosedPublic

Authored by sammccall on Fri, Jan 10, 1:40 AM.

Details

Summary

In particular there's a common chain:

OpaqueValueExpr->PseudoObjectExpr->ObjCPropertyRefExpr->ObjCPropertyDecl

and we weren't handling the first two edges

Diff Detail

Event Timeline

sammccall created this revision.Fri, Jan 10, 1:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Jan 10, 1:40 AM
ilya-biryukov requested changes to this revision.Fri, Jan 10, 1:44 AM

Could you also add a test (and possibly support this in the visitors) for findExplicitReferences?

This revision now requires changes to proceed.Fri, Jan 10, 1:44 AM

Unit tests: pass. 61742 tests passed, 0 failed and 780 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Could you also add a test (and possibly support this in the visitors) for findExplicitReferences?

I've written tests, but it doesn't work and it's not obvious to me yet how to make it work, so I'll do this in a separate patch.
(We're close to the release cut... as far as major features, this patch fixes go to def and hover. Extending to findExplicitRefs will fix rename. Xrefs works already via libindex)

Could you also add a test (and possibly support this in the visitors) for findExplicitReferences?

I've written tests, but it doesn't work and it's not obvious to me yet how to make it work, so I'll do this in a separate patch.

By which I mean, adding the same delegation to the RAV there doesn't work, and I'm not confident that jiggling it around until the test passes is correct here, so I want to understand the traversal behavior a bit better first (RAV has several special cases around OVE/POE)

kadircet accepted this revision.Fri, Jan 10, 7:38 AM

LGTM, thanks!

dgoldman accepted this revision.Fri, Jan 10, 8:37 AM

LGMT, thanks for fixing

This revision was not accepted when it landed; it landed in state Needs Revision.Fri, Jan 10, 9:08 AM
This revision was automatically updated to reflect the committed changes.