From 88a18de44f78bf0d0401070bb8cfbc90a78877ba Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Mon, 1 Jun 2026 22:21:37 +0800 Subject: [PATCH] fixes #25851; ensure --panics:on does not skip nimErr_ check after closure calls (#25855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes #25851 ## Summary: `--panics:on` drops `nimErr_` check after closure calls (#25851) ### Bug With `--exceptions:goto` and `--panics:on`, the compiler skipped the `nimErr_` check after indirect closure calls whose result flows directly into another call (e.g., `result.add elem(src)`). A raise inside the closure was silently swallowed — the loop continued, and the next `raise` hit the already-set `nimInErrorMode` flag, overflowing its `bool` storage into `OverflowDefect`. ### Root Cause **ast.nim** — `canRaise` checked `fn.typ.n[0].len < effectListLen` first (false after the expansion) and then `exceptionEffects != nil` (also false, nil), so it returned `false` — meaning "cannot raise." The C codegen trusted this and omitted the `nimErr_` check. ### Fix **ast.nim** — `canRaise` now treats `nil` `exceptionEffects` as "unknown → can raise" (`exceptionEffects == nil` as an additional true condition). This is defense-in-depth: even if some other path expands the list but leaves `exceptionEffects` nil (e.g., a type with `{.tags.}` but no `{.raises.}`), the error check is still emitted. ### Test tclosure_err_panic_goto.nim — exercises the double-trigger pattern (`drawBool` sets the error flag → closure call must propagate it) with `matrix: "; --panics:on"` covering both exception modes. --- compiler/ast.nim | 8 ++++-- tests/ccg/tclosure_err_panic_goto.nim | 36 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 tests/ccg/tclosure_err_panic_goto.nim diff --git a/compiler/ast.nim b/compiler/ast.nim index 5e9680a278..b04a102b20 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -1647,9 +1647,13 @@ proc canRaise*(fn: PNode): bool = if fn.typ.n[0].kind == nkSym: result = false else: + # A proc-typed value with no explicit raises slot still has + # unspecified effects, which sempass2 treats conservatively. + # Codegen needs to do the same in order to keep goto-exception + # checks after indirect/closure calls. result = ((fn.typ.n[0].len < effectListLen) or - (fn.typ.n[0][exceptionEffects] != nil and - fn.typ.n[0][exceptionEffects].safeLen > 0)) + fn.typ.n[0][exceptionEffects] == nil or + fn.typ.n[0][exceptionEffects].safeLen > 0) else: result = false diff --git a/tests/ccg/tclosure_err_panic_goto.nim b/tests/ccg/tclosure_err_panic_goto.nim new file mode 100644 index 0000000000..f1619ecff4 --- /dev/null +++ b/tests/ccg/tclosure_err_panic_goto.nim @@ -0,0 +1,36 @@ +discard """ + matrix: "; --panics:on" +""" +# issue #25851: --panics:on must not drop the nimErr_ check after a closure +# call whose result is consumed directly (e.g. `result.add elem(src)`). +# Regression from #25295. + +type + Overrun = object of CatchableError + Source = object + data: seq[bool] + cursor: int + ElemFn = proc(src: var Source): bool {.closure.} + +proc drawBool(src: var Source): bool = + if src.cursor >= src.data.len: raise newException(Overrun, "exhausted") + result = src.data[src.cursor]; inc src.cursor + +proc listRun(elem: ElemFn, src: var Source): seq[bool] = + result = @[] + while true: + if not src.drawBool(): break + result.add elem(src) # closure call – the result flows straight + # into `add`, which previously caused the + # compiler to skip the nimErr_ check. + +let elem: ElemFn = proc(src: var Source): bool = src.drawBool() + +# Both --panics:on and --panics:off must propagate the Overrun. +var caught = false +try: + var src = Source(data: @[true]) + discard listRun(elem, src) +except Overrun: + caught = true +doAssert caught, "Overrun exception was swallowed"