Commit 4a3cdfdc authored by Maxime Boucher's avatar Maxime Boucher Committed by Facebook Github Bot

Synchronized: disable operator= when the type isn't copy assignable

Summary: The value of std::is_copy_assignable<folly::Synchronized<T>>::value is incorrect when T isn't copy assignable. As a result, it isn't possible to use SFINAE to properly select a function when the base type is a folly::Synchronized. This diff selectively deletes the copy constructor and copy assignment operator when the underlying type T isn't copyable. This is most useful in the case of folly::Synchronized<std::unique_ptr<...>>

Reviewed By: yfeldblum

Differential Revision: D4203081

fbshipit-source-id: 1e811f9e52db26c23b1c6f1907bac9e2854ddb9d
parent adbc74b7
...@@ -433,6 +433,9 @@ struct Synchronized : public SynchronizedBase< ...@@ -433,6 +433,9 @@ struct Synchronized : public SynchronizedBase<
static constexpr bool nxMoveCtor{ static constexpr bool nxMoveCtor{
std::is_nothrow_move_constructible<T>::value}; std::is_nothrow_move_constructible<T>::value};
// used to disable copy construction and assignment
class NonImplementedType;
public: public:
using LockedPtr = typename Base::LockedPtr; using LockedPtr = typename Base::LockedPtr;
using ConstLockedPtr = typename Base::ConstLockedPtr; using ConstLockedPtr = typename Base::ConstLockedPtr;
...@@ -453,7 +456,11 @@ struct Synchronized : public SynchronizedBase< ...@@ -453,7 +456,11 @@ struct Synchronized : public SynchronizedBase<
* Note that the copy constructor may throw because it acquires a lock in * Note that the copy constructor may throw because it acquires a lock in
* the contextualRLock() method * the contextualRLock() method
*/ */
Synchronized(const Synchronized& rhs) /* may throw */ public:
/* implicit */ Synchronized(typename std::conditional<
std::is_copy_constructible<T>::value,
const Synchronized&,
NonImplementedType>::type rhs) /* may throw */
: Synchronized(rhs, rhs.contextualRLock()) {} : Synchronized(rhs, rhs.contextualRLock()) {}
/** /**
...@@ -495,7 +502,10 @@ struct Synchronized : public SynchronizedBase< ...@@ -495,7 +502,10 @@ struct Synchronized : public SynchronizedBase<
* mutex. It locks the two objects in ascending order of their * mutex. It locks the two objects in ascending order of their
* addresses. * addresses.
*/ */
Synchronized& operator=(const Synchronized& rhs) { Synchronized& operator=(typename std::conditional<
std::is_copy_assignable<T>::value,
const Synchronized&,
NonImplementedType>::type rhs) {
if (this == &rhs) { if (this == &rhs) {
// Self-assignment, pass. // Self-assignment, pass.
} else if (this < &rhs) { } else if (this < &rhs) {
......
...@@ -395,6 +395,22 @@ TEST_F(SynchronizedLockTest, NestedSyncUnSync2) { ...@@ -395,6 +395,22 @@ TEST_F(SynchronizedLockTest, NestedSyncUnSync2) {
EXPECT_EQ((CountPair{4, 4}), FakeMutex::getLockUnlockCount()); EXPECT_EQ((CountPair{4, 4}), FakeMutex::getLockUnlockCount());
} }
TEST_F(SynchronizedLockTest, TestCopyConstructibleValues) {
struct NonCopyConstructible {
NonCopyConstructible(const NonCopyConstructible&) = delete;
NonCopyConstructible& operator=(const NonCopyConstructible&) = delete;
};
struct CopyConstructible {};
EXPECT_FALSE(std::is_copy_constructible<
folly::Synchronized<NonCopyConstructible>>::value);
EXPECT_FALSE(std::is_copy_assignable<
folly::Synchronized<NonCopyConstructible>>::value);
EXPECT_TRUE(std::is_copy_constructible<
folly::Synchronized<CopyConstructible>>::value);
EXPECT_TRUE(
std::is_copy_assignable<folly::Synchronized<CopyConstructible>>::value);
}
TEST_F(SynchronizedLockTest, UpgradableLocking) { TEST_F(SynchronizedLockTest, UpgradableLocking) {
folly::Synchronized<int, FakeAllPowerfulAssertingMutex> sync; folly::Synchronized<int, FakeAllPowerfulAssertingMutex> sync;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment