This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by sammccall on Jan 10 2020, 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.Jan 10 2020, 1:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 10 2020, 1:40 AM
ilya-biryukov requested changes to this revision.Jan 10 2020, 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.Jan 10 2020, 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.Jan 10 2020, 7:38 AM

LGTM, thanks!

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

LGMT, thanks for fixing

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