[PATCH] D38989: [clang-rename] Rename enum.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:212
+      // Ignore the case where there is no prefix qualifer for the enum constant
+      // expression like `a = Green`.
+      if (!Expr->hasQualifier())
----------------
ioeric wrote:
> Why do we ignore this case? What if `Color` is moved to a different namespace? We would also need to qualify `Green` with a new namespace.
Thanks for pointing it out (we missed such test case before).

Yeah, this should be a FIXME, and added the case to the unittest.


================
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:228
+      // enum constant `Green` should be excluded).
+      //    ns1::ns2::Green;
+      //    ^      ^  ^
----------------
ioeric wrote:
> I'm wondering why the old `EndLoc` points to `G` instead of `n` in `Green`.
This is the behavior of `Expr->getLocEnd()` or `Expr->getSourceRange().getEnd()`, which returns the start location of the last token.


https://reviews.llvm.org/D38989