-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Fix Display
for cell::{Ref,RefMut}
#97225
Conversation
These guards changed to pointers in rust-lang#97027, but their `Display` was formatting that field directly, which made it show the raw pointer value. Now we go through `Deref` to display the real value again.
This comment was marked as resolved.
This comment was marked as resolved.
r? @scottmcm (rust-highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@@ -1487,7 +1487,7 @@ impl<'b, T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Ref<'b, U>> for Ref<'b, | |||
#[stable(feature = "std_guard_impls", since = "1.20.0")] | |||
impl<T: ?Sized + fmt::Display> fmt::Display for Ref<'_, T> { | |||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | |||
self.value.fmt(f) | |||
(**self).fmt(f) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ymmv: <T as fmt::Display>::fmt(self, f)
would be another way to ensure that this bug can't really get reintroduced.
As you have it is fine too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that, but it should be fine with the added test coverage.
This looks good, thanks! Also r=me with the change I mentioned, if you want to make it. @bors r+ |
📌 Commit 83abb7c has been approved by |
Rollup of 6 pull requests Successful merges: - rust-lang#97144 (Fix rusty grammar in `std::error::Reporter` docs) - rust-lang#97225 (Fix `Display` for `cell::{Ref,RefMut}`) - rust-lang#97228 (Omit stdarch workspace from rust-src) - rust-lang#97236 (Recover when resolution did not resolve lifetimes.) - rust-lang#97245 (Fix typo in futex RwLock::write_contended.) - rust-lang#97259 (Fix typo in Mir phase docs) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
These guards changed to pointers in #97027, but their
Display
wasformatting that field directly, which made it show the raw pointer
value. Now we go through
Deref
to display the real value again.Miri noticed this change, #97204, so hopefully that will be fixed.