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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No error showed when error threw with --module flag

2 participants