Remove/make private the default ***Holder constructor to allow compile time detection...

Summary: Avoid unintended C++ stuff - this is not the same as the "fleeting rvalue" - this change might break the build but I will fix it

Reviewed By: yfeldblum

Differential Revision: D4814104

fbshipit-source-id: 058a0eac44893c573062fcf5665d4fd022ee64a0

diff --git a/folly/RWSpinLock.h b/folly/RWSpinLock.h
index 63dac2f5dbc294efa737315b1fbd34a0c243583b..cc4f480a4e29b123cafd16af71963bbd3b63f03e 100644 (file)
--- a/folly/RWSpinLock.h
+++ b/folly/RWSpinLock.h
@@ -307,7 +307,7 @@ class RWSpinLock {
 
   class ReadHolder {
    public:
-    explicit ReadHolder(RWSpinLock* lock = nullptr) : lock_(lock) {
+    explicit ReadHolder(RWSpinLock* lock) : lock_(lock) {
       if (lock_) lock_->lock_shared();
     }
 
@@ -360,7 +360,7 @@ class RWSpinLock {
 
   class UpgradedHolder {
    public:
-    explicit UpgradedHolder(RWSpinLock* lock = nullptr) : lock_(lock) {
+    explicit UpgradedHolder(RWSpinLock* lock) : lock_(lock) {
       if (lock_) lock_->lock_upgrade();
     }
 
@@ -409,7 +409,7 @@ class RWSpinLock {
 
   class WriteHolder {
    public:
-    explicit WriteHolder(RWSpinLock* lock = nullptr) : lock_(lock) {
+    explicit WriteHolder(RWSpinLock* lock) : lock_(lock) {
       if (lock_) lock_->lock();
     }
 
@@ -692,8 +692,7 @@ class RWTicketSpinLockT {
     ReadHolder(ReadHolder const&) = delete;
     ReadHolder& operator=(ReadHolder const&) = delete;
 
-    explicit ReadHolder(RWSpinLock *lock = nullptr) :
-      lock_(lock) {
+    explicit ReadHolder(RWSpinLock* lock) : lock_(lock) {
       if (lock_) lock_->lock_shared();
     }
 
@@ -732,7 +731,7 @@ class RWTicketSpinLockT {
     WriteHolder(WriteHolder const&) = delete;
     WriteHolder& operator=(WriteHolder const&) = delete;
 
-    explicit WriteHolder(RWSpinLock *lock = nullptr) : lock_(lock) {
+    explicit WriteHolder(RWSpinLock* lock) : lock_(lock) {
       if (lock_) lock_->lock();
     }
     explicit WriteHolder(RWSpinLock &lock) : lock_ (&lock) {

diff --git a/folly/SharedMutex.h b/folly/SharedMutex.h
index 49dd29c64a3c65929e93ea5dad2e4be85cb386e7..571af4a7e2749342c5b1a9ea811271bc550ac6a7 100644 (file)
--- a/folly/SharedMutex.h
+++ b/folly/SharedMutex.h
@@ -1133,10 +1133,15 @@ class SharedMutexImpl {
 
  public:
   class ReadHolder {
-   public:
     ReadHolder() : lock_(nullptr) {}
 
-    explicit ReadHolder(const SharedMutexImpl* lock) : ReadHolder(*lock) {}
+   public:
+    explicit ReadHolder(const SharedMutexImpl* lock)
+        : lock_(const_cast<SharedMutexImpl*>(lock)) {
+      if (lock_) {
+        lock_->lock_shared(token_);
+      }
+    }
 
     explicit ReadHolder(const SharedMutexImpl& lock)
         : lock_(const_cast<SharedMutexImpl*>(&lock)) {
@@ -1190,10 +1195,14 @@ class SharedMutexImpl {
   };
 
   class UpgradeHolder {
-   public:
     UpgradeHolder() : lock_(nullptr) {}
 
-    explicit UpgradeHolder(SharedMutexImpl* lock) : UpgradeHolder(*lock) {}
+   public:
+    explicit UpgradeHolder(SharedMutexImpl* lock) : lock_(lock) {
+      if (lock_) {
+        lock_->lock_upgrade();
+      }
+    }
 
     explicit UpgradeHolder(SharedMutexImpl& lock) : lock_(&lock) {
       lock_->lock_upgrade();
@@ -1236,10 +1245,14 @@ class SharedMutexImpl {
   };
 
   class WriteHolder {
-   public:
     WriteHolder() : lock_(nullptr) {}
 
-    explicit WriteHolder(SharedMutexImpl* lock) : WriteHolder(*lock) {}
+   public:
+    explicit WriteHolder(SharedMutexImpl* lock) : lock_(lock) {
+      if (lock_) {
+        lock_->lock();
+      }
+    }
 
     explicit WriteHolder(SharedMutexImpl& lock) : lock_(&lock) {
       lock_->lock();

diff --git a/folly/detail/ThreadLocalDetail.cpp b/folly/detail/ThreadLocalDetail.cpp
index 12a02e461bbd738c4f8a551d6fd1f2b2ebe877c8..67eb056534042d2b0405dc74ee1f8057704d8e52 100644 (file)
--- a/folly/detail/ThreadLocalDetail.cpp
+++ b/folly/detail/ThreadLocalDetail.cpp
@@ -45,7 +45,7 @@ void StaticMetaBase::onThreadExit(void* ptr) {
   };
 
   {
-    SharedMutex::ReadHolder rlock;
+    SharedMutex::ReadHolder rlock(nullptr);
     if (meta.strict_) {
       rlock = SharedMutex::ReadHolder(meta.accessAllThreadsLock_);
     }
@@ -103,7 +103,7 @@ void StaticMetaBase::destroy(EntryID* ent) {
     std::vector<ElementWrapper> elements;
 
     {
-      SharedMutex::WriteHolder wlock;
+      SharedMutex::WriteHolder wlock(nullptr);
       if (meta.strict_) {
         /*
          * In strict mode, the logic guarantees per-thread instances are

diff --git a/folly/test/SharedMutexTest.cpp b/folly/test/SharedMutexTest.cpp
index d18e3b6e263592fa87ce91801b898c74ba71877d..9e04edc76a0603c37c0475699c0cc4d890d2b287 100644 (file)
--- a/folly/test/SharedMutexTest.cpp
+++ b/folly/test/SharedMutexTest.cpp
@@ -98,7 +98,7 @@ void runBasicHoldersTest() {
     EXPECT_FALSE(lock.try_lock_shared(token));
 
     // move ownership to another write holder via assign operator
-    typename Lock::WriteHolder holder3;
+    typename Lock::WriteHolder holder3(nullptr);
     holder3 = std::move(holder2);
     EXPECT_FALSE(lock.try_lock());
     EXPECT_FALSE(lock.try_lock_shared(token));