[llvm] r317019 - [ADT] Split optional to only include copy mechanics and dtor for non-trivial types.
NAKAMURA Takumi via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 17 19:04:05 PST 2018
Okay, then please watch builders not to fail.
2018年1月18日(木) 11:30 Eric Fiselier <eric at efcs.ca>:
> @Nakamura,
>
> Do you recall more about the build errors you were seeing?
>
> I can't reproduce with GCC 4.8.5 building LLVM and Clang.
>
> Maybe we could try re-committing this patch? At least to rediscover the
> exact error.
>
> /Eric
>
> On Wed, Nov 1, 2017 at 4:56 AM, NAKAMURA Takumi via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Excuse me, I have reverted it in r317077.
>> It confused clang with g++-4.8. See;
>>
>> http://bb9.pgr.jp/#/builders/2/builds/335
>>
>> http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/6326
>>
>> http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/10853
>>
>>
>> On Wed, Nov 1, 2017 at 3:36 AM Benjamin Kramer via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: d0k
>>> Date: Tue Oct 31 11:35:54 2017
>>> New Revision: 317019
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=317019&view=rev
>>> Log:
>>> [ADT] Split optional to only include copy mechanics and dtor for
>>> non-trivial types.
>>>
>>> This makes uses of Optional more transparent to the compiler (and
>>> clang-tidy) and generates slightly smaller code.
>>>
>>> Modified:
>>> llvm/trunk/include/llvm/ADT/Optional.h
>>> llvm/trunk/unittests/ADT/OptionalTest.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/ADT/Optional.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Optional.h?rev=317019&r1=317018&r2=317019&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/ADT/Optional.h (original)
>>> +++ llvm/trunk/include/llvm/ADT/Optional.h Tue Oct 31 11:35:54 2017
>>> @@ -27,114 +27,164 @@
>>>
>>> namespace llvm {
>>>
>>> -template<typename T>
>>> -class Optional {
>>> +namespace optional_detail {
>>> +/// Storage for any type.
>>> +template <typename T, bool IsPodLike> struct OptionalStorage {
>>> AlignedCharArrayUnion<T> storage;
>>> bool hasVal = false;
>>>
>>> -public:
>>> - using value_type = T;
>>> -
>>> - Optional(NoneType) {}
>>> - explicit Optional() {}
>>> -
>>> - Optional(const T &y) : hasVal(true) {
>>> - new (storage.buffer) T(y);
>>> - }
>>> + OptionalStorage() = default;
>>>
>>> - Optional(const Optional &O) : hasVal(O.hasVal) {
>>> + 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);
>>> + new (storage.buffer) T(*O.getPointer());
>>> }
>>> -
>>> - Optional(T &&y) : hasVal(true) {
>>> + OptionalStorage(T &&y) : hasVal(true) {
>>> new (storage.buffer) T(std::forward<T>(y));
>>> }
>>> -
>>> - Optional(Optional<T> &&O) : hasVal(O) {
>>> - if (O) {
>>> - new (storage.buffer) T(std::move(*O));
>>> + OptionalStorage(OptionalStorage &&O) : hasVal(O.hasVal) {
>>> + if (O.hasVal) {
>>> + new (storage.buffer) T(std::move(*O.getPointer()));
>>> O.reset();
>>> }
>>> }
>>>
>>> - ~Optional() {
>>> - reset();
>>> - }
>>> -
>>> - Optional &operator=(T &&y) {
>>> + OptionalStorage &operator=(T &&y) {
>>> if (hasVal)
>>> - **this = std::move(y);
>>> + *getPointer() = std::move(y);
>>> else {
>>> new (storage.buffer) T(std::move(y));
>>> hasVal = true;
>>> }
>>> return *this;
>>> }
>>> -
>>> - Optional &operator=(Optional &&O) {
>>> - if (!O)
>>> + OptionalStorage &operator=(OptionalStorage &&O) {
>>> + if (!O.hasVal)
>>> reset();
>>> else {
>>> - *this = std::move(*O);
>>> + *this = std::move(*O.getPointer());
>>> O.reset();
>>> }
>>> return *this;
>>> }
>>>
>>> - /// Create a new object by constructing it in place with the given
>>> arguments.
>>> - template<typename ...ArgTypes>
>>> - void emplace(ArgTypes &&...Args) {
>>> - reset();
>>> - hasVal = true;
>>> - new (storage.buffer) T(std::forward<ArgTypes>(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.
>>> - Optional &operator=(const T &y) {
>>> + OptionalStorage &operator=(const T &y) {
>>> if (hasVal)
>>> - **this = y;
>>> + *getPointer() = y;
>>> else {
>>> new (storage.buffer) T(y);
>>> hasVal = true;
>>> }
>>> return *this;
>>> }
>>> -
>>> - Optional &operator=(const Optional &O) {
>>> - if (!O)
>>> + OptionalStorage &operator=(const OptionalStorage &O) {
>>> + if (!O.hasVal)
>>> reset();
>>> else
>>> - *this = *O;
>>> + *this = *O.getPointer();
>>> return *this;
>>> }
>>>
>>> + ~OptionalStorage() { reset(); }
>>> +
>>> void reset() {
>>> if (hasVal) {
>>> - (**this).~T();
>>> + (*getPointer()).~T();
>>> hasVal = false;
>>> }
>>> }
>>>
>>> - const T* getPointer() const { assert(hasVal); return
>>> reinterpret_cast<const T*>(storage.buffer); }
>>> - T* getPointer() { assert(hasVal); return
>>> reinterpret_cast<T*>(storage.buffer); }
>>> - const T& getValue() const LLVM_LVALUE_FUNCTION { assert(hasVal);
>>> return *getPointer(); }
>>> - T& getValue() LLVM_LVALUE_FUNCTION { assert(hasVal); return
>>> *getPointer(); }
>>> + T *getPointer() {
>>> + assert(hasVal);
>>> + return reinterpret_cast<T *>(storage.buffer);
>>> + }
>>> + const T *getPointer() const {
>>> + assert(hasVal);
>>> + return reinterpret_cast<const T *>(storage.buffer);
>>> + }
>>> +};
>>> +
>>> +/// Storage for trivially copyable types only.
>>> +template <typename T> struct OptionalStorage<T, true> {
>>> + AlignedCharArrayUnion<T> 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 <typename T> class Optional {
>>> + optional_detail::OptionalStorage<T, isPodLike<T>::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<T>(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 <typename... ArgTypes> void emplace(ArgTypes &&... Args) {
>>> + reset();
>>> + Storage.hasVal = true;
>>> + new (getPointer()) T(std::forward<ArgTypes>(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<const T *>(Storage.storage.buffer);
>>> + }
>>> + T *getPointer() {
>>> + assert(Storage.hasVal);
>>> + return reinterpret_cast<T *>(Storage.storage.buffer);
>>> + }
>>> + const T &getValue() const LLVM_LVALUE_FUNCTION { return
>>> *getPointer(); }
>>> + T &getValue() LLVM_LVALUE_FUNCTION { return *getPointer(); }
>>>
>>> - explicit operator bool() const { return hasVal; }
>>> - bool hasValue() const { return hasVal; }
>>> + explicit operator bool() const { return Storage.hasVal; }
>>> + bool hasValue() const { return Storage.hasVal; }
>>> const T* operator->() const { return getPointer(); }
>>> T* operator->() { return getPointer(); }
>>> - const T& operator*() const LLVM_LVALUE_FUNCTION { assert(hasVal);
>>> return *getPointer(); }
>>> - T& operator*() LLVM_LVALUE_FUNCTION { assert(hasVal); return
>>> *getPointer(); }
>>> + const T &operator*() const LLVM_LVALUE_FUNCTION { return
>>> *getPointer(); }
>>> + T &operator*() LLVM_LVALUE_FUNCTION { return *getPointer(); }
>>>
>>> template <typename U>
>>> constexpr T getValueOr(U &&value) const LLVM_LVALUE_FUNCTION {
>>> @@ -142,8 +192,8 @@ public:
>>> }
>>>
>>> #if LLVM_HAS_RVALUE_REFERENCE_THIS
>>> - T&& getValue() && { assert(hasVal); return std::move(*getPointer()); }
>>> - T&& operator*() && { assert(hasVal); return std::move(*getPointer());
>>> }
>>> + T &&getValue() && { return std::move(*getPointer()); }
>>> + T &&operator*() && { return std::move(*getPointer()); }
>>>
>>> template <typename U>
>>> T getValueOr(U &&value) && {
>>>
>>> Modified: llvm/trunk/unittests/ADT/OptionalTest.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/OptionalTest.cpp?rev=317019&r1=317018&r2=317019&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/unittests/ADT/OptionalTest.cpp (original)
>>> +++ llvm/trunk/unittests/ADT/OptionalTest.cpp Tue Oct 31 11:35:54 2017
>>> @@ -518,5 +518,14 @@ TEST_F(OptionalTest, OperatorGreaterEqua
>>> CheckRelation<GreaterEqual>(InequalityLhs, InequalityRhs, !IsLess);
>>> }
>>>
>>> +#if (__has_feature(is_trivially_copyable) && defined(_LIBCPP_VERSION))
>>> || \
>>> + (defined(__GNUC__) && __GNUC__ >= 5)
>>> +static_assert(std::is_trivially_copyable<Optional<int>>::value,
>>> + "Should be trivially copyable");
>>> +static_assert(
>>> +
>>> !std::is_trivially_copyable<Optional<NonDefaultConstructible>>::value,
>>> + "Shouldn't be trivially copyable");
>>> +#endif
>>> +
>>> } // end anonymous namespace
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180118/403b14cf/attachment.html>
More information about the llvm-commits
mailing list