[PATCH] D39332: [clang-refactor] Introduce "local-qualified-rename" action.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Implementation LG.

My questions are mostly framework-level:

- should this be an Action, or just a Rule under the existing rename Action?
- {qname vs selection} x {local vs global} x ... seems like a combinatorial explosion in the number of rules. Shouldn't concerns like format of the input be taken care of before we get to the Rule?

@arphaman



================
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:143
+    }
+    auto USRs = getUSRsForDeclaration(ND, Context.getASTContext());
+    return tooling::createRenameAtomicChanges(
----------------
if this is a local refactor, and there are no USRs (symbol doesn't exist), is this an error that needs to be signaled somehow?


================
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:
----------------
As discussed offline, it's not clear why this is a separate Action, rather than a different Rule that's part of the same Action.

@arphaman how does the framework answer this question?


https://reviews.llvm.org/D39332