[llvm] r354200 - Revert r354199: Make Optional<T> Trivially Copyable when T is trivially copyable

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 18 12:06:07 PST 2019


It can be helpful to mention the motivation for a revert in the commit
message so others can follow along - I know this has had a few issues with
buildbots and such - could you link to/quote the failure you were
addressing by reverting this patch?

On Sat, Feb 16, 2019 at 1:46 AM Serge Guelton via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: serge_sans_paille
> Date: Sat Feb 16 01:47:23 2019
> New Revision: 354200
>
> URL: http://llvm.org/viewvc/llvm-project?rev=354200&view=rev
> Log:
> Revert r354199: Make Optional<T> Trivially Copyable when T is trivially
> copyable
>
> 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=354200&r1=354199&r2=354200&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/Optional.h (original)
> +++ llvm/trunk/include/llvm/ADT/Optional.h Sat Feb 16 01:47:23 2019
> @@ -108,59 +108,6 @@ template <typename T, bool = is_triviall
>      return reinterpret_cast<const T *>(storage.buffer);
>    }
>  };
> -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(const OptionalStorage &O)  = default;
> -  OptionalStorage(T &&y) : hasVal(true) {
> -    new (storage.buffer) T(std::forward<T>(y));
> -  }
> -  OptionalStorage(OptionalStorage &&O) = default;
> -
> -  OptionalStorage &operator=(T &&y) {
> -    if (hasVal)
> -      *getPointer() = std::move(y);
> -    else {
> -      new (storage.buffer) T(std::move(y));
> -      hasVal = true;
> -    }
> -    return *this;
> -  }
> -  OptionalStorage &operator=(OptionalStorage &&O) = default;
> -
> -  OptionalStorage &operator=(const T &y) {
> -    if (hasVal)
> -      *getPointer() = y;
> -    else {
> -      new (storage.buffer) T(y);
> -      hasVal = true;
> -    }
> -    return *this;
> -  }
> -  OptionalStorage &operator=(const OptionalStorage &O) = default;
> -
> -  ~OptionalStorage() = default;
> -
> -  void reset() {
> -    if (hasVal) {
> -      (*getPointer()).~T();
> -      hasVal = false;
> -    }
> -  }
> -
> -  T *getPointer() {
> -    assert(hasVal);
> -    return reinterpret_cast<T *>(storage.buffer);
> -  }
> -  const T *getPointer() const {
> -    assert(hasVal);
> -    return reinterpret_cast<const T *>(storage.buffer);
> -  }
> -};
>
>  } // namespace optional_detail
>
>
> Modified: llvm/trunk/unittests/ADT/OptionalTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/OptionalTest.cpp?rev=354200&r1=354199&r2=354200&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/ADT/OptionalTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/OptionalTest.cpp Sat Feb 16 01:47:23 2019
> @@ -14,15 +14,8 @@
>
>  #include <array>
>
> -
>  using namespace llvm;
>
> -static_assert(llvm::is_trivially_copyable<Optional<int>>::value,
> -              "trivially copyable");
> -
> -static_assert(llvm::is_trivially_copyable<Optional<std::array<int,
> 3>>>::value,
> -              "trivially copyable");
> -
>  namespace {
>
>  struct NonDefaultConstructible {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://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/20190218/7d53ca94/attachment.html>


More information about the llvm-commits mailing list