[PATCH] D125609: [ADT] Adopt the new casting infrastructure for PointerUnion

[Aman LaChapelle via Phabricator via llvm-commits]

bzcheeseman added inline comments.


================
Comment at: llvm/include/llvm/ADT/PointerUnion.h:243
+struct CastInfo<To, const PointerUnion<PTs...>>
+    : ConstStrippingForwardingCast<To, const PointerUnion<PTs...>,
+                                   CastInfo<To, PointerUnion<PTs...>>> {};
----------------
0x59616e wrote:
> bzcheeseman wrote:
> > Public inheritance here too. Do you have checks that `const` is handled correctly?
> I've added some test for const case, but I'm not sure whether it's comprehensive enough or not.
If you see the tests for the cast infra there are some spots where we have `auto result = cast<T>(something);` followed by a static_assert that `decltype(result)` must be a specific type. That was the kind of thing I meant here - i.e. if I have a const pointer union and I do `cast<float *>` (for example), should it return `const float *`? Or not? I guess what I was going for was to explicitly test "what happens when I have a const input to a cast? How explicit do I have to be?

At the bottom there you're really close - it looks like you're on the right track but it would be great to get a static_assert for what the output type of a cast on PU4 is, just to make it explicit.


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

https://reviews.llvm.org/D125609