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

Drop support for "use math" #21

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Drop support for "use math" #21

merged 2 commits into from
Nov 6, 2023

Conversation

saghul
Copy link
Contributor

@saghul saghul commented Nov 6, 2023

No description provided.

@saghul
Copy link
Contributor Author

saghul commented Nov 6, 2023

Oh, I broke something and 262 found it! I'll take a look later.

@saghul
Copy link
Contributor Author

saghul commented Nov 6, 2023

This one is ready @bnoordhuis !

Copy link
Contributor

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

👍

I did have a ready-to-submit commit that removes all the #ifdef CONFIG_BIGNUM that's probably going to conflict heavily with this. Oh well.

/* assume bigint mode: floats are parsed as integers if no decimal
point nor exponent */
#define ATOD_MODE_BIGINT (1 << 9)
#define ATOD_MODE_UNUSED (1 << 9)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#define ATOD_MODE_UNUSED (1 << 9)

Or did you have a reason for keeping it around?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nah.

quickjs.c Outdated
Comment on lines 10318 to 10319
} else if ((atod_type == ATOD_TYPE_FLOAT64) && is_float && radix != 10) {
goto fail;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
} else if ((atod_type == ATOD_TYPE_FLOAT64) && is_float && radix != 10) {
goto fail;
} else if (atod_type == ATOD_TYPE_FLOAT64 && is_float && radix != 10) {
goto fail;

goto fail;
case JS_TAG_FLOAT64:
case JS_TAG_BIG_FLOAT:
goto fail;
/* fall tru */
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/* fall tru */

quickjs.c Outdated
Comment on lines 21085 to 20786
if (s->cur_func->js_mode & JS_MODE_MATH)
s->token.val = TOK_MATH_POW_ASSIGN;
else
s->token.val = TOK_XOR_ASSIGN;
s->token.val = TOK_XOR_ASSIGN;
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you can replace this block (the code inside the #ifdef CONFIG_BIGNUM) with just the #else case.

@saghul
Copy link
Contributor Author

saghul commented Nov 6, 2023

👍

I did have a ready-to-submit commit that removes all the #ifdef CONFIG_BIGNUM that's probably going to conflict heavily with this. Oh well.

Sorry I got the impression you hadn't started :-/

@bnoordhuis
Copy link
Contributor

Don't worry about it, it happens.

@saghul saghul merged commit 1fb9a50 into master Nov 6, 2023
7 checks passed
@saghul saghul deleted the no-use-math branch November 6, 2023 22:07
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.

2 participants