ymandel added a comment.
In D61335#1488212 <https://reviews.llvm.org/D61335#1488212>, @ilya-biryukov
wrote:
> - Could you provide a few real-world motivating examples? Especially
> interested in cases that were complicated before and are easy to do now?
Sure, below are some examples based on actual tidies I've written (although not
released). Let me know whether you think any of these should be expressed in
the comments:
EXAMPLE 1
Consider a type `T` with a deterministic serialization function, `serialize()`.
For performance reasons, we would like to make it non deterministic.
Therefore, we want to drop the expectation that `a.serialize() = b.serialize()
iff a = b` (although we’ll maintain `deserialize(a.serialize()) = a`).
Let’s assume this comes up in testing. We have three cases to consider:
EXPECT_EQ(a.serialize(), b.serialize());
EXPECT_EQ(a, b.serialize());
EXPECT_EQ(a.serialize(), b);
With ordered choice, you can encode the above directly into matchers and then
compose them into an ordered choice rule. Without it, you’d have to write
these as
EXPECT_EQ(a.serialize(), b.serialize());
EXPECT_EQ(a, b.serialize()); // where a != a’.serialize()
EXPECT_EQ(a.serialize(), b); // where b != b’.serialize()
where the constraints in the comments are explicitly in the matchers.
EXAMPLE 2
Consider patterns of the form
absl::WrapUnique(e.release())
where `e` is an expression of type `std::unique_ptr<...>`. This pattern was
necessary in gcc for certain circumstances relating to upcasting the value type
of the `unique_ptr`. It is not necessary in clang, and so should be removed.
In some cases, it should be rewritten to `e` and sometimes to `std::move(e)`,
depending on the value category of `e` and the context (the expression of a
return statement or not). That is, there are two cases:
`absl::WrapUnique(e.release())` rewrites to `e`, if `e` is an r/x-value OR the
call appears as a return expression,
`absl::WrapUnique(e.release())` rewrites to `std::move(e)`.
With ordered choice, the second case is written plainly (that is, just
translate that expression to a matcher). Without it, you need to further
express: unless `e` is an r/x-value OR the call appears as a return expression.
EXAMPLE 3
More generally, anywhere you’d use `anyOf(m1.bind(“m1”), m2.bind(“m2”))` and
then dispatch on those tags in your code to decide what to do, we’ll lift that
behavior to the rule level, so you can write
makeOrderedRule({
makeRule(m1, action1), makeRule(m2, action2), …
});
> - Do we expect most the rules to be written using the new API or is this
> something that's gonna be used in `5-10%` of the cases?
I'd expect higher than 10% but not the majority of cases. However, given that
this semantics exactly matches that of `anyOf`, it may well be more common than
I expect.
> - Could we consider expressing the `CompositeRewriteRule` as a `RewriteRule`
> itself? That might require extending the `RewriteRule` abstraction, but the
> mental model that makes sense to me is: a rewrite rule matches some AST nodes
> and applies replacements to them. A `CompositeRewriteRule` seems to match the
> same model, despite the fact it contains other rules. Would be nice if we
> could make this fact internal to the implementation.
>
> Also a bit uneasy about the naming. `Composite` suggests the individual
> rules compose one after another, while in practice only one alternative is
> chosen.
Great points! I'm fine with collapsing the types into one. As you can see from
line 260 in Transformer.h, we're already folding the single rule into the
composite rule. As for naming, agreed, but does that concern drop away once we
have only a single RewriteRule definition?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61335/new/
https://reviews.llvm.org/D61335
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
