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.

Sign up for GitHub

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

Jump to bottom

Remove wrong assertion in match checking. #111015

Merged
merged 1 commit into from
May 1, 2023
Merged

Remove wrong assertion in match checking. #111015

merged 1 commit into from
May 1, 2023

Conversation

cjgillot
Copy link
Contributor

This assertions is completely misguided, introduced by #108504. The responsible PR is on beta, nominating for backport.

Instead of checking that this is not a &&, it would make sense to check that neither arms of that && is a let. This seems like a lot of code for unclear benefit.

r? @saethlin

@cjgillot cjgillot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 30, 2023
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 30, 2023
@saethlin
Copy link
Member

Thanks for fixing this and letting me know, but I don't understand the code in question and I'm pretty sure I don't have permissions to approve this even if I wanted to. (It's odd that rustbot let you assign me 🤔)

I agree with the beta nomination, we already know of one crate in the wild that this surely affects (human_name).

@cjgillot
Copy link
Contributor Author

r? compiler

@rustbot rustbot assigned davidtwco and unassigned saethlin Apr 30, 2023
@compiler-errors
Copy link
Member

compiler-errors commented Apr 30, 2023 •
edited
Loading

Instead of checking that this is not a &&, it would make sense to check that neither arms of that && is a let. This seems like a lot of code for unclear benefit.

let-chains can't be grouped by parentheses, and && is left-to-right associative, so I actually don't see a case where we'd encounter a nested let in this part of the code, since we already "recurse" on the lhs in the loop below the code you touched. Seems fine to me, and even if I'm misunderstanding some subtle AST case, this would just regress a lint.

r? @compiler-errors @bors r+

@bors
Copy link
Contributor

bors commented Apr 30, 2023

📌 Commit 84cb7ec has been approved by compiler-errors

It is now in the queue for this repository.

@rustbot rustbot assigned compiler-errors and unassigned davidtwco Apr 30, 2023
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 30, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 30, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 1, 2023
@bors
Auto merge of rust-lang#111040 - matthiaskrgr:rollup-g2sns0f, r=matth…
2034b6d 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#110823 (Tweak await span to not contain dot)
 - rust-lang#111015 (Remove wrong assertion in match checking.)
 - rust-lang#111023 (Test precise capture with a multi-variant enum and exhaustive patterns)
 - rust-lang#111032 (Migrate `builtin_macros::asm` diagnostics to translatable diagnostics)
 - rust-lang#111033 (Ping Nadrieril when changing exhaustiveness checking)
 - rust-lang#111037 (Close parentheses for `offset_of` in AST pretty printing)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 07726e3 into rust-lang:master May 1, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 1, 2023
@cjgillot cjgillot deleted the chained-let-and branch May 1, 2023 08:06
@saethlin saethlin mentioned this pull request May 3, 2023
Closed
@apiraino
Copy link
Contributor

apiraino commented May 4, 2023

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label May 4, 2023
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 6, 2023
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.71.0, 1.70.0 May 6, 2023
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request May 6, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 7, 2023
@bors
Auto merge of rust-lang#111290 - Mark-Simulacrum:beta-backport, r=Mar…
2013813 
…k-Simulacrum

[beta] backport

This PR backports:

- rust-lang#111015: Remove wrong assertion in match checking.
- rust-lang#110917: only error combining +whole-archive and +bundle for rlibs
- rust-lang#111201: bootstrap: add .gitmodules to the sources

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@compiler-errors compiler-errors

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.70.0
Development

Successfully merging this pull request may close these issues.
8 participants
@cjgillot @saethlin @compiler-errors @bors @apiraino @davidtwco @Mark-Simulacrum @rustbot