[PATCH] D99561: Support visitor pattern by PointerUnion.

[Scott Linder via Phabricator via llvm-commits]

scott.linder added inline comments.


================
Comment at: llvm/include/llvm/ADT/PointerUnion.h:307
+  /// Named differently because the calling convention is different. Name is
+  /// inspired by Rust's `match` keyword.
+  template <typename... LAMBDAS> auto match(LAMBDAS... lambdas) {
----------------
scott.linder wrote:
> I don't know that I'm in favor of having this version at all, but even if we do I don't know that comparing it to Rust's `match` helps explain it. I see Rust's `match` as closer to `std::visit`, in that neither use the postfix-dot notation.
> 
> Naming aside, I also just think the bar for ergonomics needs to be higher to justify having more than one way to do something.
To be more specific about why I would lean towards not adding it, this `match` function lets us write:

```
variant.match(
  [](...) { ... },
  [](...) { ... },
  ...
  [](...) { ... }
);
```

instead of:

```
visit(makeVisitor(
  [](...) { ... },
  [](...) { ... },
  ...
  [](...) { ... }
), variant);
```

Which is a net elimination of ~3 tokens (i.e. remove `makeVisitor`, `(`, `)` and replace `,` with `.`) in an expression that likely has a non-trivial amount of meaningful code in each "..."

Compare that to the `sort` case, where there can be more boilerplate than actual meaningful code:
```
sort(foo);
sort(foo.begin(), foo.end());
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99561