Rustup #9388
Merged
Rustup #9388
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It is passed an argument that is never used.
Simplify visitors By removing some unused arguments. r? `@cjgillot`
Update Clippy r? `@Manishearth`
Visit attributes in more places. This adds 3 loosely related changes (I can split PRs if desired): - Attribute checking on pattern struct fields. - Attribute checking on struct expression fields. - Lint level visiting on pattern struct fields, struct expression fields, and generic parameters. There are still some lints which ignore lint levels in various positions. This is a consequence of how the lints themselves are implemented. For example, lint levels on associated consts don't work with `unused_braces`.
- Rename `ast::Lit::token` as `ast::Lit::token_lit`, because its type is
`token::Lit`, which is not a token. (This has been confusing me for a
long time.)
reasonable because we have an `ast::token::Lit` inside an `ast::Lit`.
- Rename `LitKind::{from,to}_lit_token` as
`LitKind::{from,to}_token_lit`, to match the above change and
`token::Lit`.
Clean up `LitKind` r? ``@petrochenkov``
Because it's never used meaningfully.
# Stabilization proposal The feature was implemented in rust-lang/rust#50045 by est31 and has been in nightly since 2018-05-16 (over 4 years now). There are [no open issues][issue-label] other than the tracking issue. There is a strong consensus that `break` is the right keyword and we should not use `return`. There have been several concerns raised about this feature on the tracking issue (other than the one about tests, which has been fixed, and an interaction with try blocks, which has been fixed). 1. nrc's original comment about cost-benefit analysis: rust-lang/rust#48594 (comment) 2. joshtriplett's comments about seeing use cases: rust-lang/rust#48594 (comment) 3. withoutboats's comments that Rust does not need more control flow constructs: rust-lang/rust#48594 (comment) Many different examples of code that's simpler using this feature have been provided: - A lexer by rpjohnst which must repeat code without label-break-value: rust-lang/rust#48594 (comment) - A snippet by SergioBenitez which avoids using a new function and adding several new return points to a function: rust-lang/rust#48594 (comment). This particular case would also work if `try` blocks were stabilized (at the cost of making the code harder to optimize). - Several examples by JohnBSmith: rust-lang/rust#48594 (comment) - Several examples by Centril: rust-lang/rust#48594 (comment) - An example by petrochenkov where this is used in the compiler itself to avoid duplicating error checking code: rust-lang/rust#48594 (comment) - Amanieu recently provided another example related to complex conditions, where try blocks would not have helped: rust-lang/rust#48594 (comment) Additionally, petrochenkov notes that this is strictly more powerful than labelled loops due to macros which accidentally exit a loop instead of being consumed by the macro matchers: rust-lang/rust#48594 (comment) nrc later resolved their concern, mostly because of the aforementioned macro problems. joshtriplett suggested that macros could be able to generate IR directly (rust-lang/rust#48594 (comment)) but there are no open RFCs, and the design space seems rather speculative. joshtriplett later resolved his concerns, due to a symmetry between this feature and existing labelled break: rust-lang/rust#48594 (comment) withoutboats has regrettably left the language team. joshtriplett later posted that the lang team would consider starting an FCP given a stabilization report: rust-lang/rust#48594 (comment) [issue-label]: https://github.com/rust-lang/rust/issues?q=is%3Aissue+is%3Aopen+label%3AF-label_break_value+ ## Report + Feature gate: - https://github.com/rust-lang/rust/blob/d695a497bbf4b20d2580b75075faa80230d41667/src/test/ui/feature-gates/feature-gate-label_break_value.rs + Diagnostics: - https://github.com/rust-lang/rust/blob/6b2d3d5f3cd1e553d87b5496632132565b6779d3/compiler/rustc_parse/src/parser/diagnostics.rs#L2629 - https://github.com/rust-lang/rust/blob/f65bf0b2bb1a99f73095c01a118f3c37d3ee614c/compiler/rustc_resolve/src/diagnostics.rs#L749 - https://github.com/rust-lang/rust/blob/f65bf0b2bb1a99f73095c01a118f3c37d3ee614c/compiler/rustc_resolve/src/diagnostics.rs#L1001 - https://github.com/rust-lang/rust/blob/111df9e6eda1d752233482c1309d00d20a4bbf98/compiler/rustc_passes/src/loops.rs#L254 - https://github.com/rust-lang/rust/blob/d695a497bbf4b20d2580b75075faa80230d41667/compiler/rustc_parse/src/parser/expr.rs#L2079 - https://github.com/rust-lang/rust/blob/d695a497bbf4b20d2580b75075faa80230d41667/compiler/rustc_parse/src/parser/expr.rs#L1569 + Tests: - https://github.com/rust-lang/rust/blob/master/src/test/ui/label/label_break_value_continue.rs - https://github.com/rust-lang/rust/blob/master/src/test/ui/label/label_break_value_unlabeled_break.rs - https://github.com/rust-lang/rust/blob/master/src/test/ui/label/label_break_value_illegal_uses.rs - https://github.com/rust-lang/rust/blob/master/src/test/ui/lint/unused_labels.rs - https://github.com/rust-lang/rust/blob/master/src/test/ui/run-pass/for-loop-while/label_break_value.rs ## Interactions with other features Labels follow the hygiene of local variables. label-break-value is permitted within `try` blocks: ```rust let _: Result<(), ()> = try { 'foo: { Err(())?; break 'foo; } }; ``` label-break-value is disallowed within closures, generators, and async blocks: ```rust 'a: { || break 'a //~^ ERROR use of unreachable label `'a` //~| ERROR `break` inside of a closure } ``` label-break-value is disallowed on [_BlockExpression_]; it can only occur as a [_LoopExpression_]: ```rust fn labeled_match() { match false 'b: { //~ ERROR block label not supported here _ => {} } } macro_rules! m { ($b:block) => { 'lab: $b; //~ ERROR cannot use a `block` macro fragment here unsafe $b; //~ ERROR cannot use a `block` macro fragment here |x: u8| -> () $b; //~ ERROR cannot use a `block` macro fragment here } } fn foo() { m!({}); } ``` [_BlockExpression_]: https://doc.rust-lang.org/nightly/reference/expressions/block-expr.html [_LoopExpression_]: https://doc.rust-lang.org/nightly/reference/expressions/loop-expr.html
…henkov Stabilize `#![feature(label_break_value)]` See the stabilization report in rust-lang/rust#48594 (comment).
|
r? @dswij (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ |
bors
added a commit
that referenced
this pull request
Aug 28, 2022
Rustup Hopefully this is done right. changelog: None
|
💔 Test failed - checks-action_test |
|
Wonderful. Disk space issues again. |
|
There doesn't seem to be much extra disk space used when testing locally. Is there a reason |
|
@bors try |
bors
added a commit
that referenced
this pull request
Aug 28, 2022
Rustup Hopefully this is done right. changelog: None
|
☀️ Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
|
The size of incremental builds looks to have gone up quite a bit recently. These are the sizes on windows after running
The sizes get worse as tests are run for each crate. The clippy_lints rlib is over 1GB in size for incremental builds and running tests for each crate results in multiple copies. For reference it's a little over 200MB for non-incremental builds. ping @flip1995 @Manishearth if we're ok with changing CI away from incremental builds. An alternative would be to somehow up the disk space given to the test runner. |
|
That makes sense! |
|
Then I'll go ahead with this. It can always be switched back later if need be. @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hopefully this is done right.
changelog: None