arphaman added inline comments.
================
Comment at: include/clang/Tooling/Refactoring/RefactoringResult.h:21
+struct RefactoringResult {
+ enum ResultKind {
+ /// A set of source replacements represented using a vector of
----------------
ioeric wrote:
> arphaman wrote:
> > ioeric wrote:
> > > I'm a bit unsure about the abstraction of the refactoring result. I would
> > > expected refactoring results to be source changes always. Do you have any
> > > refactoring tool that outputs occurrences in mind?
> > In Xcode we require rename to return symbol occurrences because the IDE is
> > responsible for figuring out:
> > 1) The new name. Thus we can't produce replacements when renaming because
> > we don't know what the new name is when gathering the occurrences.
> > 2) Whether these occurrences should be actually applied. Thus we can't
> > produce replacements because it's up to the user to decide if they want to
> > rename some occurrence in a comment for example.
> >
> > In general 2) can be applied to tools like clang-refactor that could allow
> > users to select occurrences that don't guarantee a direct semantic match
> > (comments, etc.) in an interactive manner.
> >
> > I think clangd has a rather naive rename support, so these points may not
> > apply, but we may want to extend clangd's support for rename in the future
> > as well.
> I feel occurrences are more of an intermediate state of a refactoring action
> than a result. I'm wondering if it makes sense to introduce a separate class
> to represent such intermediate states? I am a bit nervous to fuse multiple
> classes into one; the interfaces can get pretty ugly when more result kinds
> are added.
Good point. I agree.
I think it would be better to differentiate between `RefactoringActionRules`
then. Ordinary rules return a set of AtomicChanges instead of
RefactoringResult. But then we could also have "interactive" rules that return
"partial" results like symbol occurrences.
I think I'll try the following approach:
```
class RefactoringActionRule {
virtual ~RefactoringActionRule() {}
};
class RefactoringActionSourceChangeRule: public RefactoringActionRule {
public:
virtual Expected<Optional<AtomicChanges>>
createSourceReplacements(RefactoringOperation &Operation) = 0;
};
class RefactoringActionSymbolOccurrencesRule: public RefactoringActionRule {
public:
virtual Expected<Optional<SymbolOccurrences>>
findSymbolOccurrences(RefactoringOperation &Operation) = 0;
};
```
Repository:
rL LLVM
https://reviews.llvm.org/D36075
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
