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

Underscore not permitted at start or end of numeric literal segment #132

Merged
merged 5 commits into from Oct 10, 2020

Conversation

damieng
Copy link
Contributor

@damieng damieng commented Oct 10, 2020

When _ numeric literal support was added it was permitted everywhere in the numeric/hex/boolean literal.

It should not be allowed at the start of end of a segment.

We were also missing a test case for real literals starting with a .

Discovered in #131

@damieng damieng changed the title Underscore start or end of numeric literals not permitted Underscore not permitted at start or end of numeric literal segment Oct 10, 2020
@damieng damieng added the bug label Oct 10, 2020
@initram
Copy link
Contributor

initram commented Oct 10, 2020

Actually this is allowed 0x_12 and 0b_1010

But otherwise it looks good.

@damieng
Copy link
Contributor Author

damieng commented Oct 10, 2020

Ah, the joys of them not bothering to actually update a proper spec but just throwing out English language interpretations.

Will fix and push again.

@initram
Copy link
Contributor

initram commented Oct 10, 2020

I think you now allow binary and hex to end in underscore, which I don't think is allowed. I think you can just swap the star and plus in those regexes. Otherwise test them using regex101.com

Copy link
Contributor

@initram initram 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 to me

@damieng damieng merged commit 658b326 into master Oct 10, 2020
@damieng damieng deleted the do-not-allow-underscore-prefix-numbers branch October 10, 2020 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants