allow generic compileTime proc folding #22022
Merged
Conversation
This file contains 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
metagn
commented
Jun 6, 2023
This comment was marked as outdated.
This comment was marked as outdated.
metagn
changed the title
metagn
force-pushed
the
generic-compiletime-proc
branch
from
41f851c
to
c1fae98
Compare
metagn
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
metagn
force-pushed
the
generic-compiletime-proc
branch
from
c1fae98
to
399c7fd
Compare
|
I've changed it so non-magic calls do not fold at compile time in |
Araq
reviewed
Aug 17, 2024
| @@ -49,7 +49,9 @@ proc semTypeOf(c: PContext; n: PNode): PNode = | |||
| else: | |||
| m = mode.intVal | |||
| result = newNodeI(nkTypeOfExpr, n.info) | |||
| inc c.inTypeofContext | |||
| let typExpr = semExprWithType(c, n[1], if m == 1: {efInTypeof} else: {}) | |||
There was a problem hiding this comment.
As you can see here we already have efInTypeof. Can this be used instead of c.inTypeofContext?
There was a problem hiding this comment.
That's what I tried first, it doesn't always propagate, even in simple cases like typeof(foo(compileTimeProc())) where it's nested in another call.
|
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
narimiran
pushed a commit
that referenced
this pull request
Sep 13, 2024
fixes #10753, fixes #22021, refs #19365 (was fixed by #22029, but more faithful test added) For whatever reason `compileTime` proc calls did not fold if the proc was generic ([since this folding was introduced](c25ffbf#diff-539da3a63df08fa987f1b0c67d26cdc690753843d110b6bf0805a685eeaffd40)). I'm guessing the intention was for *unresolved* generic procs to not fold, which is now the logic. Non-magic `compileTime` procs also now don't fold at compile time in `typeof` contexts to avoid possible runtime errors (only the important) and prevent double/needless evaluation. (cherry picked from commit f7c11a8)
This was referenced Sep 22, 2024
Araq
pushed a commit
that referenced
this pull request
Sep 22, 2024
…port:2.0] (#24152) fixes #24150, refs #22022 An exception is raised in the `semExprWithType` call, which means `dec c.inTypeofContext` is never called, but `compiles` allows compilation to continue. This means `c.inTypeofContext` is left perpetually nonzero, which prevents `compileTime` evaluation for the rest of the program. To fix this, `defer:` is used for the `dec c.inTypeofContext` call, as is done for [`instCounter`](https://github.com/nim-lang/Nim/blob/d51d88700b2fb3bd228d5e8f7385e2e4a2e2880c/compiler/seminst.nim#L374) in other parts of the compiler.
narimiran
pushed a commit
that referenced
this pull request
Sep 28, 2024
…port:2.0] (#24152) fixes #24150, refs #22022 An exception is raised in the `semExprWithType` call, which means `dec c.inTypeofContext` is never called, but `compiles` allows compilation to continue. This means `c.inTypeofContext` is left perpetually nonzero, which prevents `compileTime` evaluation for the rest of the program. To fix this, `defer:` is used for the `dec c.inTypeofContext` call, as is done for [`instCounter`](https://github.com/nim-lang/Nim/blob/d51d88700b2fb3bd228d5e8f7385e2e4a2e2880c/compiler/seminst.nim#L374) in other parts of the compiler. (cherry picked from commit a177720)
metagn
added a commit
to metagn/Nim
that referenced
this pull request
Oct 3, 2024
Araq
pushed a commit
that referenced
this pull request
Oct 6, 2024
fixes #24228, refs #22022 As described in #24228 (comment), instantiating generic routines inside `typeof` causes all code inside to be treated as being in a typeof context, and thus preventing compile time proc folding, causing issues when code is generated for the instantiated routine. Now, instantiated generic procs are treated as never being inside a `typeof` context. This is probably an arbitrary special case and more issues with the `typeof` behavior from #22022 are likely. Ideally this behavior would be removed but it's necessary to accomodate the current [proc `declval` in the package `stew`](status-im/nim-stew#190), at least without changes to `compileTime` that would either break other code (making it not eagerly fold by default) or still require a change in stew (adding an option to disable the eager folding). Alternatively we could also make the eager folding opt-in only for generic compileTime procs so that #22022 breaks nothing whatsoever, but a universal solution would be better. Edit: Done in #24230 via experimental switch
narimiran
pushed a commit
that referenced
this pull request
Oct 23, 2024
fixes #24228, refs #22022 As described in #24228 (comment), instantiating generic routines inside `typeof` causes all code inside to be treated as being in a typeof context, and thus preventing compile time proc folding, causing issues when code is generated for the instantiated routine. Now, instantiated generic procs are treated as never being inside a `typeof` context. This is probably an arbitrary special case and more issues with the `typeof` behavior from #22022 are likely. Ideally this behavior would be removed but it's necessary to accomodate the current [proc `declval` in the package `stew`](status-im/nim-stew#190), at least without changes to `compileTime` that would either break other code (making it not eagerly fold by default) or still require a change in stew (adding an option to disable the eager folding). Alternatively we could also make the eager folding opt-in only for generic compileTime procs so that #22022 breaks nothing whatsoever, but a universal solution would be better. Edit: Done in #24230 via experimental switch (cherry picked from commit ea9811a)
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.
fixes #10753, fixes #22021, refs #19365 (was fixed by #22029, but more faithful test added)
For whatever reason
compileTimeproc calls did not fold if the proc was generic (since this folding was introduced). I'm guessing the intention was for unresolved generic procs to not fold, which is now the logic.Non-magic
compileTimeprocs also now don't fold at compile time intypeofcontexts to avoid possible runtime errors (only the type is important) and prevent double/needless evaluation.