Implement `Debug` for `RwLock` and `arc::Weak` #22573

nwin · 2015-02-20T12:12:10Z

Implements Debug for RwLock and arc::Weak in the same way it is implemented for rc::Weak (basically copy & paste).

The lack of this implementation prevents the automatic implementation of Debug for structs containing members of these types.

rust-highfive · 2015-02-20T12:12:28Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @huonw (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

huonw · 2015-02-20T12:18:39Z

Maybe you could add an implementation for Mutex too?

I wonder if this and RwLock could mirror RefCell by calling try_lock or try_read (respectively) to print the data if a handle can be acquired, otherwise falling back to locked or something:

use std::cell::RefCell;
fn main() {
    let cell = RefCell::new(1);
    println!("{:?}", cell);
    let _b = cell.borrow_mut();
    println!("{:?}", cell);
}

RefCell { value: 1 }
RefCell { <borrowed> }

huonw · 2015-02-20T12:19:03Z

Thanks for the patch!

nwin · 2015-02-20T12:27:07Z

The same could also work for (a)rc::Weak. I’m just wondering if it is a good idea to acquire these resources just for Debug-printing.

nwin · 2015-02-20T12:50:50Z

I added Debug for Mutex and made the changes you suggested to RwLock. I didn’t do that for Weak (possible cycles).

alexcrichton · 2015-02-20T16:43:37Z

src/libstd/sync/mutex.rs

+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self.try_lock() {
+            Ok(guard) => write!(f, "Mutex {{ data: {:?} }}", *guard),
+            Err(_) => write!(f, "Mutex {{ <locked> }}")


This could also test for each variant of Err as one case is that the mutex is already locked but another case is that the mutex is poisoned (but the lock was successfully acquired).

alexcrichton · 2015-02-25T00:12:12Z

@nwin looks good to me, thanks! Could you squash the commits together as well?

Also feel free to comment on the PR whenever you update it, unfortunately github doesn't send out notifications for force-pushes.

nwin · 2015-02-25T07:56:16Z

Ok, done.

alexcrichton · 2015-02-25T08:04:54Z

@bors: r+ 8c778f2

Thanks!

bors · 2015-02-25T08:19:49Z

⌛ Testing commit 8c778f2 with merge d885154...

bors · 2015-02-25T08:30:02Z

💔 Test failed - auto-linux-64-x-android-t

Manishearth · 2015-02-25T09:09:02Z

@bors: retry

bors · 2015-02-25T09:22:29Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-02-25T09:22:37Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-02-25T09:22:37Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-02-25T09:25:14Z

💔 Test failed - auto-win-64-nopt-t

Manishearth · 2015-02-25T09:31:23Z

@bors: retry

(Please ignore the failures for now)

bors · 2015-02-25T10:58:37Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-02-25T10:58:37Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-02-25T10:58:37Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-02-25T10:58:37Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-02-25T11:01:16Z

💔 Test failed - auto-win-32-opt

alexcrichton · 2015-02-25T17:43:10Z

@bors: retry

bors · 2015-02-25T18:07:54Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-02-25T18:13:32Z

💔 Test failed - auto-win-64-nopt-t

alexcrichton · 2015-02-25T18:26:25Z

@bors: retry

bors · 2015-02-25T20:31:36Z

⚡ Previous build results are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt...

bors · 2015-02-25T20:32:57Z

💔 Test failed - auto-linux-64-x-android-t

nwin · 2015-02-25T20:33:05Z

bors seems to be in a mood today… ;)

Manishearth · 2015-02-25T20:35:30Z

Oh, no, that's just me manually cancellig (forcing) the build for the rollup. Ignore all bors comments (you're in the rollup) :)

Manishearth · 2015-02-25T23:10:31Z

@bors: retry

Manishearth · 2015-02-25T23:10:46Z

So sorry, thought I had added this to the rollup.

bors · 2015-02-26T04:13:08Z

⌛ Testing commit 8c778f2 with merge 044ee8b...

bors · 2015-02-26T04:31:57Z

💔 Test failed - auto-mac-64-opt

bors · 2015-02-26T04:31:57Z

💔 Test failed - auto-mac-32-opt

Manishearth · 2015-02-26T05:56:42Z

shakes fist at stage2

/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/mutex.rs:262:17: 262:27 warning: pattern binding `WouldBlock` is named the same as one of the variants of the type `sync::poison::TryLockError<sync::mutex::MutexGuard<'_, T>>` [E0170]
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/mutex.rs:262             Err(WouldBlock) => write!(f, "Mutex {{ <locked> }}")
                                                                                                            ^~~~~~~~~~
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/mutex.rs:262:17: 262:27 help: if you meant to match on a variant, consider making the path in the pattern qualified: `sync::poison::TryLockError<sync::mutex::MutexGuard<'_, T>>::WouldBlock`
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/mutex.rs:262             Err(WouldBlock) => write!(f, "Mutex {{ <locked> }}")
                                                                                                            ^~~~~~~~~~
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/rwlock.rs:267:17: 267:27 warning: pattern binding `WouldBlock` is named the same as one of the variants of the type `sync::poison::TryLockError<sync::rwlock::RwLockReadGuard<'_, T>>` [E0170]
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/rwlock.rs:267             Err(WouldBlock) => write!(f, "RwLock {{ <locked> }}")
                                                                                                             ^~~~~~~~~~
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/rwlock.rs:267:17: 267:27 help: if you meant to match on a variant, consider making the path in the pattern qualified: `sync::poison::TryLockError<sync::rwlock::RwLockReadGuard<'_, T>>::WouldBlock`
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/rwlock.rs:267             Err(WouldBlock) => write!(f, "RwLock {{ <locked> }}")
                                                                                                             ^~~~~~~~~~
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/mutex.rs:262:17: 262:27 error: unused variable: `WouldBlock`, #[deny(unused_variables)] on by default
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/mutex.rs:262             Err(WouldBlock) => write!(f, "Mutex {{ <locked> }}")
                                                                                                            ^~~~~~~~~~
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/mutex.rs:262:17: 262:27 error: variable `WouldBlock` should have a snake case name such as `would_block`, #[deny(non_snake_case)] on by default
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/mutex.rs:262             Err(WouldBlock) => write!(f, "Mutex {{ <locked> }}")
                                                                                                            ^~~~~~~~~~
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/rwlock.rs:267:17: 267:27 error: unused variable: `WouldBlock`, #[deny(unused_variables)] on by default
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/rwlock.rs:267             Err(WouldBlock) => write!(f, "RwLock {{ <locked> }}")
                                                                                                             ^~~~~~~~~~
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/rwlock.rs:267:17: 267:27 error: variable `WouldBlock` should have a snake case name such as `would_block`, #[deny(non_snake_case)] on by default
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sync/rwlock.rs:267             Err(WouldBlock) => write!(f, "RwLock {{ <locked> }}")

nwin · 2015-02-26T09:19:33Z

Stupid me. This time I ran the complete compilation process and updated the PR.

Manishearth · 2015-02-26T09:26:03Z

@bors: retry clean

Manishearth · 2015-02-26T09:26:26Z

@bors: clean r+

bors · 2015-02-26T09:26:27Z

@bors r=Manishearth 36ba96e

bors · 2015-02-26T11:44:21Z

⌛ Testing commit 36ba96e with merge 0021151...

bors · 2015-02-26T13:39:55Z

💔 Test failed - auto-win-64-nopt-t

nwin · 2015-02-26T23:19:40Z

This looks like a temporary issue to me…can anybody trigger a retry with bors?

alexcrichton · 2015-02-27T00:17:28Z

@bors: retry

bors · 2015-02-27T01:27:19Z

⌛ Testing commit 36ba96e with merge 545b0c4...

bors · 2015-02-27T02:58:14Z

💔 Test failed - auto-mac-64-nopt-t

alexcrichton · 2015-02-27T03:00:31Z

@bors: retry

Implements `Debug` for `RwLock` and `arc::Weak` in the same way it is implemented for `rc::Weak` (basically copy & paste). The lack of this implementation prevents the automatic implementation of `Debug` for structs containing members of these types.

bors · 2015-02-27T10:35:52Z

⌛ Testing commit 36ba96e with merge bd0d8e4...

bors · 2015-02-27T12:57:53Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

rust-highfive assigned huonw Feb 20, 2015

alexcrichton reviewed Feb 20, 2015
View reviewed changes

nwin force-pushed the impl-debug-rwlock-weak branch from 87cefcd to 8c778f2 Compare February 25, 2015 07:54

Implement Debug for RwLock, arc::Weak and Mutex

36ba96e

nwin force-pushed the impl-debug-rwlock-weak branch from 8c778f2 to 36ba96e Compare February 26, 2015 09:18

bors merged commit 36ba96e into rust-lang:master Feb 27, 2015

Implement Debug for RwLock and arc::Weak #22573

Implement Debug for RwLock and arc::Weak #22573

Conversation

nwin commented Feb 20, 2015

rust-highfive commented Feb 20, 2015

huonw commented Feb 20, 2015

huonw commented Feb 20, 2015

nwin commented Feb 20, 2015

nwin commented Feb 20, 2015

alexcrichton Feb 20, 2015

Choose a reason for hiding this comment

alexcrichton commented Feb 25, 2015

nwin commented Feb 25, 2015

alexcrichton commented Feb 25, 2015

bors commented Feb 25, 2015

bors commented Feb 25, 2015

Manishearth commented Feb 25, 2015

bors commented Feb 25, 2015

bors commented Feb 25, 2015

bors commented Feb 25, 2015

bors commented Feb 25, 2015

Manishearth commented Feb 25, 2015

bors commented Feb 25, 2015

bors commented Feb 25, 2015

bors commented Feb 25, 2015

bors commented Feb 25, 2015

bors commented Feb 25, 2015

alexcrichton commented Feb 25, 2015

bors commented Feb 25, 2015

bors commented Feb 25, 2015

alexcrichton commented Feb 25, 2015

bors commented Feb 25, 2015

bors commented Feb 25, 2015

nwin commented Feb 25, 2015

Manishearth commented Feb 25, 2015

Manishearth commented Feb 25, 2015

Manishearth commented Feb 25, 2015

bors commented Feb 26, 2015

bors commented Feb 26, 2015

bors commented Feb 26, 2015

Manishearth commented Feb 26, 2015

nwin commented Feb 26, 2015

Manishearth commented Feb 26, 2015

Manishearth commented Feb 26, 2015

bors commented Feb 26, 2015

bors commented Feb 26, 2015

bors commented Feb 26, 2015

nwin commented Feb 26, 2015

alexcrichton commented Feb 27, 2015

bors commented Feb 27, 2015

bors commented Feb 27, 2015

alexcrichton commented Feb 27, 2015

bors commented Feb 27, 2015

bors commented Feb 27, 2015

Implement `Debug` for `RwLock` and `arc::Weak` #22573

Implement `Debug` for `RwLock` and `arc::Weak` #22573