Zero is not a power of two. Fix #1047 #1071
Merged
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
Also, fixed some docstrings and added {.noSideEffect.} pragma to nextPowerOfTwo().
Araq
added a commit
that referenced
this pull request
Apr 6, 2014
Zero is not a power of two. Fixes #1047
Araq
added a commit
that referenced
this pull request
Apr 7, 2014
Zero is not a power of two. Fixes #1047
Clyybber
pushed a commit
to Clyybber/Nim
that referenced
this pull request
Feb 24, 2024
## Summary Consider `--panics:on` during AST -> MIR translation and move all decision making regarding whether a call potentially raises an exception out of the code generators. ## Details Calls within the MIR already encode whether or not they have an exceptional exit (i.e., potentially raise an exception). However, `--panics:on` was not considered during the AST -> MIR translation, meaning that all non-magic were always considered as potentially raising an exception, even if pancis were enabled and the procedure raised no catchable errors. This leaves considering `--panics:on` to the code generators, which is a problem, as all behaviour-related decision-making needs to happen prior code generation. Therefore, `mirgen` now consider whether panics are enabled. This also means that the `Defect` versus `CatchableError` distinction no longer exists after the MIR phase. ### Architectural changes * the `canRaiseDisp` procedure is renamed to `canRaise` and moved to the `ast_query` module (where the other `canRaise` procedure are) * in addition, `canRaiseDisp` is adjusted to account for imported JavaScript procedures (identified by the `sfInfixCall` flag; compared to imported C functions, it's possible for them to raise exceptions) * the pre-existing `canRaise` overload is un-exported * `mirgen` now uses the new panics-considering `canRaise` overload for deciding whether a call can raise ### Dedicated node kind The `mnkCheckedCall` node kind is introduced, replacing the `geRaises` flag. This is progress towards: * removing the `effects` field from `MirNode` (so that call nodes can store the number of sub-nodes instead) * bringing the MIR and `CgNode` IR closer together The `mnkCheckedCall` node kind is then translated to the `cnkCheckedCall` node kind. For knowing whether the a call has an exceptional exit, only the node kind needs to be inspected. Finally, the all places that handle the `mnkCall`/`cnkCall` node kind now also handle the `mnkCheckedCall`/`cnkCheckedCall` node kind. When pretty-printing, a `mnkCheckedCall` is rendered like a `mnkCall`, but with a trailing `(raises)` annotation. ### Lifetime hooks In order to ensure that lifetime-hooks with exceptional exits work the same as they did previously, `injectdestructors` emits potentially raising calls according to the `canRaise` query. This needs to be addressed eventually , either by inserting the calls prior to AST -> MIR translation (unlikely) or by disallowing the hooks from raising exceptions. ### Warnings `cgen` now only consider whether the call is checked when reporting an `ObservableStores` warning. This means that without `--panics:on`, the warning is reported a lot more often now. Since the direction is to remove the warning by not performing observable stores in the first place, this regression is deemed okay.
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.
Fix #1047. Also, some fixes on docstrings and a typo fixed in a previous commit.