Skip to content

[JuliaLowering] Fix break/continue in try#61525

Open
mlechu wants to merge 1 commit intoJuliaLang:masterfrom
mlechu:jl-trycatch-break
Open

[JuliaLowering] Fix break/continue in try#61525
mlechu wants to merge 1 commit intoJuliaLang:masterfrom
mlechu:jl-trycatch-break

Conversation

@mlechu
Copy link
Copy Markdown
Member

@mlechu mlechu commented Apr 7, 2026

Alternative to #61523, fix JuliaLang/JuliaLowering.jl#171.

Push to ctx.handler_token_stack before construction of the FinallyHandler, since the JumpTarget constructor needs it. The extra entry is ignored in enter_finally_block's call to emit_leave_handler, but not in the one in emit_break. These changes should directly match flisp.

I think we're still missing some pop_exceptions compared to flisp, but wanted to get this PR up given the other one. As noted in the tests, I need to port #55876, which duplicates the finally block and only adds pop_exception to one copy.

@mlechu mlechu requested a review from topolarity April 7, 2026 15:32
Copy link
Copy Markdown
Member

@topolarity topolarity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test coverage is better than mine 🙂

I also think this is much closer to what flisp does so let's take it

@test isempty(current_exceptions())

# TODO: commented out to avoid polluting the exception stack; need to port
# https://github.com/JuliaLang/julia/pull/55876
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should go ahead and do this now while we're fixing up try-finally?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally planned to, but this change should be OK on its own too. I need to port rename_assigned_ssavalues to properly copy finally blocks, which could potentially be a pain.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah darn, I was hoping our ssavar logic would handle that OK if you just lower it twice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

continue inside for does not respect finally

2 participants