-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Implement if-let match guards #79051
Conversation
29e5689
to
3a0fc6c
Compare
87f8143
to
a18c1fe
Compare
This comment has been minimized.
This comment has been minimized.
a18c1fe
to
da63c00
Compare
This comment has been minimized.
This comment has been minimized.
fb65462
to
3333608
Compare
@matthewjasper This should be ready for review now! |
This comment has been minimized.
This comment has been minimized.
e19b725
to
0cb8491
Compare
Thanks a lot @Nadrieril for the pointers! |
967aeab
to
95d536e
Compare
All right, CI passes again now 😅 |
This comment has been minimized.
This comment has been minimized.
⌛ Testing commit 0917260 with merge 02d3146f99803253859dae64af1f8c9508ddc37f... |
💔 Test failed - checks-actions |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. 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 |
Failure looks spurious? |
Looks like aarch64-gnu was cancelled on Github's end? There's no logs. @bors retry |
Rollup of 11 pull requests Successful merges: - rust-lang#79051 (Implement if-let match guards) - rust-lang#79877 (Allow `since="TBD"` for rustc_deprecated) - rust-lang#79882 (Fix issue rust-lang#78496) - rust-lang#80026 (expand-yaml-anchors: Make the output directory separator-insensitive) - rust-lang#80039 (Remove unused `TyEncoder::tcx` required method) - rust-lang#80069 (Test that `core::assert!` is valid) - rust-lang#80072 (Fixed conflict with drop elaboration and coverage) - rust-lang#80073 (Add support for target aliases) - rust-lang#80082 (Revert rust-lang#78790 - rust-src vendoring) - rust-lang#80097 (Add `popcount` and `popcnt` as doc aliases for `count_ones` methods.) - rust-lang#80103 (Remove docs for non-existent parameters in `rustc_expand`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…, r=davidtwco Change the message for `if_let_guard` feature gate `if-let` guards are now implemented by rust-lang#79051 🎉 Thanks `@camelid` for pointing this out 🙂
…, r=davidtwco Change the message for `if_let_guard` feature gate `if-let` guards are now implemented by rust-lang#79051 🎉 Thanks ``@camelid`` for pointing this out 🙂
…thewjasper Implement if-let match guards Implements rust-lang/rfcs#2294 (tracking issue: rust-lang#51114). I probably should do a few more things before this can be merged: - [x] Add tests (added basic tests, more advanced tests could be done in the future?) - [x] Add lint for exhaustive if-let guard (comparable to normal if-let statements) - [x] Fix clippy However since this is a nightly feature maybe it's fine to land this and do those steps in follow-up PRs. Thanks a lot `@matthewjasper` ❤️ for helping me with lowering to MIR! Would you be interested in reviewing this? r? `@ghost` for now
This was a regression of 3% on the match-stress-enum benchmark, but since it's directly adding more code to it that seems relatively expected. https://perf.rust-lang.org/compare.html?start=a6491be5be9344a325b7e49b0114f3cf67ef199e&end=9b84d36a0b9ea3bf305f36f08d50aa42c26f96c2&stat=instructions:u |
let tpat = self.lower_pattern(&mut cx, pat, &mut false).0; | ||
check_if_let_guard(&mut cx, &tpat, pat.hir_id); | ||
} | ||
} |
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 like this loop could be merged with the one above, that would probably reduce the perf impact
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'll try opening a PR. (EDIT: #80367)
Suggested by Nadrieril in rust-lang#79051 (comment).
…adrieril Combine two loops in `check_match` Suggested by Nadrieril in rust-lang#79051 (comment). Opening to get a perf run. Hopefully this code doesn't require everything in the first loop to be done before running the second! (It shouldn't though.) cc `@Nadrieril`
Implements rust-lang/rfcs#2294 (tracking issue: #51114).
I probably should do a few more things before this can be merged:
However since this is a nightly feature maybe it's fine to land this and do those steps in follow-up PRs.
Thanks a lot @matthewjasper ❤️ for helping me with lowering to MIR! Would you be interested in reviewing this?
r? @ghost for now