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

Kirill Bobyrev via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 21 01:25:57 PDT 2016


omtcyfz 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).

If everyone seems to be in favor of such changes, I'm OK with it, but in general I think it makes things more complicated and I'm not sure if it's necessary at the moment; I expressed my ideas about it in comments to the other patch. But if that's what the common use-case is... So, //TL;DR// I personally don't see why one would want to rename multiple things at once while we still can't rename a single symbol correctly in too many cases...

P.S. it seems logical to me to support `-offset` option in `-rename-all`, too. And introducing `-rename-all` without actually supporting multiple renaming actions "at once" seems weird to me, too.


================
Comment at: clang-rename/tool/ClangRename.cpp:226
@@ +225,3 @@
+  if (argc > 1) {
+    typedef int (*MainFunction)(int, const char *[]);
+    MainFunction Func = StringSwitch<MainFunction>(argv[1])
----------------
use `std::function` here?


https://reviews.llvm.org/D21814





More information about the cfe-commits mailing list