[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