diff options
author | James Player <jplayer@nvidia.com> | 2021-01-16 09:34:20 -0500 |
---|---|---|
committer | Alexandre Ganea <alexandre.ganea@ubisoft.com> | 2021-01-16 09:37:04 -0500 |
commit | 25c1578a46ff93f920b7ad4e3057465902ced8f5 (patch) | |
tree | d95473e0956c6beb821db013d1e2b4efd7b5b3ea /llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp | |
parent | b765eaf9a617bd3da30f47ece731b33593929885 (diff) | |
download | llvm-25c1578a46ff93f920b7ad4e3057465902ced8f5.zip llvm-25c1578a46ff93f920b7ad4e3057465902ced8f5.tar.gz llvm-25c1578a46ff93f920b7ad4e3057465902ced8f5.tar.bz2 |
Fix llvm::Optional build breaks in MSVC using std::is_trivially_copyable
Current code breaks this version of MSVC due to a mismatch between `std::is_trivially_copyable` and `llvm::is_trivially_copyable` for `std::pair` instantiations. Hence I was attempting to use `std::is_trivially_copyable` to set `llvm::is_trivially_copyable<T>::value`.
I spent some time root causing an `llvm::Optional` build error on MSVC 16.8.3 related to the change described above:
```
62>C:\src\ocg_llvm\llvm-project\llvm\include\llvm/ADT/BreadthFirstIterator.h(96,12): error C2280: 'llvm::Optional<std::pair<std::pair<unsigned int,llvm::Graph<4>::NodeSubset> *,llvm::Optional<llvm::Graph<4>::ChildIterator>>> &llvm::Optional<std::pair<std::pair<unsigned int,llvm::Graph<4>::NodeSubset> *,llvm::Optional<llvm::Graph<4>::ChildIterator>>>::operator =(const llvm::Optional<std::pair<std::pair<unsigned int,llvm::Graph<4>::NodeSubset> *,llvm::Optional<llvm::Graph<4>::ChildIterator>>> &)': attempting to reference a deleted function (compiling source file C:\src\ocg_llvm\llvm-project\llvm\unittests\ADT\BreadthFirstIteratorTest.cpp)
...
```
The "trivial" specialization of `optional_detail::OptionalStorage` assumes that the value type is trivially copy constructible and trivially copy assignable. The specialization is invoked based on a check of `is_trivially_copyable` alone, which does not imply both `is_trivially_copy_assignable` and `is_trivially_copy_constructible` are true.
[[ https://en.cppreference.com/w/cpp/named_req/TriviallyCopyable | According to the spec ]], a deleted assignment operator does not make `is_trivially_copyable` false. So I think all these properties need to be checked explicitly in order to specialize `OptionalStorage` to the "trivial" version:
```
/// Storage for any type.
template <typename T, bool = std::is_trivially_copy_constructible<T>::value
&& std::is_trivially_copy_assignable<T>::value>
class OptionalStorage {
```
Above fixed my build break in MSVC, but I think we need to explicitly check `is_trivially_copy_constructible` too since it might be possible the copy constructor is deleted. Also would be ideal to move over to `std::is_trivially_copyable` instead of the `llvm` namespace verson.
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D93510
Diffstat (limited to 'llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp')
0 files changed, 0 insertions, 0 deletions