-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
don't mark declval
as {.compileTime.}
#190
Conversation
`declval` is meant to be used to determine types of expressions, and never be executed (#33). However it's marked `{.compileTime.}`, which would normally make it always execute during constant folding, but due to a bug in Nim (nim-lang/Nim#10753), generic procs like `declval` do not undergo constant folding. So the `{.compileTime.}` annotation is removed for this to work when the bug in Nim is fixed.
Would be nice if this got reviewed, this is needed for CI to pass with the bugfix in nim-lang/Nim#22022, and this library is probably too active/important to fork for CI. |
this "feels" wrong - ie declval is clearly a compile-time thing so having it marked otherwise seems counter-intuitive - is there a fundamental language reason it should not be marked as such or is it just that fixing one bug exposes another? |
The proc foo(x: int) {.compileTime.} = echo x
foo(123) is supposed to behave the same as: proc foo(x: int) = echo x
static: foo(123) There is a bug in Nim where specifically generic routines are exempt from this and don't get folded at compile time, I couldn't come up with a new mechanism to fit |
what prevents Separately, it's a bit of an open question how useful this "force compile-time evaluation" mechanism is - constant expression folding sounds like something the compiler could be doing "most" of the time irrespective of the pragma, except when side effects like rounding and overflow detection are involved. |
In this case the lack of the
The use case that brought me onto it was similar to how one would want to handle BigInt constant constructors, so maybe one could do something like: proc bigint(s: string): BigInt = ...
proc bigint(s: static string): BigInt {.compileTime.} = ... but the
The compiler switch |
the latter does not need a notably, in the strformat case, the primary motivator was actually to avoid unwanted raises effects that propagate to the whole program, even when the format string (and therefore the potential for raising) is known at compile-time and the way of "enforcing" compile-time behavior is to request it at the call site (using |
btw, this would not work in general since bigint requires runtime memory allocation, since the size of the bigint is arbitrary / unbounded. |
Ended up not being needed, closing
I was talking about the parsing |
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
declval
is meant to be used to determine types of expressions, and never be executed (#33). However it's marked{.compileTime.}
, which would normally make it always execute during constant folding, but due to a bug in Nim (nim-lang/Nim#10753), generic procs likedeclval
do not undergo constant folding. So the{.compileTime.}
annotation is removed for this to work when the bug in Nim is fixed.(Unfortunately this still only errors at execution time, I don't know a mechanism that errors at compile time but only outside typeof/concepts)