Add BigInt64Array shim for Safari 14#17103
Conversation
c59631b to
9e78b25
Compare
kripken
left a comment
There was a problem hiding this comment.
Is this fast enough to be useful?
We usually put polyfills under src/polyfill/, see examples there. It should also be gated behind a check to avoid the constant code size overhead. Perhaps it could check MIN_SAFARI_VERSION perhaps.
I dunno, probably? Do you have a specific benchmark in mind? The main goal is to be able to use |
sbc100
left a comment
There was a problem hiding this comment.
This looks good to me as it stands.
We have not used globalThis in any of our other polyfills that I know of... but if it works in this case I guess that is fine.
|
Oh I can just use function f(){
if(true) {
var x = 2;
}
console.log(x); // 2
}
console.log(x); // ReferenceError |
|
This raises: parseTools.js preprocessor error in preamble.js:289: "#if !POLYFILL"!
Internal compiler error in src/compiler.js!
Please create a bug report at https://github.com/emscripten-core/emscripten/issues/
with a log of the build and the input files used to run. Exception message: "ReferenceError: POLYFILL is not defined" | ReferenceError: POLYFILL is not defined |
|
Oh that may only be a problem in the backport. |
This reverts commit 7b96688.
|
Okay I added a fairly thorough test suite and caught a few bugs. There are still a few cases where the real thing throws an error but my shim just does something, but I don't think it matters. |
|
Can this be merged? |
|
Bump |
sbc100
left a comment
There was a problem hiding this comment.
For the other polyfils we have tended to test them in place ? By running emscripten -sXXX_VERSION=YY to enable them. But this direct testing you have implemented seems reasonable too.
|
Bump |
|
@sbc100 , any remaining comments from you? I think everything was resolved but didn't want to merge before asking. |
|
lgtm |
As discussed in #16693,
-s WASM_BIGINTrequires bothBigInt(present since Safari 14.0 in 90.91% of users' browsers) andBigInt64Array(present since Safari 15.0 in 87.67% of users' browsers). We want to target Safari 14.0 as our minimal supported version, so we need this shim to work.I didn't shim all of the TypedArray APIs, just the ones that I think are most important. It would be pretty easy to shim the rest of them if you want.