Review comments

tokio-rs · May 5, 2020 · 3a4fad7 · 3a4fad7
1 parent f26d1a5
commit 3a4fad7
Showing 1 changed file with 48 additions and 25 deletions.
diff --git a/tokio/src/sync/rwlock.rs b/tokio/src/sync/rwlock.rs
@@ -195,6 +195,11 @@ impl<'a, T> RwLockReadGuard<'a, T> {
     /// used as `RwLockReadGuard::map(...)`. A method would interfere with
     /// methods of the same name on the contents of the locked data.
     ///
+    /// This is an asynchronous version of [`RwLockReadGuard::map`] from the
+    /// [parking_lot crate].
+    ///
+    /// [`RwLockReadGuard::map`]: https://docs.rs/lock_api/latest/lock_api/struct.RwLockReadGuard.html#method.map
+    /// [parking_lot crate]: https://crates.io/crates/parking_lot
     /// # Examples
     ///
     /// ```
@@ -211,14 +216,14 @@ impl<'a, T> RwLockReadGuard<'a, T> {
     /// # }
     /// ```
     #[inline]
-    pub fn map<U, F>(self, f: F) -> MappedRwLockReadGuard<'a, U>
+    pub fn map<U, F>(this: Self, f: F) -> MappedRwLockReadGuard<'a, U>
     where
         F: FnOnce(&T) -> &U,
     {
-        let data = f(unsafe { &*self.lock.c.get() });
-        let s = &self.lock.s;
+        let data = f(&*this) as *const U;
+        let s = &this.lock.s;
         // NB: Forget to avoid drop impl from being called.
-        mem::forget(self);
+        mem::forget(this);
         MappedRwLockReadGuard {
             s,
             data,
@@ -236,6 +241,12 @@ impl<'a, T> RwLockReadGuard<'a, T> {
     /// `RwLockReadGuard::map(..)`. A method would interfere with methods of the
     /// same name on the contents of the locked data.
     ///
+    /// This is an asynchronous version of [`RwLockReadGuard::try_map`] from the
+    /// [parking_lot crate].
+    ///
+    /// [`RwLockReadGuard::try_map`]: https://docs.rs/lock_api/latest/lock_api/struct.RwLockReadGuard.html#method.try_map
+    /// [parking_lot crate]: https://crates.io/crates/parking_lot
+    ///
     /// # Examples
     ///
     /// ```
@@ -252,17 +263,17 @@ impl<'a, T> RwLockReadGuard<'a, T> {
     /// # }
     /// ```
     #[inline]
-    pub fn try_map<U, F>(self, f: F) -> Result<MappedRwLockReadGuard<'a, U>, Self>
+    pub fn try_map<U, F>(this: Self, f: F) -> Result<MappedRwLockReadGuard<'a, U>, Self>
     where
         F: FnOnce(&T) -> Option<&U>,
     {
-        let data = match f(unsafe { &*self.lock.c.get() }) {
-            Some(data) => data,
-            None => return Err(self),
+        let data = match f(&*this) {
+            Some(data) => data as *const U,
+            None => return Err(this),
         };
-        let s = &self.lock.s;
+        let s = &this.lock.s;
         // NB: Forget to avoid drop impl from being called.
-        mem::forget(self);
+        mem::forget(this);
         Ok(MappedRwLockReadGuard {
             s,
             data,
@@ -317,6 +328,12 @@ impl<'a, T> RwLockWriteGuard<'a, T> {
     /// `RwLockWriteGuard::map(..)`. A method would interfere with methods of
     /// the same name on the contents of the locked data.
     ///
+    /// This is an asynchronous version of [`RwLockWriteGuard::map`] from the
+    /// [parking_lot crate].
+    ///
+    /// [`RwLockWriteGuard::map`]: https://docs.rs/lock_api/latest/lock_api/struct.RwLockWriteGuard.html#method.map
+    /// [parking_lot crate]: https://crates.io/crates/parking_lot
+    ///
     /// # Examples
     ///
     /// ```
@@ -338,14 +355,14 @@ impl<'a, T> RwLockWriteGuard<'a, T> {
     /// # }
     /// ```
     #[inline]
-    pub fn map<U, F>(self, f: F) -> MappedRwLockWriteGuard<'a, U>
+    pub fn map<U, F>(mut this: Self, f: F) -> MappedRwLockWriteGuard<'a, U>
     where
         F: FnOnce(&mut T) -> &mut U,
     {
-        let data = f(unsafe { &mut *self.lock.c.get() });
-        let s = &self.lock.s;
+        let data = f(&mut *this) as *mut U;
+        let s = &this.lock.s;
         // NB: Forget to avoid drop impl from being called.
-        mem::forget(self);
+        mem::forget(this);
         MappedRwLockWriteGuard {
             s,
             data,
@@ -364,6 +381,12 @@ impl<'a, T> RwLockWriteGuard<'a, T> {
     /// used as `RwLockWriteGuard::map(...)`. A method would interfere with
     /// methods of the same name on the contents of the locked data.
     ///
+    /// This is an asynchronous version of [`RwLockWriteGuard::try_map`] from the
+    /// [parking_lot crate].
+    ///
+    /// [`RwLockWriteGuard::try_map`]: https://docs.rs/lock_api/latest/lock_api/struct.RwLockWriteGuard.html#method.try_map
+    /// [parking_lot crate]: https://crates.io/crates/parking_lot
+    ///
     /// # Examples
     ///
     /// ```
@@ -385,17 +408,17 @@ impl<'a, T> RwLockWriteGuard<'a, T> {
     /// # }
     /// ```
     #[inline]
-    pub fn try_map<U, F>(self, f: F) -> Result<MappedRwLockWriteGuard<'a, U>, Self>
+    pub fn try_map<U, F>(mut this: Self, f: F) -> Result<MappedRwLockWriteGuard<'a, U>, Self>
     where
         F: FnOnce(&mut T) -> Option<&mut U>,
     {
-        let data = match f(unsafe { &mut *self.lock.c.get() }) {
-            Some(data) => data,
-            None => return Err(self),
+        let data = match f(&mut *this) {
+            Some(data) => data as *mut U,
+            None => return Err(this),
         };
-        let s = &self.lock.s;
+        let s = &this.lock.s;
         // NB: Forget to avoid drop impl from being called.
-        mem::forget(self);
+        mem::forget(this);
         Ok(MappedRwLockWriteGuard {
             s,
             data,
@@ -468,12 +491,12 @@ unsafe impl<T> Sync for RwLock<T> where T: Send + Sync {}
 unsafe impl<'a, T> Sync for RwLockReadGuard<'a, T> where T: Send + Sync {}
 unsafe impl<'a, T> Sync for RwLockWriteGuard<'a, T> where T: Send + Sync {}
 // NB: These impls need to be explicit since we're storing a raw pointer.
-// Safety: Stores a raw pointer to `T`, so if `T` is `Sync`, so is the lock
-// guard over `T`.
+// Safety: Stores a raw pointer to `T`, so if `T` is `Sync`, the lock guard over
+// `T` is `Send`.
 unsafe impl<'a, T> Send for MappedRwLockReadGuard<'a, T> where T: Sync {}
-// Safety: Stores a raw pointer to `T`, so if `T` is `Sync`, so is the lock
-// guard over `T` - but since this is also provides mutable access, we need to
-// make sure that the type is `Send` since its value can be sent across thread
+// Safety: Stores a raw pointer to `T`, so if `T` is `Sync`, the lock guard over
+// `T` is `Send` - but since this is also provides mutable access, we need to
+// make sure that `T` is `Send` since its value can be sent across thread
 // boundaries.
 unsafe impl<'a, T> Send for MappedRwLockWriteGuard<'a, T> where T: Send + Sync {}