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

Closure iterator ignores "return" if exception is raised and caught in finally #20362

Open
Menduist opened this issue Sep 15, 2022 · 6 comments

Comments

@Menduist
Copy link
Contributor

Menduist commented Sep 15, 2022

What happened?

# proc version
proc b: int =
  try:
    return 5
  finally:
    try:
      raise newException(IOError, "")
    except:
      discard
  echo "hey proc"

# doesn't print anything
discard b()

# closure iterator version
iterator a: int {.closure.} =
  try:
    yield 5
    return 5
  finally:
    try:
      yield 10 # just to force transformation
      raise newException(IOError, "")
    except:
      discard
  echo "hey iterator"

# prints "hey iterator"
for _ in a(): discard

Nim Version

Every nim version

Current Standard Output Logs

hey iterator

Expected Standard Output Logs

[nothing]

Possible Solution

unrollFinally always get overriden in the except branch of the closure iterator transformation, but here it should stay true.

@Menduist Menduist changed the title Closure iterator transformation ignores "return" if exception is raised and caught in finally Closure iterator ignores "return" if exception is raised and caught in finally Sep 15, 2022
@Araq
Copy link
Member

Araq commented Sep 20, 2022

I fail to see the bug here. When it reaches

    try:
      yield 10 # just to force transformation
      raise newException(IOError, "")
    except:
      discard

it first yields 10. On the next iteration raise newException is triggered which is immediately caught by except: discard that consumes the exception. Execution continues with echo "hey iterator" because only after that the iterator is actually done.

@Araq
Copy link
Member

Araq commented Sep 20, 2022

Or in other words

  try:
    return 5
  finally:
    try:
      yield 10

is undefined behavior as the finally section should run after the return regardless but the finally contains a yield so it's like an "aborted return" instruction as the yield says "do continue afterwards" and the return says "final iteration".

@Menduist
Copy link
Contributor Author

Menduist commented Sep 20, 2022

so it's like an "aborted return" instruction as the yield says "do continue afterwards"

I don't think yield is supposed to "abort a return" or "do continue afterwards"
Otherwise:

iterator a: int {.closure.} =
  try:
    yield 5
    return 5
  finally:
    yield 10 # should "abort return"
  echo "hey iterator" # should run, but doesn't since return isn't aborted

But that's not the case

imo, we should have exactly the same flow between an iterator with no yield, and one with yields everywhere.

@Araq
Copy link
Member

Araq commented Sep 21, 2022

But that's not the case

Interesting. I thought that one would abort the return too. Well and it does, doesn't it:

iterator a2: int {.closure.} =
  try:
    yield 5
    return 5
  finally:
    echo "A"
    yield 10 # should "abort return"
    echo "B"
  echo "hey iterator" # should run, but doesn't since return isn't aborted

for x in a2(): echo x

produces:

5
A
10
B

@Araq
Copy link
Member

Araq commented Sep 21, 2022

The semantics are "after a return run the finally section including its yield instructions". That seems pretty consistent and reasonable to me.

@Menduist
Copy link
Contributor Author

Menduist commented Sep 21, 2022

The semantics are "after a return run the finally section including its yield instructions". That seems pretty consistent and reasonable to me.

Indeed. However, if an exception is raised & swallowed in the finally, the return does get aborted:

iterator a2: int {.closure.} =
  try:
    yield 5
    return 5
  finally:
    echo "A"
    try:
      # "if true" to disarm the unreachable code error.
      # switching to "if false" gives correct output (no "hey iterator")
      # since the exception isn't raised
      if true: raise newException(IOError, "")
      yield 10
    except: discard
    echo "B"
  echo "hey iterator"

for x in a2(): echo x

Output:

5
A
B
hey iterator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants