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

[Sheng via Phabricator via llvm-commits]

0x59616e 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...>>> {};
----------------
bzcheeseman wrote:
> 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.
What's your opinion about whether it should return a const or non-const pointer when casting from a const `PointerUnion` ?


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

https://reviews.llvm.org/D125609