-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Suggest as_mut
when Pin<T>
is used after move
#93181
Conversation
r? @lcnr (rust-highfive has picked a reviewer for you, use r? to override) |
/// Used for better diagnostics. | ||
is_option_or_result: bool, | ||
self_name: Option<Symbol>, |
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.
self_name: Option<Symbol>, | |
self_diagnostic_name: Option<Symbol>, |
r=me after @nagisa s comment |
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.
Oh yes, very nice!
err.span_suggestion_verbose( | ||
fn_call_span.shrink_to_lo(), | ||
&format!( | ||
"consider calling `.{}()` to borrow the type's contents", |
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.
Extra nit, but I personally would love that level of precision in the diagnostics: in the case where P = &mut _
, that is, when we have to deal with Pin<&mut …>
, we already have a borrow; and what we need is a re-borrow.
-
This may look silly, but I think one of the main areas of confusion with
Pin
for beginners is this need for.as_mut()
, precisely because it's a type which, contrary to&mut
, features no implicit reborrowing.So even an extra note for that very case, stating this very fact, could be quite educational:
note: this function takes ownership of the receiver `self`, which moves `foo` --> $DIR/pin-content-move.rs:6:12 | LL | fn foo(self: Pin<&mut Self>) {} | ^^^^ help: consider calling `.as_mut()` to reborrow the type's contents | LL | foo.as_mut().foo(); | +++++++++ note: this is needed because `Pin<&mut _>`, contrary to `&mut _`, does not feature implicit reborrowing. | LL | foo.as_mut().foo(); | +++++++++
-
While that instance seems like a nit, in cases such as
Pin<Rc<…>>
etc. for shared-pinned stuff, the.as_mut()
suggestion is also likely to be incorrect.So, if it weren't too annoying to implement, I'd do a heuristic whereby
Rc
,Arc
, and&mut _
would be special-cased, and would ask for, respectively:.as_ref()
to borrow,.as_ref()
to borrow,.as_mut()
to reborrow, and otherwise defaulting to what we have here:.as_mut()
to borrow.
Co-authored-by: Daniel Henry-Mantilla <[email protected]>
It looks like there's a false positive when the receiver actually takes self by value. We may need to do some type-checking to avoid this. |
The job Click to see the possible cause of the failure (guessed by this bot)
|
☔ The latest upstream changes (presumably #93956) made this pull request unmergeable. Please resolve the merge conflicts. |
@ibraheemdev @rustbot label: +S-inactive |
Resolves #65409