[PATCH] D36075: [refactor] Initial support for refactoring action rules
Manuel Klimek via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 28 08:21:47 PDT 2017
klimek added inline comments.
================
Comment at: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h:23-37
+/// A common refactoring action rule interface.
+class RefactoringActionRule {
+public:
+ enum RuleKind { SourceChangeRefactoringRuleKind };
+
+ RuleKind getRuleKind() const { return Kind; }
+
----------------
arphaman wrote:
> klimek wrote:
> > Sorry for being late, was out on vacation.
> > Generally, why do we need this tag-based abstraction here instead of using the more typical OO double-dispatch where necessary?
> > (We do this in the AST a lot, but the AST is special, because there we want to implement a lot of different algorithms that rely on the node type, while I don't see how that applies here)
> Generally the clients will have to somehow distinguish between the types of results that are produced by rules to figure out what to do (e.g. AtomicChanges -> apply, SymbolOccurrences -> ask user, Continuation -> look for more ASTs). So far I've thought that the LLVM based dynamic casts will work well for this, e.g.
>
> ```
> if (auto *Action = dyn_cast<SourceChangeRefactoringRule>()) {
> Expected<Optional<AtomicChanges>> Changes = Action->createSourceReplacements();
> applyChanges(Changes);
> } else if (...) {
> ...
> } else (...) {
> ...
> }
> ```
>
> But you're probably right, there might be a better way to do this rather than the tag based approach. Something like a consumer class that clients can implement that provides consumer functions that take in the specific results. I reckon a single consumer will actually work better in the long-run when we might Continuations that both return changes in the first TU and information for searches in other TUs. I'll see if I can get a patch out that removes this tag and uses the consumer approach.
Cool, looking forward to it :)
Repository:
rL LLVM
https://reviews.llvm.org/D36075
More information about the cfe-commits
mailing list