hokein added inline comments.
================
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:143
+ }
+ auto USRs = getUSRsForDeclaration(ND, Context.getASTContext());
+ return tooling::createRenameAtomicChanges(
----------------
sammccall wrote:
> 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?
This case should not be happened IMO. If we find the `ND`, we will rename `ND`
at least.
================
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:
----------------
sammccall wrote:
> 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?
There is a
[document](https://clang.llvm.org/docs/RefactoringEngine.html#refactoring-action-rules)
describing it, but still ambiguous.
We also had some questions about `local-rename` from the discussion, need
@arphaman's input:
* `OccurrenceFinder` is not exposed now, it is merely used in
`RenameOccurrences`. We think there should be a public interface to the
clients, like for implementing interactive mode in IDE?
* Currently the rules defined in the same action must have mutual command-line
options, otherwise clang-refactor would complain the command-line option are
being registered more than once. It might be very strict for some cases. For
example, `-new-name` is most likely being used by many rules in `local-rename`
action.
https://reviews.llvm.org/D39332
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
