Index: llvm/trunk/include/llvm/ADT/Optional.h =================================================================== --- llvm/trunk/include/llvm/ADT/Optional.h +++ llvm/trunk/include/llvm/ADT/Optional.h @@ -29,36 +29,77 @@ class raw_ostream; namespace optional_detail { + /// Storage for any type. -template ::value> struct OptionalStorage { +// +template struct OptionalTrivialStorage { AlignedCharArrayUnion storage; bool hasVal = false; + OptionalTrivialStorage() = default; + OptionalTrivialStorage(OptionalTrivialStorage const &) = default; + OptionalTrivialStorage(OptionalTrivialStorage &&) = default; + OptionalTrivialStorage &operator=(OptionalTrivialStorage const &) = default; + OptionalTrivialStorage &operator=(OptionalTrivialStorage &&) = default; + ~OptionalTrivialStorage() = default; + + OptionalTrivialStorage(const T &y) : hasVal(true) { + new (storage.buffer) T(y); + } + OptionalTrivialStorage(T &&y) : hasVal(true) { + new (storage.buffer) T(std::move(y)); + } + + OptionalTrivialStorage &operator=(const T &y) { + new (storage.buffer) T(y); + hasVal = true; + return *this; + } + OptionalTrivialStorage &operator=(T &&y) { + new (storage.buffer) T(std::move(y)); + hasVal = true; + return *this; + } + + T *getPointer() { + assert(hasVal); + return reinterpret_cast(storage.buffer); + } + const T *getPointer() const { + assert(hasVal); + return reinterpret_cast(storage.buffer); + } + void reset() { hasVal = false; } +}; + +template struct OptionalStorage : OptionalTrivialStorage { OptionalStorage() = default; - OptionalStorage(const T &y) : hasVal(true) { new (storage.buffer) T(y); } - OptionalStorage(const OptionalStorage &O) : hasVal(O.hasVal) { - if (hasVal) - new (storage.buffer) T(*O.getPointer()); - } - OptionalStorage(T &&y) : hasVal(true) { - new (storage.buffer) T(std::forward(y)); + OptionalStorage(const T &y) : OptionalTrivialStorage(y) {} + OptionalStorage(T &&y) : OptionalTrivialStorage(std::move(y)) {} + + OptionalStorage(const OptionalStorage &O) : OptionalTrivialStorage() { + this->hasVal = O.hasVal; + if (this->hasVal) + new (this->storage.buffer) T(*O.getPointer()); } - OptionalStorage(OptionalStorage &&O) : hasVal(O.hasVal) { + + OptionalStorage(OptionalStorage &&O) : OptionalTrivialStorage() { + this->hasVal = O.hasVal; if (O.hasVal) { - new (storage.buffer) T(std::move(*O.getPointer())); + new (this->storage.buffer) T(std::move(*O.getPointer())); } } OptionalStorage &operator=(T &&y) { - if (hasVal) - *getPointer() = std::move(y); + if (this->hasVal) + *this->getPointer() = std::move(y); else { - new (storage.buffer) T(std::move(y)); - hasVal = true; + OptionalTrivialStorage::operator=(std::move(y)); } return *this; } + OptionalStorage &operator=(OptionalStorage &&O) { if (!O.hasVal) reset(); @@ -74,11 +115,10 @@ // requirements (notably: the existence of a default ctor) when implemented // in that way. Careful SFINAE to avoid such pitfalls would be required. OptionalStorage &operator=(const T &y) { - if (hasVal) - *getPointer() = y; + if (this->hasVal) + *this->getPointer() = y; else { - new (storage.buffer) T(y); - hasVal = true; + OptionalTrivialStorage::operator=(y); } return *this; } @@ -93,26 +133,19 @@ ~OptionalStorage() { reset(); } void reset() { - if (hasVal) { - (*getPointer()).~T(); - hasVal = false; + if (this->hasVal) { + (*this->getPointer()).~T(); + OptionalTrivialStorage::reset(); } } - - T *getPointer() { - assert(hasVal); - return reinterpret_cast(storage.buffer); - } - const T *getPointer() const { - assert(hasVal); - return reinterpret_cast(storage.buffer); - } }; } // namespace optional_detail template class Optional { - optional_detail::OptionalStorage Storage; + typename std::conditional::value, + optional_detail::OptionalTrivialStorage, + optional_detail::OptionalStorage>::type Storage; public: using value_type = T; Index: llvm/trunk/unittests/ADT/OptionalTest.cpp =================================================================== --- llvm/trunk/unittests/ADT/OptionalTest.cpp +++ llvm/trunk/unittests/ADT/OptionalTest.cpp @@ -16,6 +16,12 @@ namespace { +static_assert(llvm::is_trivially_copyable>::value, + "trivially copyable"); + +static_assert(llvm::is_trivially_copyable>>::value, + "trivially copyable"); + struct NonDefaultConstructible { static unsigned CopyConstructions; static unsigned Destructions; @@ -43,6 +49,10 @@ unsigned NonDefaultConstructible::Destructions = 0; unsigned NonDefaultConstructible::CopyAssignments = 0; +static_assert( + !llvm::is_trivially_copyable>::value, + "not trivially copyable"); + // Test fixture class OptionalTest : public testing::Test { }; @@ -201,6 +211,10 @@ }; unsigned MultiArgConstructor::Destructions = 0; +static_assert( + !llvm::is_trivially_copyable>::value, + "not trivially copyable"); + TEST_F(OptionalTest, Emplace) { MultiArgConstructor::ResetCounts(); Optional A; @@ -248,6 +262,9 @@ unsigned MoveOnly::Destructions = 0; unsigned MoveOnly::MoveAssignments = 0; +static_assert(!llvm::is_trivially_copyable>::value, + "not trivially copyable"); + TEST_F(OptionalTest, MoveOnlyNull) { MoveOnly::ResetCounts(); Optional O; @@ -349,6 +366,9 @@ unsigned Immovable::Constructions = 0; unsigned Immovable::Destructions = 0; +static_assert(!llvm::is_trivially_copyable>::value, + "not trivially copyable"); + TEST_F(OptionalTest, ImmovableEmplace) { Optional A; Immovable::ResetCounts();