feat(stdlib): Convert runtime printing utils to @unsafe #1135
Conversation
| let decimalCount32Dummy = (n: WasmI32) => 0n | ||
| let utoa32BufferedDummy = (a: WasmI32, b: WasmI32, c: WasmI32) => void | ||
|
|
||
| // When these boxes are backpatched, the reference count of each function will |
There was a problem hiding this comment.
Should we maybe emit a warning or something when using boxes in --compilation-mode=runtime? It seems like this could bite us
(not for this PR)
There was a problem hiding this comment.
Maybe? The plan is for only malloc.gr and gc.gr to use runtime mode, so it might not be necessary. But it's not really just a problem for boxes; it's for anything that gets allocated in runtime mode that gets mutated by things outside of the runtime.
I actually thought about a couple of solutions for this:
- The fake reference count of runtime objects is
1; could just set it to0xefffffff. While this would work, I'd be worried about some large program that could decRef enough that the problem would occur, and it'd be a hell of a time debugging that. - Could alter
freeto never free anything in runtime land. Actually fairly easy to implement with lowish perf overhead, though I dislike that the reference counts for runtime values would be essentially random. That's more of an aesthetic problem though since the count doesn't matter at all. - Disallow mutable boxes/arrays/record fields in runtime mode. That'd completely prevent this (which is handy) though of course we couldn't use those constructs (which isn't a big deal since we don't need them, but it'd prevent doing backrefs for debugging like this, etc.).
But also maybe this comment is fine 🤷
Up to you if you want to make an issue for it.
There was a problem hiding this comment.
You know me, I'm always happy with issues where we can have a longer discussion!
ospencer
force-pushed
the
oscar/no-closure-globals
branch
from
977a0aa
to
536ba56
Compare
ospencer
force-pushed
the
oscar/unsafe-string
branch
2 times, most recently
from
0d68959
to
ee6fce8
Compare
stdlib/runtime/numberUtils.gr
Outdated
| @@ -936,101 +968,108 @@ export let utoa32Buffered = (buf, value, radix) => { | |||
| } | |||
| } | |||
|
|
|||
| @unsafe | |||
| export let utoa32 = (value, radix) => { | |||
| if (WasmI32.ltS(radix, 2n) || WasmI32.gtS(radix, 36n)) { | |||
| throw Exception.InvalidArgument( | |||
| "toString() radix argument must be between 2 and 36" | |||
| ) | |||
| } | |||
| let str = if (WasmI32.eqz(value)) { | |||
There was a problem hiding this comment.
Can you just remove let str here and at the end where you return it?
stdlib/runtime/numberUtils.gr
Outdated
| } | ||
|
|
||
| @unsafe | ||
| export let utoa64 = (value, radix) => { | ||
| if (WasmI32.ltS(radix, 2n) || WasmI32.gtS(radix, 36n)) { | ||
| throw Exception.InvalidArgument( | ||
| "toString() radix argument must be between 2 and 36" | ||
| ) | ||
| } | ||
| let str = if (WasmI64.eqz(value)) { |
There was a problem hiding this comment.
Can you remove this str variable?
| let decimalCount32Dummy = (n: WasmI32) => 0n | ||
| let utoa32BufferedDummy = (a: WasmI32, b: WasmI32, c: WasmI32) => void | ||
|
|
||
| // When these boxes are backpatched, the reference count of each function will |
There was a problem hiding this comment.
You know me, I'm always happy with issues where we can have a longer discussion!
| [], | ||
| [...](String, StringList), |
There was a problem hiding this comment.
Did we have an issue about this somewhere that should be closed by this PR?
There was a problem hiding this comment.
Probably #552 but that's not quite ready to be closed.
ospencer
force-pushed
the
oscar/no-closure-globals
branch
from
536ba56
to
d8cbb65
Compare
ospencer
force-pushed
the
oscar/unsafe-string
branch
from
ee6fce8
to
b6658e8
Compare
ospencer
force-pushed
the
oscar/unsafe-string
branch
from
b6658e8
to
33952ba
Compare
The major thing to notice in this PR is that
runtime/numberUtils.gris no longer--compilation-mode=runtime. This means that it uses regular allocated Grain memory, which means that string literals, etc. are allowed to be used. Ideally,malloc.gr/gc.gr/memory.grwill be the only files in runtime mode.