Fixes #1168 #1169
Merged
Fixes #1168 #1169
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Works according to the build bots. |
Varriount
added a commit
that referenced
this pull request
May 21, 2014
Clyybber
pushed a commit
to Clyybber/Nim
that referenced
this pull request
Feb 29, 2024
## Summary
Regarding overflow-check behaviour, `abs` is now treated the same as
any other procedure, meaning that whether or not overflow checks are
enabled at the `abs` callsite is ignored. This is a change in
behaviour for the C and JavaScript backends, but not for the VM
backend.
In addition, if an `abs` call is the only raising statement in a
scope, it raising an exception now properly invokes the destructor
for the relevant locals in the scope.
## Details
* `cgen` and `jsgen` treat `mAbsI` as a normal procedure (`vmgen`
already did)
* the `abs` definitions in `system` are moved above the
`{.push checks: off.}` pragma affecting them. This prevents them
from having overflow checks unconditionally disabled
* `mirgen` uses a dedicated translation path for `mAbsI`, in order to
pick a checked or unchecked call, based on whether overflow
checks and panics are enabled
Calls to `abs` now using checked calls (if needed) makes the
exceptional control-flow visible at the MIR level and subsequently to
the data-flow analysis / destructor injection.
### Tests
* add a test case for ensuring proper destruction when only `abs`
can raise
* enable `toverflow_abs` for the C target. The JS target still
doesn't work, due to the improper 64-bit integer overflow check
implementation
* change the expected output of `toverflow_abs` such that it also
matches that of the C test
* `abs` for integers is not a generic procedure, the unnecessary
"instantiation" in `toverflow_abs` is removed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.