hokein added inline comments.
================
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:101
+ std::string NewQualifiedName) {
+ return QualifiedRenameRule(std::move(OldQualifiedName),
+ std::move(NewQualifiedName));
----------------
arphaman wrote:
> ioeric wrote:
> > hokein wrote:
> > > arphaman wrote:
> > > > It might be better to find the declaration (and report error if needed)
> > > > during in initiation, and then pass the `ND` to the class. Maybe then
> > > > both `RenameOccurrences` and `QualifiedRenameRule` could subclass from
> > > > one base class that actually does just this:
> > > >
> > > > ```
> > > > auto USRs = getUSRsForDeclaration(ND, Context.getASTContext());
> > > > assert(!USRs.empty());
> > > > return tooling::createRenameAtomicChanges(
> > > > USRs, NewQualifiedName,
> > > > Context.getASTContext().getTranslationUnitDecl());
> > > > ```
> > > Done. Introduced a common interface `USRRenameRule`.
> > `USRRenameRule` doesn't seem to be a very useful abstraction. I think what
> > Alex proposed is a base class that implements `createSourceReplacements`
> > which could be shared by both `RenameOccurrences` and
> > `QualifiedRenameRule`. Intuitively, un-qualified rename is a special case
> > of qualified rename (maybe?), where namespace is not changed.
> Yep, I meant that indeed.
Ah, sorry for the misunderstanding.
Unfortunately, `RenameOccurrences` and `QualifiedRenameRule` could not share
the same `createSourceReplacements` implementation,
* Their supported use cases are different. `QualifiedRenameRule` only supports
renaming class, function, enums, alias and class members, which doesn't cover
all the cases of `RenameOccurrences` (like renaming local variable in function
body).
* we have separated implementations in the code
base(`createRenameAtomicChanges` for qualified rename,
`createRenameReplacements` for un-qualified rename). The implementation of
qualified rename does more things, including calculating the shortest prefix
qualifiers, getting correct range of replacement.
================
Comment at: lib/Tooling/Refactoring/Rename/RenamingAction.cpp:115
+ /*Description=*/
+ "Finds and renames qualified symbols in code with no indexer support",
+ };
----------------
ioeric wrote:
> We should also document the behavior when namespace is changed. What would
> happen to symbol definitions and symbol references?
Sound good! Done.
https://reviews.llvm.org/D39332
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
