-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[NLL] Fix various unused mut errors #51964
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
Conversation
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.
Left a few nits and a question but basically 💯
// used as mut. This is OK here because the upvar | ||
// expressions have no side effects and act on | ||
// disjoint places. | ||
Some(Category::Place) => { |
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.
to make sure I am remembering correctly — this occurs on copy/move variables, whereas a by-ref binding would use as_operand
?
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.
this might be worth noting in a comment
src/librustc_mir/build/mod.rs
Outdated
name = Some(ident.name); | ||
if let hir::PatKind::Binding(binding_annotation, _, ident, _) = pat.node { | ||
if binding_annotation == hir::BindingAnnotation::Unannotated | ||
|| binding_annotation == hir::BindingAnnotation::Mutable |
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 don't quite understand this part — does this not affect debuginfo in negative ways or something?
Also, a nit: maybe we should use a match here?
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 presume this corresponds to
Change mir generation so that the parameter variable doesn't get a name when a ref pattern is used as an argument
?
In that case, I would want to see some comments to that effect...
81d5037
to
56c17dc
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
56c17dc
to
85dbe63
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
85dbe63
to
3fe3ea0
Compare
3fe3ea0
to
125c9d9
Compare
@bors r+ |
📌 Commit 125c9d9 has been approved by |
…omatsakis [NLL] Fix various unused mut errors Closes #51801 Closes #50897 Closes #51830 Closes #51904 cc #51918 - keeping this one open in case there are any more issues This PR contains multiple changes. List of changes with examples of what they fix: * Change mir generation so that the parameter variable doesn't get a name when a `ref` pattern is used as an argument ```rust fn f(ref y: i32) {} // doesn't trigger lint ``` * Change mir generation so that by-move closure captures don't get first moved into a temporary. ```rust let mut x = 0; // doesn't trigger lint move || { x = 1; }; ``` * Treat generator upvars the same as closure upvars ```rust let mut x = 0; // This mut is now necessary and is not linted against. move || { x = 1; yield; }; ``` r? @nikomatsakis
☀️ Test successful - status-appveyor, status-travis |
Closes #51801
Closes #50897
Closes #51830
Closes #51904
cc #51918 - keeping this one open in case there are any more issues
This PR contains multiple changes. List of changes with examples of what they fix:
ref
pattern is used as an argumentr? @nikomatsakis