report roc_panic to the user in the web repl #5921
Conversation
www/public/repl/repl.js
Outdated
| const strPtr = rocStrBytes[0] | (rocStrBytes[1] << 8) | (rocStrBytes[2] << 16) | (rocStrBytes[3] << 24); | ||
| const strLen = rocStrBytes[4] | (rocStrBytes[5] << 8) | (rocStrBytes[6] << 16) | (rocStrBytes[7] << 24); |
There was a problem hiding this comment.
based on some searches there is no better way to achieve this?!
There was a problem hiding this comment.
You can do something like this (please test for bugs!)
const rocStrWords = new Uint32Array(memory.buffer, rocstr_ptr, 3);
const [ptr, len, cap] = rocStrWords;There was a problem hiding this comment.
The main idea being to use Uint32Array rather than Uint8Array
These classes are "views" onto an ArrayBuffer, but interpret the buffer as either u32 or u8 respectively.
The ArrayBuffer itself can't really do much.
The second argument to the constructor is called byteOffset in the docs. Third argument is length, which I think is the length of the final Uint32Array but test it!
There was a problem hiding this comment.
Also memoryBuffer appears to be unused?
You might have un-pushed changes because I see you marked one of my comments as resolved but I don't see the changes.
There was a problem hiding this comment.
huh, I tried the "transmute" from bytes before and then it did not work. maybe I used the wrong length. Anyway that does in fact work now
folkertdev
force-pushed
the
wasm-repl-crash
branch
from
a338bd0
to
793ab8e
Compare
www/public/repl/repl.js
Outdated
| const strPtr = rocStrBytes[0] | (rocStrBytes[1] << 8) | (rocStrBytes[2] << 16) | (rocStrBytes[3] << 24); | ||
| const strLen = rocStrBytes[4] | (rocStrBytes[5] << 8) | (rocStrBytes[6] << 16) | (rocStrBytes[7] << 24); |
There was a problem hiding this comment.
You can do something like this (please test for bugs!)
const rocStrWords = new Uint32Array(memory.buffer, rocstr_ptr, 3);
const [ptr, len, cap] = rocStrWords;
www/public/repl/repl.js
Outdated
| const strPtr = rocStrBytes[0] | (rocStrBytes[1] << 8) | (rocStrBytes[2] << 16) | (rocStrBytes[3] << 24); | ||
| const strLen = rocStrBytes[4] | (rocStrBytes[5] << 8) | (rocStrBytes[6] << 16) | (rocStrBytes[7] << 24); |
There was a problem hiding this comment.
The main idea being to use Uint32Array rather than Uint8Array
These classes are "views" onto an ArrayBuffer, but interpret the buffer as either u32 or u8 respectively.
The ArrayBuffer itself can't really do much.
The second argument to the constructor is called byteOffset in the docs. Third argument is length, which I think is the length of the final Uint32Array but test it!
www/public/repl/repl.js
Outdated
| const strPtr = rocStrBytes[0] | (rocStrBytes[1] << 8) | (rocStrBytes[2] << 16) | (rocStrBytes[3] << 24); | ||
| const strLen = rocStrBytes[4] | (rocStrBytes[5] << 8) | (rocStrBytes[6] << 16) | (rocStrBytes[7] << 24); |
There was a problem hiding this comment.
Also memoryBuffer appears to be unused?
You might have un-pushed changes because I see you marked one of my comments as resolved but I don't see the changes.
www/public/repl/repl.js
Outdated
| let stringBytes = ""; | ||
| if (finalByte < 0) { | ||
| // small string | ||
| const length = finalByte ^ 0b1000_0000; | ||
| stringBytes = new Uint8Array(memory.buffer, rocstr_ptr, length); | ||
| } else { | ||
| // big string | ||
| const rocStrWords = new Uint32Array(memory.buffer, rocstr_ptr, 3); | ||
| const [ptr, len, _cap] = rocStrWords; | ||
|
|
||
| const SEAMLESS_SLICE_BIT = 1 << 31; | ||
| const length = len & (~SEAMLESS_SLICE_BIT); | ||
|
|
||
| stringBytes = new Uint8Array(memory.buffer, ptr, length); | ||
| } |
There was a problem hiding this comment.
in practice I think only big (non-slice) strings can end up here right now. But it's good to just get this logic right, there is a good chance it'll get copied over time.
folkertdev
force-pushed
the
wasm-repl-crash
branch
from
80c29eb
to
349a469
Compare
folkertdev
force-pushed
the
wasm-repl-crash
branch
from
349a469
to
eb61d35
Compare
www/public/repl/repl.js
Outdated
| let stringBytes = ""; | ||
| if (finalByte < 0) { | ||
| // small string | ||
| const length = finalByte ^ 0b1000_0000; |
There was a problem hiding this comment.
There's a bug here!
I did the following test in my browser console.
Let's test this logic for a Roc string of length 5, where the last byte is 0x85.
Instead of building the whole string, let's put that number into an Int8Array and take it out again.
a = new Int8Array([0x85])
finalByte = a[0] // -123
length = finalByte ^ 0b1000_0000 // -251
correctLength = finalByte + 128 // 5Once you take it out of the Int8Array as a single number it is no longer 8 bits. It's converted to a JavaScript number. Bitwise ops in JS are effectively signed 32 bit. Bitwise ops on negative JS numbers are annoying, but addition just does what you'd expect. So finalByte + 128 will work correctly.
There was a problem hiding this comment.
love JS numbers... fixed now
| const [ptr, len, _cap] = rocStrWords; | ||
|
|
||
| const SEAMLESS_SLICE_BIT = 1 << 31; | ||
| const length = len & (~SEAMLESS_SLICE_BIT); |
There was a problem hiding this comment.
This bitwise op should work fine. It can't be a negative number because we read it out of an unsigned array. 👍
www/public/repl/repl.js
Outdated
| let stringBytes = ""; | ||
| if (finalByte < 0) { | ||
| // small string | ||
| const length = finalByte ^ 0b1000_0000; | ||
| stringBytes = new Uint8Array(memory.buffer, rocstr_ptr, length); | ||
| } else { | ||
| // big string | ||
| const rocStrWords = new Uint32Array(memory.buffer, rocstr_ptr, 3); | ||
| const [ptr, len, _cap] = rocStrWords; | ||
|
|
||
| const SEAMLESS_SLICE_BIT = 1 << 31; | ||
| const length = len & (~SEAMLESS_SLICE_BIT); | ||
|
|
||
| stringBytes = new Uint8Array(memory.buffer, ptr, length); | ||
| } |
I don't know JS very well so this works but might be unidiomatic?
it now looks lilke this