Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: execute finally block after break from try..finally in a loop (ref #12806) #13660

Closed
wants to merge 3 commits into from

Conversation

ihnorton
Copy link
Member

Replace break and continue statements inside of a try block with a labeled break to the finally handler, followed by a conditional unlabeled break, which will then be the intended loop-exit. Fixes #12806; new tests added from reworked examples there and in #1784.

I'd like some feedback on one consequence. Given the following example:

for i in 1:2
try
    break
catch
finally
    println("fin $1")
    i == 1 && continue
end
end

On master, the loop just exits -- the finally block is not executed (i.e., #12806).
With this patch, it is possible to continue after breaking to the finally block:

fin 1
fin 2

(without the continue, we only hit finally once, as expected)

@ihnorton ihnorton changed the title RFC: fix handling of 'break' in try block (ref #12806) RFC: execute finally block after break from try..finally in a loop (ref #12806) Oct 17, 2015
@JeffBezanson
Copy link
Member

Wow, doing continue inside finally is pretty evil. This behavior seems ok to me though.

finally
push!(a, 1)
end
@test x == 5
Copy link
Member

Choose a reason for hiding this comment

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

Where does x come from? I think the return above is causing these tests to be skipped.

@JeffBezanson
Copy link
Member

Fixed by #24075.

@DilumAluthge DilumAluthge deleted the ihn/fix12806 branch March 25, 2021 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants