Check for duplicate loop labels in function bodies. #24162
Conversation
|
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
|
(this isn't really done; at the very least, it needs a test or two, and I want to wait to see feedback from the |
|
Okay, added tests, and I guess the internals thread isn't going to get much action now. |
|
r+ modulo nit |
pnkfelix
force-pushed
the
fsk-detect-duplicate-loop-labels
branch
2 times, most recently
from
05d4881
to
816a500
Compare
|
@nikomatsakis Mind skimming over the additions from ed5ac77 ? |
pnkfelix
force-pushed
the
fsk-detect-duplicate-loop-labels
branch
3 times, most recently
from
096f296
to
46e0d94
Compare
|
@nikomatsakis this definitely needs a re-review now. |
pnkfelix
force-pushed
the
fsk-detect-duplicate-loop-labels
branch
from
46e0d94
to
f4c39cb
Compare
|
@pnkfelix r+ modulo nit (your call on whether to make an issue) This actually does feel cleaner to me. Doing the pre-pass to collect the labels is nice. |
pnkfelix
force-pushed
the
fsk-detect-duplicate-loop-labels
branch
from
f4c39cb
to
e6880ab
Compare
…labels, r=nikomatsakis Check for duplicate loop labels in function bodies. See also: http://internals.rust-lang.org/t/psa-rejecting-duplicate-loop-labels/1833 Fix rust-lang#21633
|
@bors r- (there's some test fallout I need to address) |
|
(no this still does not work...) |
|
(The main difficulty remaining is the handling of hygiene. My current approach to collapsing the rules so that labels are scoped to the entire function body does not play well with our (mis)handling of hygiene for lifetime-like things... at this point this might not happen for 1.0, in which case we will just have to live with the unfortunate scoping rules and work around them if we add e.g. |
pnkfelix
force-pushed
the
fsk-detect-duplicate-loop-labels
branch
from
fdb8414
to
aafed03
Compare
|
⌛ Testing commit 464e586 with merge 1fd19cf... |
|
💔 Test failed - auto-linux-64-nopt-t |
…ies. Note: this Warns rather than error on shadowing problems involving labels. We took this more conservative option mostly due to issues with hygiene being broken for labels and/or lifetimes. Add FIXME regarding non-hygienic comparison.
pnkfelix
force-pushed
the
fsk-detect-duplicate-loop-labels
branch
from
e46b32b
to
2b3cd40
Compare
|
long ago it did, but it would time out and such so https://github.com/rust-lang/rust/blob/master/.travis.yml#L17 |
|
@bors: rollup- |
…ikomatsakis Check for duplicate loop labels in function bodies. See also: http://internals.rust-lang.org/t/psa-rejecting-duplicate-loop-labels/1833 The change, which we are putting in as future-proofing in preparation for future potential additions to the language (namely labeling arbitrary blocks and using those labels in borrow expressions), means that code like this will start emitting warnings: ```rust fn main() { { 'a: loop { break; } } { 'a: loop { break; } } } ``` To make the above code compile without warnings, write this instead: ```rust fn main() { { 'a: loop { break; } } { 'b: loop { break; } } } ``` Since this change is only introducing a new warnings, this change is non-breaking. Fix #21633
steveklabnik
added
the
beta-nominated
|
Since this is future-proofing, I'm adding beta-nominated |
|
@bors: p=1 |
…abels Check for duplicate loop labels in function bodies. See also: http://internals.rust-lang.org/t/psa-rejecting-duplicate-loop-labels/1833 The change, which we are putting in as future-proofing in preparation for future potential additions to the language (namely labeling arbitrary blocks and using those labels in borrow expressions), means that code like this will start emitting warnings: ```rust fn main() { { 'a: loop { break; } } { 'a: loop { break; } } } ``` To make the above code compile without warnings, write this instead: ```rust fn main() { { 'a: loop { break; } } { 'b: loop { break; } } } ``` Since this change is only introducing a new warnings, this change is non-breaking. Fix rust-lang#21633
|
@steveklabnik wrote:
ah. Maybe we could try doing a "build" that just runs the type+borrow checking for a stage1 build, via Update: never mind, that would not suffice today, since |
pnkfelix
added
beta-accepted
|
i was not sure about whether to accept, but since it is already part of alex's rollup: going from nominated to (nominated, accepted) |
pnkfelix
added
the
beta-accepted
alexcrichton
removed
the
beta-nominated
ghost
mentioned this pull request
Check for duplicate loop labels in function bodies.
See also: http://internals.rust-lang.org/t/psa-rejecting-duplicate-loop-labels/1833
The change, which we are putting in as future-proofing in preparation for future potential additions to the language (namely labeling arbitrary blocks and using those labels in borrow expressions), means that code like this will start emitting warnings:
To make the above code compile without warnings, write this instead:
Since this change is only introducing a new warnings, this change is non-breaking.
Fix #21633