fix: decode string with specified buffer size #19
Conversation
|
This won't work correctly with pthreads, because |
| abort('FATAL ERROR: ' + | ||
| (location_len === -1 ? UTF8ToString(location) : emnapiRt.utf8Decoder.decode(HEAPU8.subarray(location, location + location_len))) + | ||
| ' ' + | ||
| (message_len === -1 ? UTF8ToString(message) : emnapiRt.utf8Decoder.decode(HEAPU8.subarray(message, message + message_len))) |
There was a problem hiding this comment.
Maybe better to extract this into a helper emnapiRt.readString? Could be a bit cleaner than duplicating === -1 ? ... everywhere.
| export const utf8Decoder: { decode: (input: BufferSource) => string } = typeof TextDecoder === 'function' | ||
| ? new TextDecoder() | ||
| : { | ||
| decode (input: BufferSource) { |
There was a problem hiding this comment.
I'd suggest to either exclude the fallback, or at least guard it under #if TEXTDECODER == 2 etc like Emscripten does.
It's quite a bit of code for something that none of the modern engines needs.
| throw new TypeError('The "input" argument must be an instance of ArrayBuffer or ArrayBufferView') | ||
| } | ||
| const bytes = isArrayBuffer ? new Uint16Array(input) : new Uint16Array(input.buffer, input.byteOffset, input.byteLength / 2) | ||
| return String.fromCharCode.apply(String, bytes as any) |
There was a problem hiding this comment.
This is often tempting, but will fail with stack overflow on any large strings (probably worth adding a test for that). It's important to use a loop here instead.
|
Thanks. working in progress |
|
Done. c8f1175 |
|
Looks good. I think it would be still worth adding regression tests for 2 mentioned cases - one to check that strings work when compiled with |
Fixes #17