Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alloc::sync::Weak<T, A> is missing core::fmt::Debug #131

Closed
matthewpipie opened this issue Aug 24, 2024 · 4 comments
Closed

alloc::sync::Weak<T, A> is missing core::fmt::Debug #131

matthewpipie opened this issue Aug 24, 2024 · 4 comments

Comments

@matthewpipie
Copy link

I'm fairly new to Rust so I'm not sure if this is intended, but I am running into this issue while storing a Weak inside a derive(Debug) struct. I checked the Rust source and found this line:

#[stable(feature = "arc_weak", since = "1.4.0")]
impl<T: ?Sized> fmt::Debug for Weak<T> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "(Weak)")
    }
}

However, this only implements Debug for Weak, not Weak<T, A>.

Should this be changed to say Weak<T, A> instead? I'm happy to make a PR to rust-lang that changes it to:

#[stable(feature = "arc_weak", since = "1.4.0")]
impl<T: ?Sized, A: Allocator> fmt::Debug for Weak<T, A> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "(Weak)")
    }
}

Additionally, though out of the scope of this issue, is there a good reason that this debug function does not try to display the Option that it implicitly contains?

@Skgland
Copy link

Skgland commented Aug 24, 2024

Is this also applicable to the Weak of Rc?

Additionally, though out of the scope of this issue, is there a good reason that this debug function does not try to display the Option that it implicitly contains?

In Short: Reference Counted pointers often end up in recursive structures, so printing the contents of Weak would likely lead to recursing infinitely (until the stack overflows).

The PR that removed the Debug bound explains this
rust-lang/rust#90291

It also can't be readded as adding such a bound would be a breaking change.

@matthewpipie
Copy link
Author

Is this also applicable to the Weak of Rc?

It appears that Rc's Weak correctly implements this:

#[stable(feature = "rc_weak", since = "1.4.0")]
impl<T: ?Sized, A: Allocator> fmt::Debug for Weak<T, A> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "(Weak)")
    }
}

(src: https://doc.rust-lang.org/1.80.1/src/alloc/rc.rs.html#3348 )

In Short: Reference Counted pointers often end up in recursive structures, so printing the contents of Weak would likely lead to recursing infinitely (until the stack overflows).

This makes a ton of sense, thanks!

@matthewpipie
Copy link
Author

Made a PR! rust-lang/rust#129673

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 28, 2024
… r=dtolnay

Add fmt::Debug to sync::Weak<T, A>

Currently, `sync::Weak<T>` implements `Debug`, but `sync::Weak<T, A>` does not. This appears to be an oversight, as `rc::Weak<T, A>` implements `Debug`. (Note: `sync::Weak` is the weak for `Arc`, and `rc::Weak` is the weak for `Rc`.)

This PR adds the Debug trait for `sync::Weak<T, A>`. The issue was initially brought up here: rust-lang/wg-allocators#131
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Aug 29, 2024
… r=dtolnay

Add fmt::Debug to sync::Weak<T, A>

Currently, `sync::Weak<T>` implements `Debug`, but `sync::Weak<T, A>` does not. This appears to be an oversight, as `rc::Weak<T, A>` implements `Debug`. (Note: `sync::Weak` is the weak for `Arc`, and `rc::Weak` is the weak for `Rc`.)

This PR adds the Debug trait for `sync::Weak<T, A>`. The issue was initially brought up here: rust-lang/wg-allocators#131
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Aug 29, 2024
… r=dtolnay

Add fmt::Debug to sync::Weak<T, A>

Currently, `sync::Weak<T>` implements `Debug`, but `sync::Weak<T, A>` does not. This appears to be an oversight, as `rc::Weak<T, A>` implements `Debug`. (Note: `sync::Weak` is the weak for `Arc`, and `rc::Weak` is the weak for `Rc`.)

This PR adds the Debug trait for `sync::Weak<T, A>`. The issue was initially brought up here: rust-lang/wg-allocators#131
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 29, 2024
Rollup merge of rust-lang#129673 - matthewpipie:arc-weak-debug-trait, r=dtolnay

Add fmt::Debug to sync::Weak<T, A>

Currently, `sync::Weak<T>` implements `Debug`, but `sync::Weak<T, A>` does not. This appears to be an oversight, as `rc::Weak<T, A>` implements `Debug`. (Note: `sync::Weak` is the weak for `Arc`, and `rc::Weak` is the weak for `Rc`.)

This PR adds the Debug trait for `sync::Weak<T, A>`. The issue was initially brought up here: rust-lang/wg-allocators#131
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Aug 30, 2024
Add fmt::Debug to sync::Weak<T, A>

Currently, `sync::Weak<T>` implements `Debug`, but `sync::Weak<T, A>` does not. This appears to be an oversight, as `rc::Weak<T, A>` implements `Debug`. (Note: `sync::Weak` is the weak for `Arc`, and `rc::Weak` is the weak for `Rc`.)

This PR adds the Debug trait for `sync::Weak<T, A>`. The issue was initially brought up here: rust-lang/wg-allocators#131
@matthewpipie
Copy link
Author

Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants