[PATCH] D51872: isPodLike: handle tuple and array

JF Bastien via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 10 13:18:46 PDT 2018


jfb abandoned this revision.
jfb added inline comments.


================
Comment at: include/llvm/Support/type_traits.h:59
+template <> struct isPodLike<std::tuple<>> { static const bool value = true; };
+template <typename T> struct isPodLike<std::tuple<T>> {
+  static const bool value = isPodLike<T>::value;
----------------
jfb wrote:
> ldionne wrote:
> > ldionne wrote:
> > > I'm not sure what the intent is, but `std::tuple` is not a POD even when its elements are -- it typically applies the EBO and I'm pretty sure it could actually use other tricks that would not make it a POD. Specifically, that would make it not Standard Layout AFAICT. 
> > https://wandbox.org/permlink/uTOa0CFb15ZBQby0
> Interesting! The idea is to work around GCC < 5 not having `std::is_trivially_copyable`... and `isPodLike` already says that `std::pair` matches `isPodLike` which is actually not true as you demonstrate.
> 
> My use case was `bit_cast` of `std::array` which *is* trivially copyable.
> 
> So now I have to change `isPodLike` to remove the `std::pair` overload, not add one for `std::tuple`, and do add one for `std::array`.
> 
> I'm therefore not just making it more capable, I'm also fixing a bug with `std::pair`.
> 
> To make everything worst, this is slightly inconsistent because of GCC 5! Methink we really want to upgrade minimum compiler past GCC 5. This BoF sounds like a great place to discuss it:
> https://llvm.org/devmtg/2018-10/talk-abstracts.html#bof3
I think I don't want to change `isPodLike` anymore. It's a crappy alternative to `std::is_trivially_copyable`. I think I want to special-case `bit_cast` instead to only check `sizeof` and builtin (if available) on GCC < 5, and rely on bots to catch bad usage otherwise (that is, most developers will do the full check, and outdated developers will break the bots).

I think `isPodLike` should die, but separately.


Repository:
  rL LLVM

https://reviews.llvm.org/D51872





More information about the llvm-commits mailing list