[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D61335#1495324 <https://reviews.llvm.org/D61335#1495324>, @ilya-biryukov wrote:

> Sorry for the delay.
>  I personally like the `RewriteRule::Action` best. Allows to use a rather common term, while still avoiding any possible confusion.


I tried going with `RewriteRule::Action` (it was my preference as well), but found that it didn't work well once I tried folding the composite rules into RewriteRule.  The problem is that the Action structure forces you to eagerly join the matchers (which is what my first diff did). But then it doesn't work to join multiple rules that themselves were built with makeOrderedRule. Previously, this was impossible because they had different types, but now a user could conceivably do this.  So, I changed the implementation build the joined matcher later.  RewriteRule now just stores all of the (sub)rules in a single list, and `makeOrderedRule` is a trivial merge.  The real work is done at `registerMatchers()`-time where we build the joined matcher for the whole rule. But, this means keeping the whole (sub)rule around, not just the Action part.

Alternatives I considered:

1. We could use the Action structure inside Transformer -- instead of storing a RewriteRule, we could simply store a single matcher and vector of Action. In the constructor, we would call `buildMatcher(Rule)` and then copy only the action parts from the rules.  However, I don't think this added complexity will gain us anything other than a structure which more faithfully reflects the control flow -- the matchers in each case are "dead" once we build the joined matcher, and the Action approach would reflect that.

2. We could add more structure to Case, like:

  struct RewriteRule {
    struct Action {
      SmallVector<ASTEdit, 1> Edits;
      TextGenerator Explanation;
    };
    struct Case {
      ast_matchers::internal::DynTypedMatcher Matcher;
      Action A;
    };
    std::vector<Cases> Cases;
  };


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61335/new/

https://reviews.llvm.org/D61335