[llvm] r354126 - Speculatively revert r354051 "Recommit Optional specialization for trivially copyable types"

[Hans Wennborg via llvm-commits]

Author: hans
Date: Fri Feb 15 04:20:33 2019
New Revision: 354126

URL: http://llvm.org/viewvc/llvm-project?rev=354126&view=rev
Log:
Speculatively revert r354051 "Recommit Optional specialization for trivially copyable types"
and
r354055 "Optional specialization for trivially copyable types, part2"

These are suspected to cause Clang to get miscompiled on Ubuntu 14.04
(Trusty) which uses GCC 4.8.4. Reverting for an hour to see if this
helps. See llvm-commits thread.

> Recommit Optional specialization for trivially copyable types
>
> Unfortunately the original code gets misscompiled by GCC (at least 8.1),
> this is a tentative workaround using std::memcpy instead of inplace new
> for trivially copyable types. I'll revert if it breaks.
>
> Original revision: https://reviews.llvm.org/D57097

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=354126&r1=354125&r2=354126&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/Optional.h (original)
+++ llvm/trunk/include/llvm/ADT/Optional.h Fri Feb 15 04:20:33 2019
@@ -21,7 +21,6 @@
 #include "llvm/Support/type_traits.h"
 #include <algorithm>
 #include <cassert>
-#include <cstring>
 #include <new>
 #include <utility>
 
@@ -110,50 +109,6 @@ template <typename T, bool = is_triviall
   }
 };
 
-template <typename T> struct OptionalStorage<T, true> {
-  AlignedCharArrayUnion<T> storage;
-  bool hasVal = false;
-
-  OptionalStorage() = default;
-
-  OptionalStorage(const T &y) : hasVal(true) {
-    std::memcpy(storage.buffer, reinterpret_cast<char const *>(&y), sizeof(T));
-  }
-  OptionalStorage(const OptionalStorage &O) = default;
-  OptionalStorage(T &&y) : hasVal(true) {
-    std::memcpy(storage.buffer, reinterpret_cast<char*>(&y), sizeof(T));
-  }
-
-  OptionalStorage(OptionalStorage &&O) = default;
-
-  OptionalStorage &operator=(T &&y) {
-    hasVal = true;
-    std::memcpy(storage.buffer, reinterpret_cast<char*>(&y), sizeof(T));
-    return *this;
-  }
-  OptionalStorage &operator=(OptionalStorage &&O) = default;
-
-  OptionalStorage &operator=(const T &y) {
-    hasVal = true;
-    std::memcpy(storage.buffer, reinterpret_cast<char const*>(&y), sizeof(T));
-    return *this;
-  }
-  OptionalStorage &operator=(const OptionalStorage &O) = default;
-
-  ~OptionalStorage() = default;
-
-  T *getPointer() {
-    assert(hasVal);
-    return reinterpret_cast<T *>(storage.buffer);
-  }
-  const T *getPointer() const {
-    assert(hasVal);
-    return reinterpret_cast<const T *>(storage.buffer);
-  }
-
-  void reset() { hasVal = false; }
-};
-
 } // namespace optional_detail
 
 template <typename T> class Optional {

Modified: llvm/trunk/unittests/ADT/OptionalTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/OptionalTest.cpp?rev=354126&r1=354125&r2=354126&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/OptionalTest.cpp (original)
+++ llvm/trunk/unittests/ADT/OptionalTest.cpp Fri Feb 15 04:20:33 2019
@@ -18,12 +18,6 @@ using namespace llvm;
 
 namespace {
 
-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");
-
 struct NonDefaultConstructible {
   static unsigned CopyConstructions;
   static unsigned Destructions;
@@ -51,10 +45,6 @@ unsigned NonDefaultConstructible::CopyCo
 unsigned NonDefaultConstructible::Destructions = 0;
 unsigned NonDefaultConstructible::CopyAssignments = 0;
 
-static_assert(
-    !llvm::is_trivially_copyable<Optional<NonDefaultConstructible>>::value,
-    "not trivially copyable");
-
 // Test fixture
 class OptionalTest : public testing::Test {
 };
@@ -213,10 +203,6 @@ struct MultiArgConstructor {
 };
 unsigned MultiArgConstructor::Destructions = 0;
 
-static_assert(
-    !llvm::is_trivially_copyable<Optional<MultiArgConstructor>>::value,
-    "not trivially copyable");
-
 TEST_F(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional<MultiArgConstructor> A;
@@ -264,9 +250,6 @@ unsigned MoveOnly::MoveConstructions = 0
 unsigned MoveOnly::Destructions = 0;
 unsigned MoveOnly::MoveAssignments = 0;
 
-static_assert(!llvm::is_trivially_copyable<Optional<MoveOnly>>::value,
-              "not trivially copyable");
-
 TEST_F(OptionalTest, MoveOnlyNull) {
   MoveOnly::ResetCounts();
   Optional<MoveOnly> O;
@@ -368,9 +351,6 @@ private:
 unsigned Immovable::Constructions = 0;
 unsigned Immovable::Destructions = 0;
 
-static_assert(!llvm::is_trivially_copyable<Optional<Immovable>>::value,
-              "not trivially copyable");
-
 TEST_F(OptionalTest, ImmovableEmplace) {
   Optional<Immovable> A;
   Immovable::ResetCounts();