Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove CONFIG_BIGNUM, always enable BigInt #34

Merged
merged 3 commits into from Nov 10, 2023

Conversation

bnoordhuis
Copy link
Contributor

Fixes: #17

quickjs.c Show resolved Hide resolved
@@ -23770,9 +23266,7 @@ static __exception int js_parse_assign_expr2(JSParseState *s, int parse_flags)
static const uint8_t assign_opcodes[] = {
OP_mul, OP_div, OP_mod, OP_add, OP_sub,
OP_shl, OP_sar, OP_shr, OP_and, OP_xor, OP_or,
#ifdef CONFIG_BIGNUM
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok for op_pow to be repeated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looked very deliberate so I left it in but see the new commit, I believe this array can be removed altogether with some careful reshuffling. It's rather subtle though so I would appreciate it if you looked it over carefully.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I can make sense of that part... so I trust you, and the tests :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to make sense of it 😅 ... does the enum which contain TOK_MUL_ASSIGN need to be in sync with the opcodes? Then again, if it was wrong some test would have failed right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the enum which contain TOK_MUL_ASSIGN need to be in sync with the opcodes?

Yes. That array was used to map from the TOK domain to the OP domain. It becomes superfluous when both have the same ordering because then you can simply compute the right value.

@bnoordhuis bnoordhuis merged commit 38f88c0 into quickjs-ng:master Nov 10, 2023
13 checks passed
@bnoordhuis bnoordhuis deleted the rm-config-bignum branch November 10, 2023 15:09
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.

Remove CONFIG_BIGNUM, always enable BigInt
2 participants