hokein updated this revision to Diff 121266.
hokein marked 3 inline comments as done.
hokein added a comment.
- add USRRenameRule interface.
- fix typos.
https://reviews.llvm.org/D39332
Files:
include/clang/Tooling/Refactoring/Rename/RenamingAction.h
lib/Tooling/Refactoring/RefactoringActio
jklaehn added a comment.
I spotted two typos. :) Also, the commit message needs to be updated.
Comment at: lib/Tooling/Refactoring/RefactoringActions.cpp:61
+ StringRef getDescription() const override {
+return "The new qualified to chagne the symbol to";
+ }
arphaman added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:101
+ std::string NewQualifiedName) {
+ return QualifiedRenameRule(std::move(OldQualifiedName),
+ std::move(NewQualifiedName));
hokein marked an inline comment as done.
hokein added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:
ioeric wrote:
> arphaman wrote:
> > ioeric wrote:
>
hokein updated this revision to Diff 120792.
hokein added a comment.
- rebase to master
- move the implementation to a refactoring rule in `local-rename`
- support non-selection based refactoring action invocation call path
https://reviews.llvm.org/D39332
Files:
include/clang/Tooling/Refactor
ioeric added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:
arphaman wrote:
> ioeric wrote:
> > ioeric wrote:
> > > arphaman wrote:
> > > > hokein wrote:
arphaman added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:
ioeric wrote:
> ioeric wrote:
> > arphaman wrote:
> > > hokein wrote:
> > > > sammccall wro
ioeric added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:
ioeric wrote:
> arphaman wrote:
> > hokein wrote:
> > > sammccall wrote:
> > > > As discussed
ioeric added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringAction {
+public:
arphaman wrote:
> hokein wrote:
> > sammccall wrote:
> > > As discussed offline, it's not cle
arphaman added a comment.
I've committed https://reviews.llvm.org/D38985, so you'd have to rebase
unfortunately. Things are still somewhat unstable :)
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:154
+
+class LocalQualifiedRename final : public RefactoringActi
hokein updated this revision to Diff 120575.
hokein added a comment.
Add an assert for non-empty USRs.
https://reviews.llvm.org/D39332
Files:
include/clang/Tooling/Refactoring/RefactoringActionRegistry.def
lib/Tooling/Refactoring/Rename/RenamingAction.cpp
test/Refactor/LocalQualifiedRenam
sammccall added inline comments.
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:143
+}
+auto USRs = getUSRsForDeclaration(ND, Context.getASTContext());
+return tooling::createRenameAtomicChanges(
hokein wrote:
> sammccall wrote:
> > if
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 refac
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 li
14 matches
Mail list logo
