[PATCH] D21814: clang-rename: split existing options into two new subcommands

Miklos Vajna via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 21 05:08:32 PDT 2016


vmiklos added a comment.

> The patch looks fine to me (though I'm not sure if there are no new tests; if

>  they are interface changes should be applied).


`make check-clang-tools` + the patch at r276098 passes for me at least. But any
pending test should be trivial to adapt.

> P.S. it seems logical to me to support `-offset` option in `-rename-all`,

>  too.


OK, I've added that, with a testcase.

> And introducing `-rename-all` without actually supporting multiple

>  renaming actions "at once" seems weird to me, too.


OK, I've squashed the original diff into this one, with a testcase, which
addresses this.

A nice side-effect is that now the option parser takes care of checking if
-new-name is provided, no need to have explicit code for that in clang-rename.

> use std::function here?


Done, also changed the `typedef` to `using`.


https://reviews.llvm.org/D21814





More information about the cfe-commits mailing list