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

Rustup to rustc 1.39.0-nightly (acf7b50c7 2019-09-25) #4574

Merged
merged 4 commits into from
Sep 25, 2019

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented Sep 25, 2019

changelog: none

fixes rust-lang/rust#64777

r? @phansch @yaahc

 - Addresses inference error
 - Updates compiletest
@Manishearth Manishearth changed the title Update many_single_char_names to deal with inference error Rustup to rustc 1.39.0-nightly (acf7b50c7 2019-09-25) Sep 25, 2019
@Manishearth
Copy link
Member Author

Working on fixing the or-patterns breakage.

Copy link
Contributor

@Centril Centril left a 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 {
Copy link
Contributor

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.

Copy link
Member Author

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));
Copy link
Contributor

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() {
Copy link
Contributor

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)
Copy link
Contributor

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();
Copy link
Contributor

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)

@Manishearth
Copy link
Member Author

Holding off on landing till rust-lang/rust#64774 exists

@Manishearth
Copy link
Member Author

Might be worth landing early via the merge button so we can pipeline up a clippyup PR though

@Manishearth
Copy link
Member Author

Manishearth commented Sep 25, 2019

@bors treeclosed=10 r=oli-obk p=1

Closing tree till rust-lang/rust#64774 lands

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

📌 Commit b5cadd7 has been approved by oli-obk

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened

@Manishearth
Copy link
Member Author

@bors r=yaahc,centril

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

📌 Commit b5cadd7 has been approved by yaahc,centril

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

📌 Commit b5cadd7 has been approved by oli-obk

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened

@Manishearth
Copy link
Member Author

@bors r=yaahc,centril

oops

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

📌 Commit b5cadd7 has been approved by yaahc,centril

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened

@Manishearth
Copy link
Member Author

@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.

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

📌 Commit a756b9b has been approved by yaahc,centril

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

⌛ Testing commit a756b9b with merge d5570e4...

bors added a commit that referenced this pull request Sep 25, 2019
Rustup to rustc 1.39.0-nightly (acf7b50 2019-09-25)

changelog: none

fixes rust-lang/rust#64777

r? @phansch @yaahc
@bors
Copy link
Collaborator

bors commented Sep 25, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: yaahc,centril
Pushing d5570e4 to master...

@bors bors merged commit a756b9b into rust-lang:master Sep 25, 2019
@Manishearth Manishearth deleted the rustup branch September 25, 2019 21:42
bors added a commit to rust-lang/rust that referenced this pull request Sep 25, 2019
Update clippy

We had a series of breakages, getting this in fast before more things break.

rust-lang/rust-clippy#4574

r? @ghost
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.

clippy-driver no longer builds after rust-lang/rust#64766
4 participants