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

Eric Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 17 01:37:55 PDT 2017


ioeric 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())
----------------
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.


================
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:218
+              llvm::dyn_cast_or_null<EnumDecl>(getClosestAncestorDecl(*T))) {
+        if (ED->isScoped()) {
+          return true;
----------------
nit: no braces


================
Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:228
+      // enum constant `Green` should be excluded).
+      //    ns1::ns2::Green;
+      //    ^      ^  ^
----------------
I'm wondering why the old `EndLoc` points to `G` instead of `n` in `Green`.


https://reviews.llvm.org/D38989





More information about the cfe-commits mailing list