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

fix NaN is lexed as identifier, not as a number #397

Merged
merged 1 commit into from
May 13, 2020

Conversation

attila-lin
Copy link
Contributor

This Pull Request fixes/closes #393.

It changes the following:

  • make NaN a number
  • add test for it

@Razican Razican added this to the v0.8.0 milestone May 13, 2020
@Razican Razican added the bug Something isn't working label May 13, 2020
Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

This is perfect, thanks for the contribution!

@jasonwilliams
Copy link
Member

yep, looks good

@jasonwilliams jasonwilliams merged commit d4d2729 into boa-dev:master May 13, 2020
Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for contributing!

There is an edge case we need to support in the future which is

var NaN = 4;

which requires changes to the parser too.
This also applies to undefined.

@Razican Razican mentioned this pull request May 18, 2020
@simonbuchan
Copy link

This is fine as a hack, but NaN is actually specified as a global variable, like undefined, not a keyword, like null. (JavaScript is terrible.)
https://www.ecma-international.org/ecma-262/10.0/index.html#sec-global-object

@Razican
Copy link
Member

Razican commented May 26, 2020

This is fine as a hack, but NaN is actually specified as a global variable, like undefined, not a keyword, like null. (JavaScript is terrible.)
https://www.ecma-international.org/ecma-262/10.0/index.html#sec-global-object

Yep, we will have to improve this in the future, but first, we need to improve the lexer, the current one is a bit difficult to follow, it became too big.

I opened #421 regarding this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NaN is lexed as identifier, not as a number
5 participants