[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