-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Rustup to rustc 1.39.0-nightly (acf7b50c7 2019-09-25) #4574
Conversation
- Addresses inference error - Updates compiletest
Working on fixing the or-patterns breakage. |
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.
Looks good; I have mostly suggestions for when it's not urgent (a later time).
@@ -112,8 +112,7 @@ impl<'tcx> Visitor<'tcx> for CCHelper { | |||
walk_expr(self, e); | |||
match e.node { | |||
ExprKind::Match(_, ref arms, _) => { | |||
let arms_n: u64 = arms.iter().map(|arm| arm.pats.len() as u64).sum(); | |||
if arms_n > 1 { | |||
if arms.len() > 1 { |
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.
If I'm reading the intent of this code you might want to recurse and count each or-pattern as cc += 1
but I think the change you made makes more sense.
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.
yeah i debated this but i think we might need a more careful CC accounting for pats here
), | ||
); | ||
} else { | ||
db.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs)); |
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.
Aside: In the spirit of this lint, in the future you may want a lint that suggests C(p0) | C(p1)
=> C(p0 | p1)
.
&& arms[1].pats.len() == 1 | ||
&& arms[1].guard.is_none() | ||
{ | ||
if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() { |
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 feel that I've seen this pattern a few times now btw... maybe refactor in a follow up PR.
@@ -772,7 +754,7 @@ fn is_ref_some_arm(arm: &Arm) -> Option<BindingAnnotation> { | |||
fn has_only_ref_pats(arms: &[Arm]) -> bool { | |||
let mapped = arms | |||
.iter() | |||
.flat_map(|a| &a.pats) |
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'd just inline into arm.p.node
here.
if arms[0].pats.len() == 1 && arms[0].guard.is_none(); | ||
if arms[1].pats.len() == 1 && arms[1].guard.is_none(); | ||
if arms[0].guard.is_none(); | ||
if arms[1].guard.is_none(); |
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.
(Another instance of the same pattern)
Holding off on landing till rust-lang/rust#64774 exists |
Might be worth landing early via the merge button so we can pipeline up a clippyup PR though |
@bors treeclosed=10 r=oli-obk p=1 Closing tree till rust-lang/rust#64774 lands |
📌 Commit b5cadd7 has been approved by |
🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened |
@bors r=yaahc,centril |
💡 This pull request was already approved, no need to approve it again. |
📌 Commit b5cadd7 has been approved by |
🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened |
💡 This pull request was already approved, no need to approve it again. |
📌 Commit b5cadd7 has been approved by |
🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened |
@bors r=yaahc,centril oops |
💡 This pull request was already approved, no need to approve it again. |
📌 Commit b5cadd7 has been approved by |
🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened |
@bors treeclosed- r=yaahc,centril I'm marking OSX as an allowed failure so that we can get a clippyup landed immediately after the cargo update. There's three PRs worth of breakages here and I'd rather not buffer futher, especially since there's another big breakage coming soon. |
📌 Commit a756b9b has been approved by |
Rustup to rustc 1.39.0-nightly (acf7b50 2019-09-25) changelog: none fixes rust-lang/rust#64777 r? @phansch @yaahc
☀️ Test successful - checks-travis, status-appveyor |
Update clippy We had a series of breakages, getting this in fast before more things break. rust-lang/rust-clippy#4574 r? @ghost
changelog: none
fixes rust-lang/rust#64777
r? @phansch @yaahc