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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to