diff options
author | James Player <jplayer@nvidia.com> | 2021-01-13 14:17:31 -0800 |
---|---|---|
committer | David Blaikie <dblaikie@gmail.com> | 2021-01-13 15:23:48 -0800 |
commit | 854f0984f0b7ab9a9a541a4bcda7ea173e4113d3 (patch) | |
tree | 736a76537885174ab6db434498ed216752a41f81 /llvm/lib/Support/Program.cpp | |
parent | 7ec8f43659861be42f9d600422678196849b6e90 (diff) | |
download | llvm-854f0984f0b7ab9a9a541a4bcda7ea173e4113d3.zip llvm-854f0984f0b7ab9a9a541a4bcda7ea173e4113d3.tar.gz llvm-854f0984f0b7ab9a9a541a4bcda7ea173e4113d3.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/Support/Program.cpp')
0 files changed, 0 insertions, 0 deletions