Nim icon indicating copy to clipboard operation
Nim copied to clipboard

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

Open Menduist opened this issue 1 year ago • 6 comments

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 avatar Sep 15 '22 09:09 Menduist

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 avatar Sep 20 '22 17:09 Araq

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".

Araq avatar Sep 20 '22 17:09 Araq

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.

Menduist avatar Sep 20 '22 18:09 Menduist

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 avatar Sep 21 '22 08:09 Araq

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

Araq avatar Sep 21 '22 08:09 Araq

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

Menduist avatar Sep 21 '22 09:09 Menduist