Use AsciiToString in Embind. NFC #24355
Conversation
d03dc61 to
a2a1612
Compare
|
The only concern I have (eternal with the way libraries are built in Emscripten) is whether Do we just need a changelog line to advise users if any to switch to the other function? Or do we need to preserve this name as an alias ~forever? |
|
I'm fairly confident that we can consider this function internal only. @brendandahl WDYT? Might be worth adding a changelog entry yes. If we are concerned then its also trivial to add an alias using |
Embind had its own read readLatin1String, which is functionally identical to the AsciiToString helper we have in the shared libstrings library. The only difference is that It had extra logic for caching characters by code, which are likely not even an optimisation in modern JIT engines, and if we do want to do that in the future for some reason, it's best to do so in centralised helpers. For now, a simple search-replace gives an easy win in terms of code size.
a2a1612 to
250f62c
Compare
|
I'd consider it internal and am fine with it going away. |
|
OK, lets land as is. Enabling audo-merge. |
|
@sbc100 I already restarted firefox CI, but it failed again. I see we don't configure |
Browser tests have (annoyingly) a different retry mechanism which applies to all tests. They are all retried on failure. |
Embind had its own read readLatin1String, which is functionally identical to the AsciiToString helper we have in the shared libstrings library.
The only difference is that It had extra logic for caching characters by code, which are likely not even an optimisation in modern JIT engines, and if we do want to do that in the future for some reason, it's best to do so in centralised helpers.
For now, a simple search-replace gives an easy win in terms of code size.