[PATCH] D125609: [ADT] Adopt the new casting infrastructure for PointerUnion
Aman LaChapelle via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat May 14 20:36:18 PDT 2022
bzcheeseman added a comment.
In D125609#3514009 <https://reviews.llvm.org/D125609#3514009>, @0x59616e wrote:
> Define `CastInfoPointerUnionImpl` as below :
>
> template <typename... PTs> struct CastInfoPointerUnionImpl {
> using From = PointerUnion<PTs...>;
>
> template <typename To> static inline bool isPossible(From &F) {
> return F.Val.getInt() == FirstIndexOfType<To, PTs...>::value;
> }
>
> template <typename To> static To doCast(From &F) {
> assert(isPossible<To>(F) && "cast to an incompatible type !");
> return PointerLikeTypeTraits<To>::getFromVoidPointer(F.Val.getPointer());
> }
> };
>
> and declared it as a friend of PointerUnion.
Could you declare the CastInfo struct itself as a friend? something like `template<typename To, typename...PTs> friend struct CastInfo<To, PointerUnion<PTs...>>`? That way you don't need the impl struct so it cuts down on overall code. Truly asking here - I think it's doable but I may be wrong.
> And then `CastInfo`:
>
> template <typename To, typename... PTs>
> struct CastInfo<To, PointerUnion<PTs...>>
> : NullableValueCastFailed<To>,
> DefaultDoCastIfPossible<To, PointerUnion<PTs...>,
> CastInfo<To, PointerUnion<PTs...>>> {
> using From = PointerUnion<PTs...>;
> using Impl = CastInfoPointerUnionImpl<PTs...>;
>
> static inline bool isPossible(From &f) {
> return Impl::template isPossible<To>(f);
> }
>
> static To doCast(From &f) {
> return Impl::template doCast<To>(f);
> }
> };
This looks really nice - I think it still will even with the Impl::* methods inlined :)
> By doing this we can implement `PointerUnion::is` and `PointerUnion::get` in this way:
>
> /// Test if the Union currently holds the type matching T.
> template <typename T> bool is() const { return isa<T>(*this); }
>
> /// Returns the value of the specified pointer type.
> ///
> /// If the specified pointer type is incorrect, assert.
> template <typename T> T get() const {
> assert(isa<T>(*this) && "Invalid accessor called");
> return cast<T>(*this);
> }
>
> Does this make sense ?
Comments inline :) Overall makes good sense to me though!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125609/new/
https://reviews.llvm.org/D125609
More information about the llvm-commits
mailing list