Fix ICE when there is a continue in a labeled block

[SarthakSingh31]

Fixes #113379
Fixes #121623

r? @matthiaskrgr

[compiler-errors]

r? compiler

This needs an explanation for why it's correct. I'm also somewhat concerned that this could affect good-path code.

I'd prefer if we did something else, like lower continue to hir::ExprKind::Error when it's in a scope that doesn't allow breaks.

[SarthakSingh31]

@compiler-errors I updated the pr, now when the ast is being lowered it checks if the label belongs to a loop and if it does not it will lower to an error.
Should I remove this now redundant check:

rust/compiler/rustc_passes/src/loops.rs

Lines 189 to 194 in 6cbf092

	if let Node::Block(block) = self.tcx.hir_node(loop_id) {
	self.sess.dcx().emit_err(ContinueLabeledBlock {
	span: e.span,
	block_span: block.span,
	});
	}

[bors]

☔ The latest upstream changes (presumably #121780) made this pull request unmergeable. Please resolve the merge conflicts.

[cjgillot]

I'm surprised we weren't emitting such an error anywhere before. What about break on non-loop blocks?

[cjgillot]

Should we do the same thing with break?

[SarthakSingh31]

We are doing the same for break here https://github.com/rust-lang/rust/blob/master/compiler/rustc_ast_lowering/src/expr.rs#L1455

[SarthakSingh31]

wait, maybe we are not doing it. This just assumes that the label is a loop https://github.com/rust-lang/rust/blob/master/compiler/rustc_ast_lowering/src/expr.rs#L1446-L1448

[SarthakSingh31]

I'm surprised we weren't emitting such an error anywhere before.

@cjgillot We were emitting this error but just much later. #121682 (comment)

As asked #121682 (comment) I moved the error to when the ast is lowered.

I would also like your opinion on if it would be better to lower it to a hir::ExprKind::Continue but the destination has a hir::LoopIdError of something like label does not refer to a loop.

[SarthakSingh31]

@rustbot label -S-waiting-on-author +S-waiting-on-review

[cjgillot]

I would also like your opinion on if it would be better to lower it to a hir::ExprKind::Continue but the destination has a hir::LoopIdError of something like label does not refer to a loop.

Using ExprKind::Err is fine, that's what it's meant to be.

For break, I think some uniformity would be better. If is allows us to remove some error handling code somewhere else, that's all the better.

[bors]

☔ The latest upstream changes (presumably #123468) made this pull request unmergeable. Please resolve the merge conflicts.

[oskgo]

@SarthakSingh31 any updates on this? thanks

[SarthakSingh31]

@oskgo I had forgotten about this PR, going to try to fix it up in the next few days.

[SarthakSingh31]

@cjgillot I have removed the redundant error. This should be ready to merge.

This does not change the break code as the changes made here check if a label points to a block or a loop. This is important for continue but for break it does not matter in this case.

[bors]

☔ The latest upstream changes (presumably #125443) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #128614) made this pull request unmergeable. Please resolve the merge conflicts.

[cjgillot]

r=me after rebase

[alex-semenyuk]

@SarthakSingh31
From wg-triage. Could you please do rebase? it seems no unhandled comments here, right?

[alex-semenyuk]

@SarthakSingh31
From wg-triage. Closed this PR due to inactivity. Feel free to reopen or raised new one. Thanks for your efforts.