Page MenuHomePhabricator

clang-rename: add new -force option
ClosedPublic

Authored by vmiklos on Apr 23 2017, 12:33 PM.

Details

Summary
The use-case is when renaming a widely used name, like a lower-level
class in a codebase and clang-rename is simply invoked for each
translation unit based on the compile database. In this case it's not
interesting to show errors: not finding the symbol means there is
simply nothing to do.

Diff Detail

Repository
rL LLVM

Event Timeline

vmiklos created this revision.Apr 23 2017, 12:33 PM

Forgot to include one hunk to fix the build of the check-clang-tools target; I'll fix in a moment.

vmiklos updated this revision to Diff 96320.Apr 23 2017, 1:04 PM

Manuel, can you please review this one? Thanks.

klimek added inline comments.May 3 2017, 8:50 AM
clang-rename/USRFindingAction.cpp
186 ↗(On Diff #96320)

Why do we want to set ErrorOccurred in this case?

vmiklos added inline comments.May 3 2017, 12:18 PM
clang-rename/USRFindingAction.cpp
186 ↗(On Diff #96320)

Later in RenamingAction.cpp:48 we assume that the PrevNames vector is parallel to the NewNames one, but in the above case we return early, so that's not the case. An easy way to avoid violating that assumption was to keep setting the error, so we don't reach the problematic part of RenamingAction. (If we reach it, we'll crash.)

A perhaps more elegant way is to not set the flag, but instead check if the two vectors are parallel after checking for errors, I'll do that in a moment.

vmiklos updated this revision to Diff 97705.May 3 2017, 12:22 PM

Manuel, do you have any further comments, please? Thanks.

@klimek ping, do you have any further comments, please?

klimek accepted this revision.Jun 2 2017, 1:43 AM

lg; sorry for the delay in review

This revision is now accepted and ready to land.Jun 2 2017, 1:43 AM
This revision was automatically updated to reflect the committed changes.