This is an archive of the discontinued LLVM Phabricator instance.

[clangd] go-to-implementation on a base class jumps to all subclasses.
ClosedPublic

Authored by hokein on Dec 7 2020, 2:17 AM.

Diff Detail

Event Timeline

hokein created this revision.Dec 7 2020, 2:17 AM
hokein requested review of this revision.Dec 7 2020, 2:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 7 2020, 2:17 AM

How does this handle Dependent base classes (including CRTP). Can tests be added to demonstrate that behaviour if its supported.

clang-tools-extra/clangd/XRefs.cpp
1200

nit: Make this else if or add a continue statement to the the previous if. We know ND can't be a CXXRecordDecl its a CXXMethodDecl.
super-nit: Maybe call this CXXRD or just RD, D implies we only know its a Decl but nothing more.

clang-tools-extra/clangd/unittests/XRefsTests.cpp
1513

If you're changing this line, may as well change use StringRef here.

hokein updated this revision to Diff 314522.Jan 5 2021, 1:09 AM
hokein marked 2 inline comments as done.

address review comment.

hokein added a comment.Jan 5 2021, 1:09 AM

How does this handle Dependent base classes (including CRTP). Can tests be added to demonstrate that behaviour if its supported.

Thanks, this is a good point.

usaxena95 accepted this revision.Jan 8 2021, 2:30 AM

LGTM. Thanks!

This revision is now accepted and ready to land.Jan 8 2021, 2:30 AM