From 7c1ef4a88c4f19463460adb2480adfe398edb908 Mon Sep 17 00:00:00 2001 From: NAKAMURA Takumi Date: Wed, 1 Nov 2017 05:14:31 +0000 Subject: [PATCH] Revert rL317019, "[ADT] Split optional to only include copy mechanics and dtor for non-trivial types." Seems g++-4.8 (eg. Ubuntu 14.04) doesn't like this. llvm-svn: 317077 --- llvm/include/llvm/ADT/Optional.h | 166 +++++++++++++----------------------- llvm/unittests/ADT/OptionalTest.cpp | 9 -- 2 files changed, 58 insertions(+), 117 deletions(-) diff --git a/llvm/include/llvm/ADT/Optional.h b/llvm/include/llvm/ADT/Optional.h index 3660a0a..b782d9d 100644 --- a/llvm/include/llvm/ADT/Optional.h +++ b/llvm/include/llvm/ADT/Optional.h @@ -27,164 +27,114 @@ namespace llvm { -namespace optional_detail { -/// Storage for any type. -template struct OptionalStorage { +template +class Optional { AlignedCharArrayUnion storage; bool hasVal = false; - OptionalStorage() = default; +public: + using value_type = T; - OptionalStorage(const T &y) : hasVal(true) { new (storage.buffer) T(y); } - OptionalStorage(const OptionalStorage &O) : hasVal(O.hasVal) { + Optional(NoneType) {} + explicit Optional() {} + + Optional(const T &y) : hasVal(true) { + new (storage.buffer) T(y); + } + + Optional(const Optional &O) : hasVal(O.hasVal) { if (hasVal) - new (storage.buffer) T(*O.getPointer()); + new (storage.buffer) T(*O); } - OptionalStorage(T &&y) : hasVal(true) { + + Optional(T &&y) : hasVal(true) { new (storage.buffer) T(std::forward(y)); } - OptionalStorage(OptionalStorage &&O) : hasVal(O.hasVal) { - if (O.hasVal) { - new (storage.buffer) T(std::move(*O.getPointer())); + + Optional(Optional &&O) : hasVal(O) { + if (O) { + new (storage.buffer) T(std::move(*O)); O.reset(); } } - OptionalStorage &operator=(T &&y) { + ~Optional() { + reset(); + } + + Optional &operator=(T &&y) { if (hasVal) - *getPointer() = std::move(y); + **this = std::move(y); else { new (storage.buffer) T(std::move(y)); hasVal = true; } return *this; } - OptionalStorage &operator=(OptionalStorage &&O) { - if (!O.hasVal) + + Optional &operator=(Optional &&O) { + if (!O) reset(); else { - *this = std::move(*O.getPointer()); + *this = std::move(*O); O.reset(); } return *this; } + /// Create a new object by constructing it in place with the given arguments. + template + void emplace(ArgTypes &&...Args) { + reset(); + hasVal = true; + new (storage.buffer) T(std::forward(Args)...); + } + + static inline Optional create(const T* y) { + return y ? Optional(*y) : Optional(); + } + // FIXME: these assignments (& the equivalent const T&/const Optional& ctors) // could be made more efficient by passing by value, possibly unifying them // with the rvalue versions above - but this could place a different set of // 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) { + Optional &operator=(const T &y) { if (hasVal) - *getPointer() = y; + **this = y; else { new (storage.buffer) T(y); hasVal = true; } return *this; } - OptionalStorage &operator=(const OptionalStorage &O) { - if (!O.hasVal) + + Optional &operator=(const Optional &O) { + if (!O) reset(); else - *this = *O.getPointer(); + *this = *O; return *this; } - ~OptionalStorage() { reset(); } - void reset() { if (hasVal) { - (*getPointer()).~T(); + (**this).~T(); hasVal = false; } } - T *getPointer() { - assert(hasVal); - return reinterpret_cast(storage.buffer); - } - const T *getPointer() const { - assert(hasVal); - return reinterpret_cast(storage.buffer); - } -}; - -/// Storage for trivially copyable types only. -template struct OptionalStorage { - AlignedCharArrayUnion storage; - bool hasVal = false; - - OptionalStorage() = default; - - OptionalStorage(const T &y) : hasVal(true) { new (storage.buffer) T(y); } - OptionalStorage &operator=(const T &y) { - new (storage.buffer) T(y); - hasVal = true; - return *this; - } - - void reset() { hasVal = false; } -}; -} // namespace optional_detail - -template class Optional { - optional_detail::OptionalStorage::value> Storage; - -public: - using value_type = T; - - constexpr Optional() {} - constexpr Optional(NoneType) {} - - Optional(const T &y) : Storage(y) {} - Optional(const Optional &O) = default; - - Optional(T &&y) : Storage(std::forward(y)) {} - Optional(Optional &&O) = default; - - Optional &operator=(T &&y) { - Storage = std::move(y); - return *this; - } - Optional &operator=(Optional &&O) = default; - - /// Create a new object by constructing it in place with the given arguments. - template void emplace(ArgTypes &&... Args) { - reset(); - Storage.hasVal = true; - new (getPointer()) T(std::forward(Args)...); - } - - static inline Optional create(const T *y) { - return y ? Optional(*y) : Optional(); - } - - Optional &operator=(const T &y) { - Storage = y; - return *this; - } - Optional &operator=(const Optional &O) = default; - - void reset() { Storage.reset(); } - - const T *getPointer() const { - assert(Storage.hasVal); - return reinterpret_cast(Storage.storage.buffer); - } - T *getPointer() { - assert(Storage.hasVal); - return reinterpret_cast(Storage.storage.buffer); - } - const T &getValue() const LLVM_LVALUE_FUNCTION { return *getPointer(); } - T &getValue() LLVM_LVALUE_FUNCTION { return *getPointer(); } + const T* getPointer() const { assert(hasVal); return reinterpret_cast(storage.buffer); } + T* getPointer() { assert(hasVal); return reinterpret_cast(storage.buffer); } + const T& getValue() const LLVM_LVALUE_FUNCTION { assert(hasVal); return *getPointer(); } + T& getValue() LLVM_LVALUE_FUNCTION { assert(hasVal); return *getPointer(); } - explicit operator bool() const { return Storage.hasVal; } - bool hasValue() const { return Storage.hasVal; } + explicit operator bool() const { return hasVal; } + bool hasValue() const { return hasVal; } const T* operator->() const { return getPointer(); } T* operator->() { return getPointer(); } - const T &operator*() const LLVM_LVALUE_FUNCTION { return *getPointer(); } - T &operator*() LLVM_LVALUE_FUNCTION { return *getPointer(); } + const T& operator*() const LLVM_LVALUE_FUNCTION { assert(hasVal); return *getPointer(); } + T& operator*() LLVM_LVALUE_FUNCTION { assert(hasVal); return *getPointer(); } template constexpr T getValueOr(U &&value) const LLVM_LVALUE_FUNCTION { @@ -192,8 +142,8 @@ public: } #if LLVM_HAS_RVALUE_REFERENCE_THIS - T &&getValue() && { return std::move(*getPointer()); } - T &&operator*() && { return std::move(*getPointer()); } + T&& getValue() && { assert(hasVal); return std::move(*getPointer()); } + T&& operator*() && { assert(hasVal); return std::move(*getPointer()); } template T getValueOr(U &&value) && { diff --git a/llvm/unittests/ADT/OptionalTest.cpp b/llvm/unittests/ADT/OptionalTest.cpp index a9a37bf8..46d4fe0 100644 --- a/llvm/unittests/ADT/OptionalTest.cpp +++ b/llvm/unittests/ADT/OptionalTest.cpp @@ -518,14 +518,5 @@ TEST_F(OptionalTest, OperatorGreaterEqual) { CheckRelation(InequalityLhs, InequalityRhs, !IsLess); } -#if (__has_feature(is_trivially_copyable) && defined(_LIBCPP_VERSION)) || \ - (defined(__GNUC__) && __GNUC__ >= 5) -static_assert(std::is_trivially_copyable>::value, - "Should be trivially copyable"); -static_assert( - !std::is_trivially_copyable>::value, - "Shouldn't be trivially copyable"); -#endif - } // end anonymous namespace -- 2.7.4