mirror of
https://github.com/nim-lang/Nim.git
synced 2026-05-25 06:18:16 +00:00
cbe02aa9de
## Bug
When an `except T as e:` handler in the cpp backend raises a new
exception, the enclosing `finally` block is silently dropped under
`--mm:arc` and `--mm:orc`:
```nim
proc main() =
try:
try:
raise newException(CatchableError, "orig")
except CatchableError as e:
echo "inner: ", e.msg
raise newException(CatchableError, "re:" & e.msg)
finally:
echo "finally"
except CatchableError as outer:
echo "outer: ", outer.msg
main()
```
Expected output:
```
inner: orig
finally
outer: re:orig
```
Actual output on `nim cpp --mm:arc` (and `--mm:orc`):
```
inner: orig
outer: re:orig
```
The `finally` line is missing. The bug is specific to memory managers
that use destructor injection (arc/orc); under `--mm:refc` the original
code path works correctly because no destructor wrapper is injected.
## Root cause
When the body of `except T as e:` is processed under ARC/ORC, the
destructor injection pass injects a compiler-generated `nkHiddenTryStmt`
wrapper around the handler body to call `=destroy` on `e` when it goes
out of scope. That wrapper sits at the top of `p.nestedTryStmts` with
`inExcept = false`.
`finallyActions` (which inlines the user-finally body before a raise
propagates) only inspected the topmost entry of `nestedTryStmts`.
Because the wrapper has `inExcept = false`, the check short-circuited
and the user's finally was never inlined.
After the raise, C++'s rule that sibling catch clauses do not catch each
other's throws means the surrounding `catch(...)/finally` emitted by
`genTryCpp` never runs either, so the user's finally is silently
dropped.
## Fix
- Add an `isHidden` flag to `nestedTryStmts` entries, set to `t.kind ==
nkHiddenTryStmt` so compiler-injected try wrappers can be distinguished
from user-written ones.
- In `finallyActions`, walk past `isHidden` wrappers but stop at the
first user try. If that user try is in its except branch with a finally,
inline the finally body before the raise; otherwise leave the raise
untouched (the raise will be caught by that user try's own except
branches and the inner finally will run via normal unwinding, which is
what already happens correctly under refc).
Walking past wrappers fixes the `as e` case under arc/orc. Stopping at
user trys preserves the existing correct behaviour for nested
try/except/finally constructs (e.g. `tests/exception/tfinally.nim`'s
`nested_finally`), which would otherwise see the outer finally inlined
too eagerly when an inner raise is processed.
## Tests
Adds `tests/exception/tcpp_handler_raise_finally.nim` covering:
- `except T as e:` re-raise + outer finally
- typeless `except:` re-raise + outer finally
- try/finally without except (exception propagation through finally)
The test runs on `--mm:arc`, `--mm:orc`, and `--mm:refc`.
Locally verified on both `devel` and `version-2-2`:
- `tests/exception/` — 42 PASS, 0 FAIL, 3 SKIP
- `tests/destructor/` — all PASS
- `tests/cpp/` — all PASS (single unrelated failure: `tasync_cpp.nim`
needs the `jester` package)
- `megatest` — PASS for both `--mm:arc` and `--mm:refc`, including the
previously regressing `tfinally.nim`'s `nested_finally`
## Backport
Tagged `[backport]` in the commit message for inclusion in
`version-2-2`.
---------
Co-authored-by: puffball1567 <17452514+puffball1567@users.noreply.github.com>
138 lines
3.0 KiB
Nim
138 lines
3.0 KiB
Nim
discard """
|
|
matrix: "--mm:refc; --mm:orc"
|
|
targets: "cpp"
|
|
output: '''
|
|
caught as std::exception
|
|
expected
|
|
finally1
|
|
finally2
|
|
finally2
|
|
2
|
|
expected
|
|
finally 1
|
|
finally 2
|
|
expected
|
|
cpp exception caught
|
|
'''
|
|
# doesn't work on macos 13 seemingly due to libc++ linking issue https://stackoverflow.com/a/77375947
|
|
disabled: osx
|
|
"""
|
|
|
|
type
|
|
std_exception* {.importcpp: "std::exception", header: "<exception>".} = object
|
|
std_runtime_error* {.importcpp: "std::runtime_error", header: "<stdexcept>".} = object
|
|
std_string* {.importcpp: "std::string", header: "<string>".} = object
|
|
|
|
proc constructStdString(s: cstring): std_string {.importcpp: "std::string(@)", constructor, header: "<string>".}
|
|
|
|
proc constructRuntimeError(s: stdstring): std_runtime_error {.importcpp: "std::runtime_error(@)", constructor.}
|
|
|
|
proc what(ex: std_runtime_error): cstring {.importcpp: "((char *)#.what())".}
|
|
|
|
proc myexception =
|
|
raise constructRuntimeError(constructStdString("cpp_exception"))
|
|
|
|
try:
|
|
myexception() # raise std::runtime_error
|
|
except std_exception:
|
|
echo "caught as std::exception"
|
|
try:
|
|
raise constructStdString("x")
|
|
except std_exception:
|
|
echo "should not happen"
|
|
except:
|
|
echo "expected"
|
|
|
|
doAssert(getCurrentException() == nil)
|
|
|
|
proc earlyReturn =
|
|
try:
|
|
try:
|
|
myexception()
|
|
finally:
|
|
echo "finally1"
|
|
except:
|
|
return
|
|
finally:
|
|
echo "finally2"
|
|
|
|
earlyReturn()
|
|
doAssert(getCurrentException() == nil)
|
|
|
|
|
|
try:
|
|
block blk1:
|
|
try:
|
|
raise newException(ValueError, "mmm")
|
|
except:
|
|
break blk1
|
|
except:
|
|
echo "should not happen"
|
|
finally:
|
|
echo "finally2"
|
|
|
|
doAssert(getCurrentException() == nil)
|
|
|
|
#--------------------------------------
|
|
|
|
# raise by pointer and also generic type
|
|
|
|
type
|
|
std_vector[T] {.importcpp"std::vector", header"<vector>".} = object
|
|
|
|
proc newVector[T](len: int): ptr std_vector[T] {.importcpp: "new std::vector<'1>(@)".}
|
|
proc deleteVector[T](v: ptr std_vector[T]) {.importcpp: "delete @; @ = NIM_NIL;".}
|
|
proc len[T](v: std_vector[T]): uint {.importcpp: "size".}
|
|
|
|
var v = newVector[int](2)
|
|
try:
|
|
try:
|
|
try:
|
|
raise v
|
|
except ptr std_vector[int] as ex:
|
|
echo len(ex[])
|
|
raise newException(ValueError, "msg5")
|
|
except:
|
|
echo "should not happen"
|
|
finally:
|
|
deleteVector(v)
|
|
except:
|
|
echo "expected"
|
|
|
|
doAssert(v == nil)
|
|
doAssert(getCurrentException() == nil)
|
|
|
|
#--------------------------------------
|
|
|
|
# mix of Nim and imported exceptions
|
|
try:
|
|
try:
|
|
try:
|
|
raise newException(KeyError, "msg1")
|
|
except KeyError:
|
|
raise newException(ValueError, "msg2")
|
|
except:
|
|
echo "should not happen"
|
|
finally:
|
|
echo "finally 1"
|
|
except:
|
|
doAssert(getCurrentExceptionMsg() == "msg2")
|
|
raise constructStdString("std::string")
|
|
finally:
|
|
echo "finally 2"
|
|
except:
|
|
echo "expected"
|
|
|
|
doAssert(getCurrentException() == nil)
|
|
|
|
try:
|
|
try:
|
|
myexception()
|
|
except std_runtime_error as ex:
|
|
echo "cpp exception caught"
|
|
raise newException(ValueError, "rewritten " & $ex.what())
|
|
except:
|
|
doAssert(getCurrentExceptionMsg() == "rewritten cpp_exception")
|
|
|
|
doAssert(getCurrentException() == nil)
|