[PATCH] D93510: Fix llvm::Optional build breaks in MSVC using std::is_trivially_copyable

[James Player via Phabricator via llvm-commits]

jplayer-nv updated this revision to Diff 316750.
jplayer-nv added a comment.

I just triaged the gcc 5.4 build break.  It appears that `std::is_trivially_copy_constructible<T>` instantiates the copy constructor (which is... strange).  So we can't query a compile-time property without the compiler forcing instantiation of the deleted copy constructor in this case.

This bug exists all the way up through gcc 7.3.  Here's a godbolt link which reproduces the problem:
https://godbolt.org/z/ETW7f1

If we use `llvm::is_trivially_copy_constructible<T>` instead of `std::is_trivially_copy_constructible<T>`, then everything appears to be fine.

Interestingly, `std::is_trivially_copy_assignable<T>` appears not to instantiate the copy assignment because the compiler isn't complaining about its use.  So I'm changing the `OptionalStorage` specialization condition to:

  template <typename T, bool = (is_trivially_copy_constructible<T>::value &&
                                std::is_trivially_copy_assignable<T>::value &&
                                (std::is_trivially_move_constructible<T>::value ||
                                 !std::is_move_constructible<T>::value) &&
                                (std::is_trivially_move_assignable<T>::value ||
                                 !std::is_move_assignable<T>::value))>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93510

Files:
  llvm/include/llvm/ADT/Optional.h
  llvm/unittests/ADT/OptionalTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D93510.316750.patch
Type: text/x-patch
Size: 5840 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210114/9d16a496/attachment.bin>