[PATCH] D54472: Disable invalid isPodLike<> specialization

[serge via Phabricator via llvm-commits]

serge-sans-paille marked 3 inline comments as done.
serge-sans-paille added a comment.

@Quuxplusone this patch has been rebuilt without NDEBUG, which triggered a few minor changes I have commented above. If @chandlerc is fine with current state, we should be able to merge it. Then move on to the `llvm::Optional` special case.



================
Comment at: include/llvm/Support/type_traits.h:163
+      (has_trivial_copy_constructor || has_trivial_copy_assign ||
+       has_trivial_move_constructor || has_trivial_move_assign);
+
----------------
serge-sans-paille wrote:
> Quuxplusone wrote:
> > This last clause — `(has_trivial_copy_constructor || has_trivial_copy_assign || has_trivial_move_constructor || has_trivial_move_assign)` — does not correspond to anything in the Standard. So:
> > 
> > https://godbolt.org/z/R5hCff
> > ```
> > struct S {
> >     S(S&&) = delete;
> > };
> > static_assert(is_trivially_copyable<S>::value);
> > ```
> > ```
> > error: static_assert failed "consistent behavior"
> >   static_assert(value == std::is_trivially_copyable<T>::value, "consistent behavior");
> >   ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ```
> > I'm not sure but I would suggest just getting rid of this last clause.
> From https://en.cppreference.com/w/cpp/named_req/TriviallyCopyable (yeah that's not the standard, but I do hope it's derived from the standard)
> 
> > at least one copy constructor, move constructor, copy assignment operator, or move assignment operator is non-deleted
> 
> That's the intent I was trying to capture.
> 
> I'm okay with getting rid of this capture, especially when looking at https://godbolt.org/z/p3-Cy4.
@Quuxplusone I had to reintroduce the ` (has_trivial_move_assign || has_trivial_move_constructor || has_trivial_copy_assign || has_trivial_copy_constructor)` condition, which is indeed a standard requirement. It basically states « if the class is not meant to be copied, don't allow to copy it ».


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

https://reviews.llvm.org/D54472