[PATCH] D103223: [ADT][WIP] Proof of concept impl of generic visit for PointerUnion
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 4 13:21:16 PDT 2021
dblaikie accepted this revision.
dblaikie added inline comments.
This revision is now accepted and ready to land.
================
Comment at: llvm/include/llvm/ADT/PointerUnion.h:282
+ static constexpr size_t getIndex(const PointerUnion<PTs...> &Variant) {
+ return Variant.Val.getInt();
+ }
----------------
scott.linder wrote:
> dblaikie wrote:
> > Is this the only reason the variant traits needs to be befriended by the PointerUnion? If so, perhaps this API could be exposed in PointerUnion's public API rather than using friendship?
> From a pure perspective I think it is an implementation detail, which is the same reason I didn't expose the `index` in `IntrusiveVariant` either. It seems reasonable to expose, as `std::variant` does expose an `::index` method, but I can't think of many uses of it that don't have a better, type-safe alternative.
Eh, I don't feel too strongly about it - but figure if it's an important feature for VariantTraits, that seems like as good a reason as any for it to be public (since you can access the property via variant traits anyway - so why not access it directly at that point?).
Up to you.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103223/new/
https://reviews.llvm.org/D103223
More information about the llvm-commits
mailing list