Skip to content

Conversation

@soulofmischief
Copy link

@soulofmischief soulofmischief commented Mar 11, 2025

This fixes a regression caused by the most recent release.

I'm unfamiliar with the project, I just upstreamed quickjs-ng/quickjs@77332f2 so this patch needs review.

Tested manually locally and it seems to work. I didn't upstream the test file because it seems to be missing from quickjs-ng's latest build. I was unsure if it should also be upstreamed or written from scratch to be more idiomatic with this project.

Should fix #363 and maybe others

This fixes a regression caused by the most recent release.
@bnoordhuis
Copy link
Contributor

interrupt-test.c is called api-test.c in quickjs-ng now but contains more tests that in all likelihood won't pass with bellard/master.

When cherry-picking commits, it's considered good style to retain the original Author. The way you did it now makes it look like you wrote that code.

@soulofmischief
Copy link
Author

That's a good point, I didn't think to credit the original author since it was a small patch and there were discrepancies which meant I had to modify the patches to work with the upstream, but I have no problem crediting laishere. Thanks for pointing that out.

@soulofmischief
Copy link
Author

Closing this PR anyway as Fabrice says the issue is resolved in the most recent version

zenHeart pushed a commit to zenHeart/quickjs that referenced this pull request Mar 25, 2025
- use single test in `js_strtod` loop.
- use more explicit `ATOD_xxx` flags
- remove `ATOD_TYPE_MASK`, use `ATOD_WANT_BIG_INT` instead
- remove unused arguments `flags` and `pexponent` in `js_string_to_bigint`
- merge `js_atof` and `js_atof2`, remove `slimb_t *pexponent` argument
- simplify and document `js_atof` parser, remove cumbersome labels,
- simplify `js_parseInt` test for zero radix for `ATOD_ACCEPT_HEX_PREFIX`
- simplify `next_token` number parsing, handle legacy octal in parser only
- simplify `JS_StringToBigInt`, use flags only.
- remove unused `slimb_t exponent` token field
- add number syntax tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants