Fix spans for continues and breaks #28117
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
|
Was the modified test broken by this change? It looks like that should probably still be passing? |
|
It still passes now, but may not be very future-proof: #28106 (comment) |
|
Could this leave around the test for now and just add a new one for the spans? |
|
The new test does the exact same thing as the old one. There's no test for the new changes AFAIK, as the spans have shrunk. |
|
Although these may hit the same code path they're testing two distinct things:
In general it's better to err on the side of more tests basically, so could this just add a new test instead of modifying an existing one? |
|
The test's original intention was never to check the span of the label, but to make sure that the keywords were included in the span: #28106 The changes to |
|
@alexcrichton I approve that test change, if that means anything. For all its worth the labels thing has its own issue: #28109, therefore the labels should eventually get a test for them anyway. |
The spans of break and continue would include the next token.
|
Reverted test, although I still believe that there is a misunderstanding here. |
Fixes #28108.