Guard types should implement more traits

[huonw]

Types like std::sync::MutexGuard are semantically just a &mut with a destructor, and so it makes sense for them to implement the same sort of traits, e.g. Display and Debug.

[steveklabnik]

Triage: MutexGuard does not implement those traits today.

[Mark-Simulacrum]

Debug is implemented today, but not Display.

@rust-lang/libs: This seems relevant to libz blitz, we probably want to work out some guidelines for wrapper types like this (what traits they should "forward"). For example, I can't think of a good Display implementation for MutexGuard, but perhaps there is one.

To an extent, it could be argued that most if not all std traits should be forwarded through types like MutexGuard, but this feels like it gets painful very quickly.

[sfackler]

The Display impl for MutexGuard would just delegate to the inner value's Display.

[brson]

Smart pointers and guards should eagerly implement common traits. Debug at least should be transparent, custom implemented to defer to the inner type. Not clear whether Eq/Ord should be implemented against the inner type.

Next step here is to identify such types in std and file a PR.

Here's an issue for the guidelines: rust-lang/api-guidelines#94

[ChrisMacNaughton]

I'd like to have a go at this one

[ChrisMacNaughton]

As a first pass, I'm looking at ensuring that MutexGuard, RwLockReadGuard, and RwLockWriteGuard impl Display and Debug. How does that sound?

[Mark-Simulacrum]

Sounds good! You can also look at things like Arc and RefCells guards.

[ChrisMacNaughton]

@Mark-Simulacrum RefCell doesn't seem to have a Guard? https://doc.rust-lang.org/std/cell/index.html#structs

[Mark-Simulacrum]

https://doc.rust-lang.org/std/cell/struct.RefMut.html and https://doc.rust-lang.org/std/cell/struct.Ref.html are RefCell's guards.

[ChrisMacNaughton]

Thanks!

[brson]

Thanks a bunch @ChrisMacNaughton.

I'm reopening this because we also want Eq, PartialEq, Ord, PartialOrd, and Hash for these types.

@rust-lang/libs if we implement e.g. impl Eq<Rhs = MutexGuard<T>> for MutexGuard<T> should we also implement impl Eq<Rhs = T> for MutexGuard<T>?

That would also seem to call for something like impl Eq<Rhs = MutexGuard<T>> for T, but that doesn't seem like something that can be written in std, or can it?

[alexcrichton]

I've never been clear on the set of Eq/PartialEq and such impls we have (those with a type parameter), but adding more seems fine by me.

[ghost]

What about Borrow/BorrowMut/AsRef/AsMut impls?

[dtolnay]

I am closing for now because I believe we are going to hold off on adding more of these impls.

Ok we got a chance to discuss this during the libs triage meeting today. In discussing this we ended up being increasingly wary of the guideline we set forth to add these trait impls and ended up concluding that we may not wish to add these at this time.

These implementations, as added here, wouldn't enable something like MutexGuard<i32> == i32, but that's arguably something we'd like to work. Adding that, however, precludes MutexGuard<i32> == MutexGuard<i32>. We didn't believe there was a clear way to implement this right now as all current strategies had downsides.

The possibility of specialization in the future may end up providing a solution to this problem, and @aturon also mentioned that the lang team is going to be working during the impl period on "deref coercion ergonomics" to alleviate much of the need for these impls in the first place.

In light of all that I'm going to close this for now and we'll likely be removing the guideline indicating that these traits should be implemented temporarily.

[Stargateur]

These implementations, as added here, wouldn't enable something like MutexGuard<i32> == i32, but that's arguably something we'd like to work. Adding that, however, precludes MutexGuard<i32> == MutexGuard<i32>. We didn't believe there was a clear way to implement this right now as all current strategies had downsides.

What strategy and what are the downside ? Following https://stackoverflow.com/q/66753005/7076153 question.