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 incorrect fmt::Pointer
implementations
#328
base: master
Are you sure you want to change the base?
Conversation
@tyranron are you still planinng on addressing this? |
@tyranron ping |
@JelteF yes, sorry for not being responsive. I was a little bit overwhelmed on my job last 2 months. I'll try to finish this in next 2 weeks or so. |
No worries, it happens. "Next 2 weeks or so" sounds amazing. |
Oh... crap. This is a bit of a rabbit hole. I've figured out the solution for cases like We basically hit the difference between accessing a @JelteF I think that supporting |
@JelteF thinking about it more, I'm unsure whether we should fix Instead, I propose to leave desugaring as it is now, and:
This way, throwing the compile-time error, we bring the library user's attention to this edge case, so he will much more aware about the implications for the |
tl;dr I like your previous proposal much better. Honestly, I cannot think of a reasonable case where anyone would do something like this: I think it makes sense to document the weirdness around this, but I think we should support |
@JelteF by the way, despite me being too busy to get on this lately, it appears for the better... today, when I was dreaming, I figured out the solution I do like and that should work in all the cases: // We declare in `derive_more` additional types machinery:
#[repr(transparent)]
pub struct PtrWrapper(T);
// We do the necessecarry dereference in the `fmt::Pointer` impl for our `PtrWrapper`
impl<T: fmt::Pointer> fmt::Pointer for PtrWrapper(&T) {
fn fmt(&self, f: fmt::Writer<'_>) -> fmt::Result {
(*self.0).fmt(f)
}
}
// For any other `fmt` trait we just do the transparent implementation.
impl<T: fmt::Debug> fmt::Debug for PtrWrapper(T) {
fn fmt(&self, f: fmt::Writer<'_>) -> fmt::Result {
self.0.fmt(f)
}
}
// And also `Deref`+`DerefMut`.
// And in the macro expansion we do this:
let _0 = ::derive_more::fmt::PtrWrapper(&self.0);
// instead of this:
let _0 = &self.0;
// so here, wherever the `_0` value is engaged, it will be dereferenced once used as `{:p}`
write!(f, "--> {_0:p}")
// and any other cases should work just fine due to transparent impls of `fmt` trait and `Deref`/`DerefMut`. |
@tyranron that sounds like a great solution. And I think you wouldn't even need to implement |
Synopsis
Debug
andDisplay
derives allow referring fields via short syntax (_0
for unnamed fields andname
for named fields):The way this works is by introducing a local binding in the macro expansion:
This, however, introduces double pointer indirection. For most of the
fmt
traits, this is totally OK. However, thefmt::Pointer
is sensitive to that:Solution
Instead of introducing local bindings, try to replace this shortcuts (like
_0
) in idents and expressions with a field accessing syntax (likeself.0
). Or event just substitute_0
with(*_0)
, because for enum variants we cannot really access the fields viaself.
.Checklist
Documentation is updated(not required)