From 36f8cefa8508bbf75fca5a9b7d6f21138a08e812 Mon Sep 17 00:00:00 2001 From: Yuriy Glukhov Date: Tue, 8 Jul 2025 15:41:17 +0200 Subject: [PATCH] Fixes #21235, #23602, #24978, #25018 (#25030) Reworked closureiter transformation. - Convolutedly nested finallies should cause no problems now. - CurrentException state now follows nim runtime rules (pushes and pops appropriately), and mimics normal code, which is somewhat buggy, see #25031 - Previously state optimization (removing empty states or extra jumps) missed some opportunities, I've reimplemented it to do everything possible to optimize the states. At this point any extra states or jumps should be considered a bug. The resulting codegen (compiled binaries) is also slightly smaller. **BUT:** - I had to change C++ reraising logic, see expt.nim. Because with closure iters `currentException` is not always in sync with C++'s notion of current exception. From my tests and understanding of C++ runtime there should not be any problems, but I'm only 99% sure :) - I've reused `nfNoRewrite` flag in one specific case during the transformation. This flag is also used in term-rewriting logic. Again, 99% sure, these 2 scenarios will never intersect. --- compiler/closureiters.nim | 1001 +++++++++++++++++------------------- lib/system/embedded.nim | 3 - lib/system/excpt.nim | 9 +- lib/system/jssys.nim | 6 +- tests/async/t23602.nim | 27 + tests/iter/tyieldintry.nim | 216 +++++++- 6 files changed, 722 insertions(+), 540 deletions(-) create mode 100644 tests/async/t23602.nim diff --git a/compiler/closureiters.nim b/compiler/closureiters.nim index 835cbf0ca7..a422f66a8b 100644 --- a/compiler/closureiters.nim +++ b/compiler/closureiters.nim @@ -59,32 +59,32 @@ # If the iter has an nkTryStmt with a yield inside # - the closure iter is promoted to have exceptions (ctx.hasExceptions = true) # - exception table is created. This is a const array, where -# `abs(exceptionTable[i])` is a state idx to which we should jump from state +# `exceptionTable[i]` is exception landing state idx to which we should jump from state # `i` should exception be raised in state `i`. For all states in `try` block # the target state is `except` block. For all states in `except` block # the target state is `finally` block. For all other states there is no # target state (0, as the first block can never be neither except nor finally). -# `exceptionTable[i]` is < 0 if `abs(exceptionTable[i])` is except block, -# and > 0, for finally block. -# - local variable :curExc is created +# - env var :curExcLevel is created, finallies use it to decide their exit logic +# - if there are finallies, env var :finallyPath is created. It contains exit state labels +# for every finally level, and is changed in runtime in try, except, break, and continue +# nodes to control finally exit behavior. # - the iter body is wrapped into a +# var :tmp: Exception # try: -# closureIterSetupExc(:curExc) # ...body... # catch: # :state = exceptionTable[:state] # if :state == 0: raise # No state that could handle exception -# :unrollFinally = :state > 0 # Target state is finally -# if :state < 0: -# :state = -:state -# :curExc = getCurrentException() +# :tmp = getCurrentException() +# pushCurrentException(:tmp) # # nkReturnStmt within a try/except/finally now has to behave differently as we -# want the nearest finally block to be executed before the return, thus it is +# want parent finallies to be executed before the return, thus it is # transformed to: # :tmpResult = returnValue (if return doesn't have a value, this is skipped) -# :unrollFinally = true -# goto nearestFinally (or -1 if not exists) +# :finallyPath[0] = 0 # Finally at the bottom should just exit +# :finallyPath[N] = finallyNMinus1State # Next finally should exit to its parent +# goto finallyNState (or -1 if not exists) # finallyN is the nearest finally # # Example: # @@ -96,37 +96,44 @@ # return 3 # finally: # yield 2 +# somethingElse() # # Is transformed to (yields are left in place for example simplicity, # in reality the code is subdivided even more, as described above): # # case :state # of 0: # Try +# :finallyPath[LEVEL] = curExcLandingState # should exception occur our finally +# # must jump to its landing # yield 0 # raise ... -# :state = 2 # What would happen should we not raise +# :finallyPath[LEVEL] = 3 # Exception did not happen. Our finally can continue to state 3 +# :state = 2 # And we continue to our finally # break :stateLoop # of 1: # Except +# inc(:curExcLevel, -1) # Exception is caught # yield 1 # :tmpResult = 3 # Return -# :unrollFinally = true # Return +# :finalyPath[LEVEL] = 0 # Configure finally path. # :state = 2 # Goto Finally # break :stateLoop +# popCurrentException() # XXX: This is likely wrong, see #25031 # :state = 2 # What would happen should we not return # break :stateLoop # of 2: # Finally # yield 2 -# if :unrollFinally: # This node is created by `newEndFinallyNode` -# if :curExc.isNil: -# if nearestFinally == 0: -# return :tmpResult -# else: -# :state = nearestFinally # bubble up +# if :finallyPath[LEVEL] == 0: # This node is created by `newEndFinallyNode` +# if :curExcLevel == 0: +# :state = -1 +# return result = :tmpResult # else: -# closureIterSetupExc(nil) # raise -# state = -1 # Goto next state. In this case we just exit +# :state = :finallyPath[LEVEL] # Go to next state # break :stateLoop +# of 3: +# somethingElse() +# :state = -1 # Exit +# break :staleLoop # else: # return @@ -141,31 +148,39 @@ when defined(nimPreviewSlimSystem): import std/assertions type + FinallyTarget = object + n: PNode # nkWhileStmt, nkBlock, nkFinally + label: PNode # exit state for blocks and whiles (used by breaks), + # or enter state for finallies (used by breaks and returns) + + State = object + label: PNode # Int literal with state idx. It is filled after state split + body: PNode + excLandingState: PNode # label of exception landing state (except or finally) + inlinable: bool + deletable: bool + Ctx = object g: ModuleGraph fn: PSym tmpResultSym: PSym # Used when we return, but finally has to interfere - unrollFinallySym: PSym # Indicates that we're unrolling finally states (either exception happened or premature return) - curExcSym: PSym # Current exception + finallyPathSym: PSym + curExcLevelSym: PSym # Current exception level (because exceptions are stacked) - states: seq[tuple[label: int, body: PNode]] # The resulting states. - blockLevel: int # Temp used to transform break and continue stmts + states: seq[State] # The resulting states. Label is int literal. + finallyPathStack: seq[FinallyTarget] # Stack of split blocks, whiles and finallies stateLoopLabel: PSym # Label to break on, when jumping between states. - exitStateIdx: int # index of the last state tempVarId: int # unique name counter - tempVars: PNode # Temp var decls, nkVarSection - exceptionTable: seq[int] # For state `i` jump to state `exceptionTable[i]` if exception is raised hasExceptions: bool # Does closure have yield in try? - curExcHandlingState: int # Negative for except, positive for finally - nearestFinally: int # Index of the nearest finally block. For try/except it - # is their finally. For finally it is parent finally. Otherwise -1 + curExcLandingState: PNode # Negative for except, positive for finally + curFinallyLevel: int idgen: IdGenerator varStates: Table[ItemId, int] # Used to detect if local variable belongs to multiple states + finallyPathLen: PNode # int literal const nkSkip = {nkEmpty..nkNilLit, nkTemplateDef, nkTypeSection, nkStaticStmt, nkCommentStmt, nkMixinStmt, nkBindStmt, nkTypeOfExpr} + procDefs - emptyStateLabel = -1 localNotSeen = -1 localRequiresLifting = -2 @@ -178,11 +193,6 @@ proc newStateAssgn(ctx: var Ctx, toValue: PNode): PNode = # :state = toValue newTree(nkAsgn, ctx.newStateAccess(), toValue) -proc newStateAssgn(ctx: var Ctx, stateNo: int = -2): PNode = - # Creates state assignment: - # :state = stateNo - ctx.newStateAssgn(newIntTypeNode(stateNo, ctx.g.getSysType(TLineInfo(), tyInt))) - proc newEnvVar(ctx: var Ctx, name: string, typ: PType): PSym = result = newSym(skVar, getIdent(ctx.g.cache, name), ctx.idgen, ctx.fn, ctx.fn.info) result.typ = typ @@ -190,7 +200,6 @@ proc newEnvVar(ctx: var Ctx, name: string, typ: PType): PSym = assert(not typ.isNil, "Env var needs a type") let envParam = getEnvParam(ctx.fn) - # let obj = envParam.typ.lastSon result = addUniqueField(envParam.typ.elementType, result, ctx.g.cache, ctx.idgen) proc newEnvVarAccess(ctx: Ctx, s: PSym): PNode = @@ -204,29 +213,51 @@ proc newTmpResultAccess(ctx: var Ctx): PNode = ctx.tmpResultSym = ctx.newEnvVar(":tmpResult", ctx.fn.typ.returnType) ctx.newEnvVarAccess(ctx.tmpResultSym) -proc newUnrollFinallyAccess(ctx: var Ctx, info: TLineInfo): PNode = - if ctx.unrollFinallySym.isNil: - ctx.unrollFinallySym = ctx.newEnvVar(":unrollFinally", ctx.g.getSysType(info, tyBool)) - ctx.newEnvVarAccess(ctx.unrollFinallySym) +proc newArrayType(g: ModuleGraph; len: PNode, t: PType; idgen: IdGenerator; owner: PSym): PType = + result = newType(tyArray, idgen, owner) -proc newCurExcAccess(ctx: var Ctx): PNode = - if ctx.curExcSym.isNil: - ctx.curExcSym = ctx.newEnvVar(":curExc", ctx.g.callCodegenProc("getCurrentException").typ) - ctx.newEnvVarAccess(ctx.curExcSym) + let rng = newType(tyRange, idgen, owner) + rng.n = newTree(nkRange, g.newIntLit(owner.info, 0), len) + rng.rawAddSon(t) -proc newState(ctx: var Ctx, n, gotoOut: PNode): int = - # Creates a new state, adds it to the context fills out `gotoOut` so that it - # will goto this state. - # Returns index of the newly created state + result.rawAddSon(rng) + result.rawAddSon(t) - result = ctx.states.len - let resLit = ctx.g.newIntLit(n.info, result) - ctx.states.add((result, n)) - ctx.exceptionTable.add(ctx.curExcHandlingState) +proc newFinallyPathAccess(ctx: var Ctx, level: int, info: TLineInfo): PNode = + # ctx.:finallyPath[level] + let minPathLen = level + 1 + if ctx.finallyPathSym.isNil: + ctx.finallyPathLen = ctx.g.newIntLit(ctx.fn.info, minPathLen) + let ty = ctx.g.newArrayType(ctx.finallyPathLen, ctx.g.getSysType(ctx.fn.info, tyInt16), ctx.idgen, ctx.fn) + ctx.finallyPathSym = ctx.newEnvVar(":finallyPath", ty) + elif ctx.finallyPathLen.intVal < minPathLen: + ctx.finallyPathLen.intVal = minPathLen - if not gotoOut.isNil: - assert(gotoOut.len == 0) - gotoOut.add(ctx.g.newIntLit(gotoOut.info, result)) + result = newTreeIT(nkBracketExpr, info, ctx.g.getSysType(info, tyInt), + ctx.newEnvVarAccess(ctx.finallyPathSym), + ctx.g.newIntLit(ctx.fn.info, level)) + +proc newFinallyPathAssign(ctx: var Ctx, level: int, label: PNode, info: TLineInfo): PNode = + assert(label != nil) + let fp = newFinallyPathAccess(ctx, level, info) + result = newTree(nkAsgn, fp, label) + +proc newCurExcLevelAccess(ctx: var Ctx): PNode = + if ctx.curExcLevelSym.isNil: + ctx.curExcLevelSym = ctx.newEnvVar(":curExcLevel", ctx.g.getSysType(ctx.fn.info, tyInt16)) + ctx.newEnvVarAccess(ctx.curExcLevelSym) + +proc newStateLabel(ctx: Ctx): PNode = + ctx.g.newIntLit(TLineInfo(), 0) + +proc newState(ctx: var Ctx, n: PNode, inlinable: bool, label: PNode): PNode = + # Creates a new state, adds it to the context + # Returns label of the newly created state + result = label + if result.isNil: result = ctx.newStateLabel() + assert(result.kind == nkIntLit) + + ctx.states.add(State(label: result, body: n, excLandingState: ctx.curExcLandingState, inlinable: inlinable)) proc toStmtList(n: PNode): PNode = result = n @@ -267,57 +298,24 @@ proc hasYields(n: PNode): bool = result = true break -proc transformBreaksAndContinuesInWhile(ctx: var Ctx, n: PNode, before, after: PNode): PNode = - result = n - case n.kind - of nkSkip: - discard - of nkWhileStmt: discard # Do not recurse into nested whiles - of nkContinueStmt: - result = before - of nkBlockStmt: - inc ctx.blockLevel - result[1] = ctx.transformBreaksAndContinuesInWhile(result[1], before, after) - dec ctx.blockLevel - of nkBreakStmt: - if ctx.blockLevel == 0: - result = after - else: - for i in 0.. 0: + result = ctx.newJumpAlongFinallyChain(finallyChain, n.info) + else: + # Target is not in finally path means that it doesn't have yields (no state split), + # so we don't have to transform this break. + result = n + +proc transformReturnStmt(ctx: var Ctx, n: PNode): PNode = + # "Returning" involves jumping along all the cureent finally path. + # The last finally should exit to state 0 which is a special case for last exit + # (either return or propagating exception to the caller). + # It is eccounted for in newEndFinallyNode. + result = newNodeI(nkStmtList, n.info) + + # Returns prevent exception propagation + result.add(ctx.newNullifyCurExcLevel(n.info)) + + var finallyChain = newSeq[PNode]() + + for i in countdown(ctx.finallyPathStack.high, 0): + let b = ctx.finallyPathStack[i].n + # echo "STACK ", i, " ", b.kind + if b.kind == nkFinally: + finallyChain.add(ctx.finallyPathStack[i].label) + + if finallyChain.len > 0: + # Add proc exit state + finallyChain.add(ctx.g.newIntLit(n.info, 0)) if n[0].kind != nkEmpty: let asgnTmpResult = newNodeI(nkAsgn, n.info) @@ -894,23 +939,24 @@ proc transformReturnsInTry(ctx: var Ctx, n: PNode): PNode = asgnTmpResult.add(x) result.add(asgnTmpResult) - result.add(ctx.newNullifyCurExc(n.info)) + result.add(ctx.newJumpAlongFinallyChain(finallyChain, n.info)) + else: + # There are no (split) finallies on the path, so we can return right away + result.add(n) - let goto = newTree(nkGotoState, ctx.g.newIntLit(n.info, ctx.nearestFinally)) - result.add(goto) - - of nkSkip: - discard - of nkTryStmt: - if n.hasYields: - # the inner try will handle these transformations - discard - else: - for i in 0.. 0 and nfNoRewrite notin n.flags: + result = ctx.transformReturnStmt(n) else: for i in 0.. # :state = -1 # return e - # result = n case n.kind of nkStmtList, nkStmtListExpr: @@ -1116,9 +1176,9 @@ proc transformStateAssignments(ctx: var Ctx, n: PNode): PNode = discard of nkReturnStmt: - result = newNodeI(nkStmtList, n.info) - result.add(ctx.newStateAssgn(-1)) - result.add(n) + result = newTreeI(nkStmtList, n.info, + ctx.newStateAssgn(ctx.g.newIntLit(n.info, -1)), + n) of nkGotoState: result = newNodeI(nkStmtList, n.info) @@ -1132,147 +1192,68 @@ proc transformStateAssignments(ctx: var Ctx, n: PNode): PNode = for i in 0.. 0 - # if :state < 0: - # :state = -:state - # :curExc = getCurrentException() + for i in 0 .. ctx.states.high: + result.add(ctx.states[i].excLandingState) +proc newExceptBody(ctx: var Ctx, info: TLineInfo): PNode {.inline.} = + # Generates code: + # :state = exceptionTable[:state] + # if :state == 0: + # raise result = newNodeI(nkStmtList, info) let intTyp = ctx.g.getSysType(info, tyInt) let boolTyp = ctx.g.getSysType(info, tyBool) # :state = exceptionTable[:state] - block: - # exceptionTable[:state] - let getNextState = newTree(nkBracketExpr, - ctx.createExceptionTable(), - ctx.newStateAccess()) - getNextState.typ() = intTyp - - # :state = exceptionTable[:state] - result.add(ctx.newStateAssgn(getNextState)) + result.add ctx.newStateAssgn( + newTreeIT(nkBracketExpr, info, intTyp, + ctx.createExceptionTable(), + ctx.newStateAccess())) # if :state == 0: raise block: - let cond = newTree(nkCall, + let cond = newTreeIT(nkCall, info, boolTyp, ctx.g.getSysMagic(info, "==", mEqI).newSymNode(), ctx.newStateAccess(), newIntTypeNode(0, intTyp)) - cond.typ() = boolTyp let raiseStmt = newTree(nkRaiseStmt, ctx.g.emptyNode) let ifBranch = newTree(nkElifBranch, cond, raiseStmt) let ifStmt = newTree(nkIfStmt, ifBranch) result.add(ifStmt) - # :unrollFinally = :state > 0 - block: - let cond = newTree(nkCall, - ctx.g.getSysMagic(info, "<", mLtI).newSymNode, - newIntTypeNode(0, intTyp), - ctx.newStateAccess()) - cond.typ() = boolTyp - - let asgn = newTree(nkAsgn, ctx.newUnrollFinallyAccess(info), cond) - result.add(asgn) - - # if :state < 0: :state = -:state - block: - let cond = newTree(nkCall, - ctx.g.getSysMagic(info, "<", mLtI).newSymNode, - ctx.newStateAccess(), - newIntTypeNode(0, intTyp)) - cond.typ() = boolTyp - - let negateState = newTree(nkCall, - ctx.g.getSysMagic(info, "-", mUnaryMinusI).newSymNode, - ctx.newStateAccess()) - negateState.typ() = intTyp - - let ifBranch = newTree(nkElifBranch, cond, ctx.newStateAssgn(negateState)) - let ifStmt = newTree(nkIfStmt, ifBranch) - result.add(ifStmt) - - # :curExc = getCurrentException() - block: - result.add(newTree(nkAsgn, - ctx.newCurExcAccess(), - ctx.g.callCodegenProc("getCurrentException"))) - proc wrapIntoTryExcept(ctx: var Ctx, n: PNode): PNode {.inline.} = - let setupExc = newTree(nkCall, - newSymNode(ctx.g.getCompilerProc("closureIterSetupExc")), - ctx.newCurExcAccess()) + # Generates code: + # var :tmp = nil + # try: + # body + # except: + # :state = exceptionTable[:state] + # if :state == 0: + # raise + # :tmp = getCurrentException() + # + # pushCurrentException(:tmp) - let tryBody = newTree(nkStmtList, setupExc, n) - let exceptBranch = newTree(nkExceptBranch, ctx.newCatchBody(ctx.fn.info)) + let tryBody = newTree(nkStmtList, n) + let exceptBody = ctx.newExceptBody(ctx.fn.info) + let exceptBranch = newTree(nkExceptBranch, exceptBody) - result = newTree(nkTryStmt, tryBody, exceptBranch) + result = newTree(nkStmtList) + let getCurExc = ctx.g.callCodegenProc("getCurrentException") + let tempExc = ctx.newTempVar(getCurExc.typ, result) + result.add newTree(nkTryStmt, tryBody, exceptBranch) + exceptBody.add ctx.newTempVarAsgn(tempExc, getCurExc) + + result.add newTree(nkCall, newSymNode(ctx.g.getCompilerProc("pushCurrentException")), ctx.newTempVarAccess(tempExc)) + result.add ctx.newChangeCurExcLevel(n.info, 1) proc wrapIntoStateLoop(ctx: var Ctx, n: PNode): PNode = # while true: @@ -1295,141 +1276,106 @@ proc wrapIntoStateLoop(ctx: var Ctx, n: PNode): PNode = blockStmt.add(blockBody) loopBody.add(blockStmt) -proc deleteEmptyStates(ctx: var Ctx) = - let goOut = newTree(nkGotoState, ctx.g.newIntLit(TLineInfo(), -1)) - ctx.exitStateIdx = ctx.newState(goOut, nil) - - # Apply new state indexes and mark unused states with -1 - var iValid = 0 - for i, s in ctx.states.mpairs: - let body = skipStmtList(ctx, s.body) - if body.kind == nkGotoState and i != ctx.states.len - 1 and i != 0: - # This is an empty state. Mark with -1. - s.label = emptyStateLabel +proc countStateOccurences(ctx: var Ctx, n: PNode, stateOccurences: var openArray[int]) = + ## Find all nkGotoState(stateIdx) nodes that do not follow nkYield. + ## For every such node increment stateOccurences[stateIdx] + for i, c in n: + if c.kind == nkGotoState and c[0].kind == nkIntLit and (i > 0 and n[i - 1].kind != nkYieldStmt): + let stateIdx = c[0].intVal + if stateIdx >= 0: + inc stateOccurences[stateIdx] + elif c.kind == nkIntLit: + let idx = c.intVal + if idx >= 0 and idx < ctx.states.len and ctx.states[idx].label == c: + ctx.states[idx].inlinable = false else: - s.label = iValid - inc iValid + ctx.countStateOccurences(c, stateOccurences) - for i, s in ctx.states: - let body = skipStmtList(ctx, s.body) - if body.kind != nkGotoState or i == 0: - discard ctx.skipThroughEmptyStates(s.body) - let excHandlState = ctx.exceptionTable[i] - if excHandlState < 0: - ctx.exceptionTable[i] = -ctx.skipEmptyStates(-excHandlState) - elif excHandlState != 0: - ctx.exceptionTable[i] = ctx.skipEmptyStates(excHandlState) +proc replaceDeletedStates(ctx: var Ctx, n: PNode): PNode = + result = n + for i in 0 ..< n.safeLen: + let c = n[i] + if c.kind == nkIntLit: + let idx = c.intVal + if idx >= 0 and idx < ctx.states.len and ctx.states[idx].label == c and ctx.states[idx].deletable: + let gt = ctx.replaceDeletedStates(skipStmtList(ctx.states[idx].body)) + assert(gt.kind == nkGotoState) + n[i] = gt[0] + else: + n[i] = ctx.replaceDeletedStates(c) - var i = 1 # ignore the entry and the exit - while i < ctx.states.len - 1: - if ctx.states[i].label == emptyStateLabel: +proc replaceInlinedStates(ctx: var Ctx, n: PNode): PNode = + ## Find all nkGotoState(stateIdx) nodes that do not follow nkYield. + ## For every such node increment stateOccurences[stateIdx] + result = n + for i in 0 ..< n.safeLen: + let c = n[i] + if c.kind == nkGotoState and c[0].kind == nkIntLit and (i > 0 and n[i - 1].kind != nkYieldStmt): + let stateIdx = c[0].intVal + if stateIdx >= 0: + if ctx.states[stateIdx].inlinable: + n[i] = ctx.states[stateIdx].body + else: + n[i] = ctx.replaceInlinedStates(c) + +proc optimizeStates(ctx: var Ctx) = + # Optimize empty states away and inline inlinable states + # This step requires that unique indexes are already assigned to state labels + + # Find empty states (those consisting only of gotoState node) and mark + # them deletable. + for i in 0 .. ctx.states.high: + let s = ctx.states[i] + let body = skipStmtList(s.body) + if body.kind == nkGotoState and body[0].kind == nkIntLit and body[0].intVal >= 0: + ctx.states[i].deletable = true + + # Replace deletable state labels to labels of respective non-empty states + for i in 0 .. ctx.states.high: + ctx.states[i].body = ctx.replaceDeletedStates(ctx.states[i].body) + + # Remove deletable states + var i = 0 + while i < ctx.states.len: + if ctx.states[i].deletable: ctx.states.delete(i) - ctx.exceptionTable.delete(i) else: inc i -type - PreprocessContext = object - finallys: seq[PNode] - config: ConfigRef - blocks: seq[(PNode, int)] - idgen: IdGenerator - FreshVarsContext = object - tab: Table[int, PSym] - config: ConfigRef - info: TLineInfo - idgen: IdGenerator + # Reassign state label indexes + for i in 0 .. ctx.states.high: + ctx.states[i].label.intVal = i -proc freshVars(n: PNode; c: var FreshVarsContext): PNode = - case n.kind - of nkSym: - let x = c.tab.getOrDefault(n.sym.id) - if x == nil: - result = n + # Count state occurences + var stateOccurences = newSeq[int](ctx.states.len) + for s in ctx.states: + ctx.countStateOccurences(s.body, stateOccurences) + + # If there are inlinable states refered not exactly once, prevent them from inlining + for i, o in stateOccurences: + if o != 1: + ctx.states[i].inlinable = false + + # echo "States to optimize:" + # for i, s in ctx.states: + # if s.deletable: echo i, ": delete" + # elif s.inlinable: echo i, ": inline" + + # Inline states + for i in 0 .. ctx.states.high: + ctx.states[i].body = ctx.replaceInlinedStates(ctx.states[i].body) + + # Remove inlined states + i = 0 + while i < ctx.states.len: + if ctx.states[i].inlinable: + ctx.states.delete(i) else: - result = newSymNode(x, n.info) - of nkSkip - {nkSym}: - result = n - of nkLetSection, nkVarSection: - result = copyNode(n) - for it in n: - if it.kind in {nkIdentDefs, nkVarTuple}: - let idefs = copyNode(it) - for v in 0..it.len-3: - if it[v].kind == nkSym: - let x = copySym(it[v].sym, c.idgen) - c.tab[it[v].sym.id] = x - idefs.add newSymNode(x) - else: - idefs.add it[v] + inc i - for rest in it.len-2 ..< it.len: idefs.add it[rest] - result.add idefs - else: - result.add it - of nkRaiseStmt: - result = nil - localError(c.config, c.info, "unsupported control flow: 'finally: ... raise' duplicated because of 'break'") - else: - result = n - for i in 0..= 0: - result = newNodeI(nkStmtList, n.info) - for i in countdown(c.finallys.high, fin): - var vars = FreshVarsContext(tab: initTable[int, PSym](), config: c.config, info: n.info, idgen: c.idgen) - result.add freshVars(copyTree(c.finallys[i]), vars) - c.idgen = vars.idgen - result.add n - of nkSkip: discard - else: - for i in 0 ..< n.len: - result[i] = preprocess(c, n[i]) + # Reassign state label indexes one last time + for i in 0 .. ctx.states.high: + ctx.states[i].label.intVal = i proc detectCapturedVars(c: var Ctx, n: PNode, stateIdx: int) = case n.kind @@ -1506,26 +1452,29 @@ proc transformClosureIterator*(g: ModuleGraph; idgen: IdGenerator; fn: PSym, n: # is performed, so that the closure iter environment is always created upfront. doAssert(getEnvParam(fn) != nil, "Env param not created before iter transformation") + ctx.curExcLandingState = ctx.newStateLabel() ctx.stateLoopLabel = newSym(skLabel, getIdent(ctx.g.cache, ":stateLoop"), idgen, fn, fn.info) - var pc = PreprocessContext(finallys: @[], config: g.config, idgen: idgen) - var n = preprocess(pc, n.toStmtList) - #echo "transformed into ", n - #var n = n.toStmtList + var n = n.toStmtList + # echo "transformed into ", n - discard ctx.newState(n, nil) + discard ctx.newState(n, false, nil) let gotoOut = newTree(nkGotoState, g.newIntLit(n.info, -1)) var ns = false n = ctx.lowerStmtListExprs(n, ns) + # echo "LOWERED: ", renderTree(n) if n.hasYieldsInExpressions(): - internalError(ctx.g.config, "yield in expr not lowered") + internalError(ctx.g.config, n.info, "yield in expr not lowered") # Splitting transformation discard ctx.transformClosureIteratorBody(n, gotoOut) - # Optimize empty states away - ctx.deleteEmptyStates() + # Assign state label indexes + for i in 0 .. ctx.states.high: + ctx.states[i].label.intVal = i + + ctx.optimizeStates() let caseDispatcher = newTreeI(nkCaseStmt, n.info, ctx.newStateAccess()) @@ -1536,7 +1485,7 @@ proc transformClosureIterator*(g: ModuleGraph; idgen: IdGenerator; fn: PSym, n: for s in ctx.states: let body = ctx.transformStateAssignments(s.body) - caseDispatcher.add newTreeI(nkOfBranch, body.info, g.newIntLit(body.info, s.label), body) + caseDispatcher.add newTreeI(nkOfBranch, body.info, s.label, body) caseDispatcher.add newTreeI(nkElse, n.info, newTreeI(nkReturnStmt, n.info, g.emptyNode)) @@ -1544,11 +1493,11 @@ proc transformClosureIterator*(g: ModuleGraph; idgen: IdGenerator; fn: PSym, n: result = liftLocals(ctx, result) when false: - echo "TRANSFORM TO STATES: " + echo "TRANSFORM TO STATES:" echo renderTree(result) - echo "exception table:" - for i, e in ctx.exceptionTable: - echo i, " -> ", e + # echo "exception table:" + # for i, s in ctx.states: + # echo i, " -> ", s.excLandingState - echo "ENV: ", renderTree(getEnvParam(fn).typ.elementType.n) + # echo "ENV: ", renderTree(getEnvParam(fn).typ.elementType.n) diff --git a/lib/system/embedded.nim b/lib/system/embedded.nim index b3febe7849..5abbdef248 100644 --- a/lib/system/embedded.nim +++ b/lib/system/embedded.nim @@ -50,9 +50,6 @@ proc writeStackTrace() = discard proc unsetControlCHook() = discard proc setControlCHook(hook: proc () {.noconv.}) = discard -proc closureIterSetupExc(e: ref Exception) {.compilerproc, inline.} = - sysFatal(ReraiseDefect, "exception handling is not available") - when gotoBasedExceptions: var nimInErrorMode {.threadvar.}: bool diff --git a/lib/system/excpt.nim b/lib/system/excpt.nim index dae5c4a4a1..5563b1adf0 100644 --- a/lib/system/excpt.nim +++ b/lib/system/excpt.nim @@ -161,9 +161,6 @@ proc popCurrentException {.compilerRtl, inl.} = proc popCurrentExceptionEx(id: uint) {.compilerRtl.} = discard "only for bootstrapping compatbility" -proc closureIterSetupExc(e: ref Exception) {.compilerproc, inline.} = - currException = e - # some platforms have native support for stack traces: const nativeStackTraceSupported = (defined(macosx) or defined(linux)) and @@ -464,11 +461,9 @@ proc raiseExceptionAux(e: sink(ref Exception)) {.nodestroy.} = if globalRaiseHook != nil: if not globalRaiseHook(e): return when defined(cpp) and not defined(noCppExceptions) and not gotoBasedExceptions: - if e == currException: - {.emit: "throw;".} - else: + if e != currException: pushCurrentException(e) - {.emit: "throw `e`;".} + {.emit: "throw `e`;".} elif quirkyExceptions or gotoBasedExceptions: pushCurrentException(e) when gotoBasedExceptions: diff --git a/lib/system/jssys.nim b/lib/system/jssys.nim index 3b995f69b1..3e2ad9ec24 100644 --- a/lib/system/jssys.nim +++ b/lib/system/jssys.nim @@ -72,8 +72,10 @@ proc getCurrentExceptionMsg*(): string = proc setCurrentException*(exc: ref Exception) = lastJSError = cast[PJSError](exc) -proc closureIterSetupExc(e: ref Exception) {.compilerproc, inline.} = - ## Used to set up exception handling for closure iterators +proc pushCurrentException(e: sink(ref Exception)) {.compilerRtl, inline.} = + ## Used to set up exception handling for closure iterators. + + # XXX Shouldn't there be exception stack like in excpt.nim? setCurrentException(e) proc auxWriteStackTrace(f: PCallFrame): string = diff --git a/tests/async/t23602.nim b/tests/async/t23602.nim new file mode 100644 index 0000000000..600e97a7c8 --- /dev/null +++ b/tests/async/t23602.nim @@ -0,0 +1,27 @@ +import std/asyncdispatch + +proc errval {.async.} = + raise newException(ValueError, "err") + +proc err {.async.} = + try: + doAssert false + finally: + echo "finally" + # removing the following code will propagate the AssertionDefect + try: + await errval() + except ValueError: + echo "valueError" + +proc main {.async.} = + let errFut = err() + await errFut + +var ok = false +try: + waitFor main() +except AssertionDefect: + ok = true + +doAssert(ok) diff --git a/tests/iter/tyieldintry.nim b/tests/iter/tyieldintry.nim index 04409795b0..a2e8f25651 100644 --- a/tests/iter/tyieldintry.nim +++ b/tests/iter/tyieldintry.nim @@ -26,10 +26,10 @@ proc testClosureIterAux(it: iterator(): int, exceptionExpected: bool, expectedRe if closureIterResult != @expectedResults or exceptionCaught != exceptionExpected: if closureIterResult != @expectedResults: echo "Expected: ", @expectedResults - echo "Actual: ", closureIterResult + echo "Actual: ", closureIterResult if exceptionCaught != exceptionExpected: echo "Expected exception: ", exceptionExpected - echo "Got exception: ", exceptionCaught + echo "Got exception: ", exceptionCaught doAssert(false) proc test(it: iterator(): int, expectedResults: varargs[int]) = @@ -182,6 +182,57 @@ block: test(it, 0, 1, 2, 3) +block: # Wrong except + iterator it(): int {.closure.} = + try: + try: + yield 0 + raiseTestError() + except ValueError: + doAssert(false, "Unreachable") + finally: + checkpoint(1) + except ValueError: + yield 123 + return + + checkpoint(123) + + testExc(it, 0, 1) + +block: # Nested except without finally + iterator it(): int {.closure.} = + try: + try: + yield 0 + raiseTestError() + except ValueError: + doAssert(false, "Unreachable") + except ValueError: + yield 123 + + checkpoint(123) + + testExc(it, 0) + +block: # Return in except with no finallies around + iterator it(): int {.closure.} = + try: + try: + yield 0 + raiseTestError() + except ValueError: + doAssert(false, "Unreachable") + finally: + checkpoint(1) + except TestError: + yield 2 + return + + checkpoint(123) + + test(it, 0, 1, 2) + block: iterator it(): int {.closure.} = try: @@ -527,3 +578,164 @@ block: # Locals present in only 1 state should be on the stack yield a yield b test(it, 1, 2) + +block: # Complex finallies (#24978) + iterator it(): int {.closure.} = + try: + for i in 1..2: + try: + yield i + 10 + except: + doAssert(false, "Should not get here") + checkpoint(i + 20) + raiseTestError() + finally: + for i in 3..4: + try: + yield i + 30 + except: + doAssert(false, "Should not get here") + finally: + checkpoint(i + 40) + checkpoint(i + 50) + checkpoint(100) + + testExc(it, 11, 21, 12, 22, 33, 43, 53, 34, 44, 54, 100) + +block: # break + iterator it(): int {.closure.} = + while true: + try: + yield 1 + while true: + yield 2 + if true: + break + break + finally: + var localHere = 3 + checkpoint(localHere) + + test(it, 1, 2, 3) + +block: # break + iterator it(): int {.closure.} = + while true: + try: + try: + yield 1 + while true: + yield 2 + break + break + finally: + var localHere = 4 + yield 3 + checkpoint(localHere) + doAssert(false, "Should not get here") + finally: + yield 5 + checkpoint(6) + doAssert(false, "Should not reach here") + + test(it, 1, 2, 3, 4, 5, 6) + +block: # continue + iterator it(): int {.closure.} = + for i in 1 .. 3: + try: + try: + yield i + 10 + while true: + yield i + 20 + break + if i == 2: + continue + checkpoint(i + 30) + finally: + yield 3 + checkpoint(4) + checkpoint(5) + finally: + yield 6 + checkpoint(7) + + test(it, 11, 21, 31, 3, 4, 5, 6, 7, 12, 22, 3, 4, 6, 7, 13, 23, 33, 3, 4, 5, 6, 7) + +block: # return without finally + iterator it(): int {.closure.} = + try: + yield 1 + if true: + return + except: + doAssert(false, "Unreachable") + yield 2 + + test(it, 1) + +block: # return in finally + iterator it(): int {.closure.} = + try: + yield 1 + except: + doAssert(false, "Unreachable") + finally: + return + yield 2 + + test(it, 1) + +block: # launch iter with current exception + iterator it(): int {.closure.} = + try: + yield 1 + finally: + discard + + try: + raise newException(ValueError, "") + except: + test(it, 1) + +block: #21235 + proc myFunc() = + iterator myFuncIter(): int {.closure.} = + if false: + try: + yield 5 + except: + discard + var nameIterVar = myFuncIter + discard nameIterVar() + + var ok = false + try: + try: + raise ValueError.newException("foo") + finally: + myFunc() + except ValueError: + ok = true + doAssert(ok) + +block: # break in for without yield in try + iterator it(): int {.closure.} = + try: + block: + checkpoint(1) + for i in 0 .. 10: + checkpoint(2) + break + checkpoint(3) + + try: + yield 4 + except: + checkpoint(123) + except: + discard + finally: + checkpoint(5) + + test(it, 1, 2, 3, 4, 5)