This is an archive of the discontinued LLVM Phabricator instance.

[clang-rename] Add function unit tests.
ClosedPublic

Authored by hokein on Oct 13 2017, 6:57 AM.

Details

Summary

Also contain a fix:

  • Fix a false positive of renaming a using shadow function declaration.

Event Timeline

hokein created this revision.Oct 13 2017, 6:57 AM
This revision is now accepted and ready to land.Oct 13 2017, 8:14 AM
hokein updated this revision to Diff 119121.Oct 16 2017, 2:35 AM

Update, and add one unfixed testcase.

Sorry, I thought I have run all the tests. The previous version broke one test in RenameClass. We still need the workaround for the std::function<void(T)> special case. I added it back, and also added a corresponding test case for renaming function, but it is currently not supported and disabled, I think it is fine now, as it is a rare case IMO.

Please take a look again.

hokein edited the summary of this revision. (Show Details)Oct 16 2017, 2:39 AM
ioeric accepted this revision.Oct 16 2017, 2:48 AM

Still LGTM

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
469

The current behavior looks fine to me, but I'm not sure if this is a right FIXME? Basically, I think we want to be able to get the real DeclContext for the symbol reference here right?

hokein updated this revision to Diff 119128.Oct 16 2017, 3:22 AM
hokein marked an inline comment as done.

Remove the FIXME.

lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
469

I'm fine on removing it, as this is somewhat mentioned above. Done.

This revision was automatically updated to reflect the committed changes.