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

Make the unused_mut lint smarter #26378

Merged
merged 2 commits into from
Jul 3, 2015
Merged

Make the unused_mut lint smarter #26378

merged 2 commits into from
Jul 3, 2015

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Jun 17, 2015

This makes it somewhat more aggressive, so this is kind-of a [breaking-change] for these compiling with #[deny(unused_mut)].

r? @pnkfelix

@bors
Copy link
Contributor

bors commented Jun 20, 2015

☔ The latest upstream changes (presumably #24527) made this pull request unmergeable. Please resolve the merge conflicts.

@arielb1
Copy link
Contributor Author

arielb1 commented Jun 30, 2015

bump

@pnkfelix
Copy link
Member

pnkfelix commented Jul 2, 2015

(I don't know if I agree that #26332 is actually a bug ... that is, I don't know if I like the idea of flagging that sort of code with a warning... but I'm willing to assume that it is for the purposes of completing this review)

@pnkfelix
Copy link
Member

pnkfelix commented Jul 2, 2015

cc @rust-lang/lang

@pnkfelix
Copy link
Member

pnkfelix commented Jul 2, 2015

(The cleanup commit looks fine to me; I am just hesistant to r-plus this PR without double-checking that others agree that this change to the lint in question is desirable.)

@nrc
Copy link
Member

nrc commented Jul 3, 2015

f+ on the principle here - it sounds like it is making the lint better, and I don't think lints count as a breaking change

@pnkfelix
Copy link
Member

pnkfelix commented Jul 3, 2015

@nrc okay, my main concern was that someone who writes:

let mut x;
...
x = 3;

may intend, in the long term, to add additional statements that assign to x, and this lint will be annoying for that case.

But I am willing to believe that the change here will help more than it hurts. Especially if it helps new-comers understand the idea that the "delayed single-assignment style" like above is not considered mutation by rustc.

@pnkfelix
Copy link
Member

pnkfelix commented Jul 3, 2015

@bors r+ a18d984

@bors
Copy link
Contributor

bors commented Jul 3, 2015

⌛ Testing commit a18d984 with merge 73165be...

@bors
Copy link
Contributor

bors commented Jul 3, 2015

💔 Test failed - auto-mac-64-nopt-t

@alexcrichton
Copy link
Member

@bors: retry

On Fri, Jul 3, 2015 at 6:59 AM, bors [email protected] wrote:

[image: 💔] Test failed - auto-mac-64-nopt-t
http://buildbot.rust-lang.org/builders/auto-mac-64-nopt-t/builds/5606


Reply to this email directly or view it on GitHub
#26378 (comment).

@bors
Copy link
Contributor

bors commented Jul 3, 2015

⌛ Testing commit a18d984 with merge 728af15...

@bors
Copy link
Contributor

bors commented Jul 3, 2015

💔 Test failed - auto-mac-64-nopt-t

@alexcrichton
Copy link
Member

@bors: retry

On Fri, Jul 3, 2015 at 12:59 PM, bors [email protected] wrote:

[image: 💔] Test failed - auto-mac-64-nopt-t
http://buildbot.rust-lang.org/builders/auto-mac-64-nopt-t/builds/5609


Reply to this email directly or view it on GitHub
#26378 (comment).

@bors
Copy link
Contributor

bors commented Jul 3, 2015

⌛ Testing commit a18d984 with merge f027bdc...

bors added a commit that referenced this pull request Jul 3, 2015
This makes it somewhat more aggressive, so this is kind-of a [breaking-change] for these compiling with `#[deny(unused_mut)]`.

r? @pnkfelix
@bors bors merged commit a18d984 into rust-lang:master Jul 3, 2015
@arielb1 arielb1 mentioned this pull request Sep 15, 2015
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

Successfully merging this pull request may close these issues.

5 participants