Could the bound on Weak: Debug be relaxed?

Apologies if there is prior art here, I wasn't able to find it with a search. Before submitting a presumptuous PR I wanted to discuss it here:

Both rc::Weak<T> and sync::Weak<T> currently require T: Debug in their own Debug implementations, but they don't use it - both are implemented very simply as

write!(f, "(Weak)")

Is the T: Debug bound intentional (for future compatibility or other reasons), or is there an opportunity for it to be relaxed here?

1 Like

In principle, a Debug implementation of Weak could also use upgrade and on success show the contents of what it's pointing to. Doing this in general can have disadvantages, in particular when you have reference cycles – which is one of the main points why Weak exists in the first place – showing the target of a Weak pointer when there's a cycle will inevitably result in non-terminating Debug output (or more realistically in a stack overflow).

I suppose it's not entirely out of the question that some specialization in the future could be able to only show the contents of a weak pointer for types that cannot contain any references back to themselves (if that's something the compiler can actually do - for example a Weak<i32> can never be problematic.. ) but that seems a bit contrived, IMO, and also once that's possible, that same Debug implementation could presumably also specialize on whether or not T: Debug as well.

All in all I feel like removing the bound seems like a good idea, because I don't see the argument for future compatibility that would suggest keeping the bound. If you want, feel free to just open a PR and see if it gets accepted (removing the bound seems like an easy change so even if it's not accepted, there's not really any wasted effort).

2 Likes

Thanks for the vote of confidence; I opened one https://github.com/rust-lang/rust/pull/90291

1 Like

In one of my own projects, I've implemented Debug for types with reference cycles by using a thread_local to check for recursive calls, so the Debug output shows one level deep but not further than that. This is something that we could hypothetically do that would require Debug bound.

1 Like

Since thread_local requires std, and the reference counted pointers are available in alloc, this wouldn't be possible unless a no_std version of thread_local became available - which while it certainly might be possible - I think would end up being fairly platform-specific, and platform-specific Debug behavior would seem strange to me personally.

2 Likes

I can imagine some other possible issues with upgrading in Debug, especially with sync::Weak. Atomic operations aren't free and a thread-synchronization event in a nested Debug impl has the potential to cause a surprise performance regression in someone's code.

Ahhh, yeah, I didn't think of that because I haven't done much no_std stuff myself

Beyond just the performance impact of thread-synchronization, calling upgrade in Debug has the potential to run the destructor for the data in the Debug implementation, in the event all the strong references than existed before Debug was called into disappear before it finishes.

1 Like