Skip to content

Commit 6e0b562

Browse files
author
Serge Guelton
committed
[NFC] Make Optional<T> trivially copyable when T is trivially copyable
This is a follow-up to r354246 and a reimplementation of https://reviews.llvm.org/D57097?id=186600 that should not trigger any UB thanks to the use of an union. This may still be subject to the problem solved by std::launder, but I'm unsure how it interacts whith union. /me plans to revert if this triggers any relevant bot failure. At least this validates in Release mode with clang 6.0.1 and gcc 4.8.5. llvm-svn: 354264
1 parent 515e7cd commit 6e0b562

File tree

2 files changed

+93
-0
lines changed

2 files changed

+93
-0
lines changed

llvm/include/llvm/ADT/Optional.h

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,78 @@ class OptionalStorage {
139139
}
140140
};
141141

142+
template <typename T> class OptionalStorage<T, true> {
143+
union {
144+
char empty;
145+
T value;
146+
};
147+
bool hasVal = false;
148+
149+
public:
150+
~OptionalStorage() = default;
151+
152+
OptionalStorage() noexcept : empty{} {}
153+
154+
OptionalStorage(OptionalStorage const &other) = default;
155+
OptionalStorage(OptionalStorage &&other) = default;
156+
157+
OptionalStorage &operator=(OptionalStorage const &other) = default;
158+
OptionalStorage &operator=(OptionalStorage &&other) = default;
159+
160+
template <class... Args>
161+
explicit OptionalStorage(in_place_t, Args &&... args)
162+
: value(std::forward<Args>(args)...), hasVal(true) {}
163+
164+
void reset() noexcept {
165+
if (hasVal) {
166+
value.~T();
167+
hasVal = false;
168+
}
169+
}
170+
171+
bool hasValue() const noexcept { return hasVal; }
172+
173+
T &getValue() LLVM_LVALUE_FUNCTION noexcept {
174+
assert(hasVal);
175+
return value;
176+
}
177+
T const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
178+
assert(hasVal);
179+
return value;
180+
}
181+
#if LLVM_HAS_RVALUE_REFERENCE_THIS
182+
T &&getValue() && noexcept {
183+
assert(hasVal);
184+
return std::move(value);
185+
}
186+
#endif
187+
188+
template <class... Args> void emplace(Args &&... args) {
189+
reset();
190+
::new ((void *)std::addressof(value)) T(std::forward<Args>(args)...);
191+
hasVal = true;
192+
}
193+
194+
OptionalStorage &operator=(T const &y) {
195+
if (hasValue()) {
196+
value = y;
197+
} else {
198+
::new ((void *)std::addressof(value)) T(y);
199+
hasVal = true;
200+
}
201+
return *this;
202+
}
203+
OptionalStorage &operator=(T &&y) {
204+
if (hasValue()) {
205+
value = std::move(y);
206+
} else {
207+
::new ((void *)std::addressof(value)) T(std::move(y));
208+
hasVal = true;
209+
}
210+
return *this;
211+
}
212+
};
213+
142214
} // namespace optional_detail
143215

144216
template <typename T> class Optional {

llvm/unittests/ADT/OptionalTest.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,15 @@
1414

1515
#include <array>
1616

17+
1718
using namespace llvm;
1819

20+
static_assert(is_trivially_copyable<Optional<int>>::value,
21+
"trivially copyable");
22+
23+
static_assert(is_trivially_copyable<Optional<std::array<int, 3>>>::value,
24+
"trivially copyable");
25+
1926
namespace {
2027

2128
struct NonDefaultConstructible {
@@ -45,6 +52,10 @@ unsigned NonDefaultConstructible::CopyConstructions = 0;
4552
unsigned NonDefaultConstructible::Destructions = 0;
4653
unsigned NonDefaultConstructible::CopyAssignments = 0;
4754

55+
static_assert(
56+
!is_trivially_copyable<Optional<NonDefaultConstructible>>::value,
57+
"not trivially copyable");
58+
4859
// Test fixture
4960
class OptionalTest : public testing::Test {
5061
};
@@ -203,6 +214,10 @@ struct MultiArgConstructor {
203214
};
204215
unsigned MultiArgConstructor::Destructions = 0;
205216

217+
static_assert(
218+
!is_trivially_copyable<Optional<MultiArgConstructor>>::value,
219+
"not trivially copyable");
220+
206221
TEST_F(OptionalTest, Emplace) {
207222
MultiArgConstructor::ResetCounts();
208223
Optional<MultiArgConstructor> A;
@@ -250,6 +265,9 @@ unsigned MoveOnly::MoveConstructions = 0;
250265
unsigned MoveOnly::Destructions = 0;
251266
unsigned MoveOnly::MoveAssignments = 0;
252267

268+
static_assert(!is_trivially_copyable<Optional<MoveOnly>>::value,
269+
"not trivially copyable");
270+
253271
TEST_F(OptionalTest, MoveOnlyNull) {
254272
MoveOnly::ResetCounts();
255273
Optional<MoveOnly> O;
@@ -351,6 +369,9 @@ struct Immovable {
351369
unsigned Immovable::Constructions = 0;
352370
unsigned Immovable::Destructions = 0;
353371

372+
static_assert(!is_trivially_copyable<Optional<Immovable>>::value,
373+
"not trivially copyable");
374+
354375
TEST_F(OptionalTest, ImmovableEmplace) {
355376
Optional<Immovable> A;
356377
Immovable::ResetCounts();

0 commit comments

Comments
 (0)