Tags: nim-works/nimskull
Tags
mirgen: lower `finally` (#1468) ## Summary * simplify MIR control-flow constructs * lower `finally` into a `block` + `case` dispatcher in `mirgen` * emit exception stack management code in `mirgen` already * inline scope cleanup directly at `break` and `return` ## Details The goals are to: * use a unified exception runtime across all backends * make the MIR simpler * reduce the complexity of the language reaching the code generators, and thus the amount of translation work for the code generators For the MIR: * target lists are removed; jumps only need to specify a single target * `Finally` can only be used as the target for exceptional jumps; unstructured control-flow out of a `Finally` section is disallowed * only a single exceptional target can be specified for `Continue` * `Raise` is decoupled from exception management; it only initiates unwinding now The various MIR processing, rendering, and translation is adjusted accordingly. The CGIR equivalents to the MIR constructs change in the same way. Since `Finally` can no longer be used with non-exceptional control-flow (`Goto`, `Case`, etc.), translation of both scope cleanup and `finally` needs to change in `mirgen`. The exception runtime calls for managing the exception stack also have to be injected in `mirgen` already. ### `finally` Lowering `finally` is translated into continuation passing. However, instead of reifying the `finally` into a standalone procedure, all "invocations" of (read, jumps to) the `finally` record their original destination in a local variable, which a dispatcher emitted at the end of the `finally` clause then uses to jump to the target (or another intercepting `finally`). ### Scope Cleanup Scope cleanup in response to an exception being raised still uses `Finally`. For cleanup in response to `break` or `return`, all necessary cleanup is emitted directly before the `Goto`. This increases the workload for the move analysis / destructor elision pass, but it also results in more efficient code (since there are usually less dynamic invocations of destructors). ### Code Generation All three code generators are updated to consider the new syntax and semantics of `Finally`, `Continue`, etc. Notably: * `ccgflow` is obsolete and thus removed; code generation for `Finally`, `Continue`, etc. is now simple enough to implement directly in `cgen` * `jsgen` now translates `Finally` to a JavaScript `catch` clause (so as to not interfere with `break`s), which simplifies code generation (no more enabling/disabling of `finally` clauses for breaks) and also improves code size / performance in some cases ### Exception Runtime * `nimAbortException` has a `viaRaise` parameter now, as the C-specific error flag cannot be used anymore for detecting what causes the abort * `prepareException` is merged back into `raiseExceptionAux` The JavaScript target also using the C exception runtime now fixes a bug where breaking out of a `finally` didn't clean up the current exception properly. ### VM Exception stack management is partially decoupled from the core VM and moved to `vmops` (which hooks and implements the various exception runtime procedures). Generating the stacktrace still needs to be done directly by the VM, which prevents the exception stack management from being fully decoupled. --------- Co-authored-by: Saem Ghani <saemghani+github@gmail.com>
sem: replace `addr` calls with `nkAddr` (#1465) ## Summary In typed AST, address-of operations are now *always* represented by `nkAddr` trees. This simplifies some compiler logic, makes processing for typed macros easier, and fixes an effect tracking bug with `addr`. ## Details This is effectively a revert of nim-lang/Nim#10814. Not turning calls to `mAddr` into `nkAddr` was done to prevent the unsafe address semantics from being lost, but those no longer exist. Lowering the call into an `nkAddr` tree has the benefit that it simplifies AST analysis and processing, as address-of operation can now always be detected by `PNode.kind == nkAddr`. Old code for detecting `mAddr` magic calls is removed. The effect tracking in `sempass2` had no special case for `mAddr` calls, thus treating them as normal calls, which led to `addr(x)` being treated as an indirect invocation of `x`, when `x` is of procedural type. With the `mAddr` call now being lowered earlier, this is no longer the case.
fix(typeinfo): expanding seq doesn't clear locations (#1463) ## Summary New seq slots are now zeroed when expanding the seq via `invokeNewSeq` or `extendSeq`, making the behaviour consistent with `setLen` and `newSeq`, and also fixing crashes with `marshal` caused by the uninitialized memory. ## Details Zeroing the memory is not correct for types that don't have a zero- default, but those cannot be detected with just RTTI. Zeroing the memory is usually still better then leaving it as is. For the JavaScript and VM backends, the `zeroMem` call is excluded from compilation. Using `invokeNewSeq` and `extendSeq` is already not possible on these backends. Fixes #1462
fix(sem): noreturn analysis for `void` exprs (#1454) ## Summary `void` branches preceeding with no-return terminal branches are checked with no-return analysis to avoid false positives. ## Details The analysis is updated to recursively check `if/elif/of/except` non-terminal branches as well as the terminal branch. Fixes #1453
fix: incorrect code generation for parameter borrowing (#1460) ## Summary Fix parameters below the pass-by-reference size threshold not being passed by reference when the procedure returns a non-direct view, resulting in access violations at run-time when trying to access the returned view. ## Details * consider immutable non-direct views (e.g., `object` types with `lent` fields) when deciding whether pass-by-reference is used for the first parameter of a procedure * move the `classifyViewType` procedures from `typeallowed` to `types`, as they're wholly unrelated to the "type allowed" checks Fixes #1457.
testament: fix missing error reporting (#1451) ## Summary Fix tests with invalid specs being silently ignored (they were neither compiled nor run) when using `all` or `cat megatest`. ## Details The `isJoinableSpec` procedure didn't prevent tests with an invalid spec (`reInvalidSpec`) from being joined, which overrode the failure with `reJoined`. Since the specs then had no target enabled, they were also not included in the megatest. A few invalid test specifications went by unnoticed due to this bug -- they're now fixed.
fix(sem): avoid no-return false positives (#1452) ## Summary no-return analysis now no longer confuses `void` branches for true no-returns. ## Details No-return analysis now dives into the trees of `if/try/block/case` statements in order to determine if they are in fact no-return scenarios. This is only done for the statement variety. The updated analysis besides being more correct, in that it avoids false positive no-return identification, it still allows for a number of false negatives. Chiefly that unstructured exits ( `return/break/raise` ) are only considered in the trailing position of statement lists and not part way through. This is considered acceptable as there is an overall improvement in accuracy and it should cover most code. This includes a fix for an NPE crash in discard-or-use error reporting.
fix(sem): analysis considers nested noreturn (#1450) ## Summary No-return analysis ensures that nested final expressions ( `block/case/if/try` ) are accounted for no-return itself. ## Details Prior to this change if the last expression within a greater expression was a no-return expression it would not be deemed as such if it was a `block/case/if/try` , instead errors would be raised related to discard checks or that path not resulting in the correct type. For example, the following would result in an error where `0` must be used or discarded: ```nim proc foo() = let x = if false: 10 else: block: return ``` Fixes #1440 --------- Co-authored-by: zerbina <100542850+zerbina@users.noreply.github.com>
fix: crash when passing NimNode to static parameter (#1449) ## Summary * fix a compiler crash when passing a NimNode to a `static` parameter * fix a compiler crash when evaluating a `NimNode`-returning constant expression ## Details There were two problems: * `nkNimNodeLit` wasn't handled in constant data expression by `mirgen` * `NimNode` values returned directly from a VM invocation weren't wrapped in `nkNimNodeLit` trees For handling `NimNode` values in `vm.regToNode`, the existing deserialization logic for `NimNode` values in `vmcompilerserdes` is moved to a separate procedure, so that it can be used by `regToNode`. The `opcRepr` implementation relied on `regToNode` always returning an unwrapped `PNode` -- it is adjusted to manually handle `NimNode` values. A test covering both issues is added. Fixes #1448.
PreviousNext