Fix the copy constructor in Replaceable
authorPhil Willoughby <philwill@fb.com>
Sat, 29 Jul 2017 07:08:27 +0000 (00:08 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Sat, 29 Jul 2017 07:09:07 +0000 (00:09 -0700)
Summary:
Fix the copy constructor, and add the missing testcase which would have found
this problem.

Reviewed By: yfeldblum

Differential Revision: D5516628

fbshipit-source-id: 3e688c34f061511df5b68243111fecb83483d79d

folly/Replaceable.h
folly/test/ReplaceableTest.cpp

index 3021ed245d53f931d61d660dd2b640be82f99bc0..bdd5519e098d87cb2cb57142651082fc749ff08a 100644 (file)
@@ -307,7 +307,8 @@ struct copy_ctor_mixin<T, true> {
   copy_ctor_mixin() = default;
   inline copy_ctor_mixin(copy_ctor_mixin const& other) noexcept(
       std::is_nothrow_constructible<T, T const&>::value) {
-    ::new (reinterpret_cast<Replaceable<T>*>(this)->storage_) T(*other);
+    ::new (reinterpret_cast<Replaceable<T>*>(this)->storage_)
+        T(*reinterpret_cast<Replaceable<T> const&>(other));
   }
   copy_ctor_mixin(copy_ctor_mixin&&) = default;
   copy_ctor_mixin& operator=(copy_ctor_mixin&&) = default;
index 00eb01d9867ba38fa45de2d3bfeaf90ba853436d..6ab885095bcd1ebf8b6ca26836375d52d83b7381 100644 (file)
@@ -22,6 +22,7 @@ using namespace ::testing;
 using namespace ::folly;
 
 namespace {
+struct Basic {};
 struct alignas(128) BigAlign {};
 struct HasConst final {
   bool const b1;
@@ -81,6 +82,7 @@ using StaticAttributeTypes = ::testing::Types<
     float,
     double,
     char[11],
+    Basic,
     BigAlign,
     HasConst,
     HasRef,
@@ -250,6 +252,18 @@ TEST(ReplaceableTest, Basics) {
   EXPECT_TRUE(rHasConstB->b1);
 }
 
+TEST(ReplaceableTest, Constructors) {
+  Basic b{};
+  // From existing `T`
+  auto rBasicCopy1 = Replaceable<Basic>(b);
+  auto rBasicMove1 = Replaceable<Basic>(std::move(b));
+  // From existing `Replaceable<T>`
+  auto rBasicCopy2 = Replaceable<Basic>(rBasicCopy1);
+  auto rBasicMove2 = Replaceable<Basic>(std::move(rBasicMove1));
+  (void)rBasicCopy2;
+  (void)rBasicMove2;
+}
+
 TEST(ReplaceableTest, DestructsWhenExpected) {
   int i{0};
   {