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.

Sign up for GitHub

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

Jump to bottom

Accept underscore as numeric literal separator #6989

Merged
merged 1 commit into from
Dec 14, 2018
Merged

Accept underscore as numeric literal separator #6989

merged 1 commit into from
Dec 14, 2018

Conversation

som-snytt
Copy link
Contributor

000123, 000_123, 000'123, 3_14e-0_2.

@scala-jenkins scala-jenkins added this to the 2.13.0-M5 milestone Jul 29, 2018
@som-snytt som-snytt force-pushed the issue/6124-numeric-underscore branch from b8556e2 to dd1231d Compare July 29, 2018 20:43
@som-snytt som-snytt mentioned this pull request Jul 30, 2018
Closed
@som-snytt som-snytt force-pushed the issue/6124-numeric-underscore branch 2 times, most recently from 0295fd9 to 6c0cef5 Compare July 30, 2018 04:52
@szeiger szeiger added the WIP label Aug 2, 2018
@szeiger
Copy link
Member

szeiger commented Aug 2, 2018

Marking as WIP until the status of this change (SIP? Micro-SIP?) is clear.

@SethTisue SethTisue modified the milestones: 2.13.0-M5, 2.13.0-RC1 Aug 7, 2018
@adriaanm adriaanm modified the milestones: 2.13.0-RC1, 2.13.x Aug 7, 2018
@som-snytt
Copy link
Contributor Author

@adriaanm @SethTisue Probably a syntax change can't land in 2.13.1? "It's just syntax."

@som-snytt som-snytt force-pushed the issue/6124-numeric-underscore branch from 6c0cef5 to e69c40a Compare August 7, 2018 13:04
@adriaanm adriaanm modified the milestones: 2.13.1, 2.13.0-RC1 Aug 7, 2018
@sjrd
Copy link
Member

sjrd commented Aug 9, 2018

There is a precedent: trailing commas landed in 2.12.2.

@SethTisue
Copy link
Member

right, we don't guarantee forward source compat, only forward binary compat

@som-snytt
Copy link
Contributor Author

Wow, I thought "It's just syntax" was just a saying. I'm going to start using trailing commas everywhere now. Or everywhere except 0.13 builds, I guess.

@som-snytt som-snytt changed the title Accept numeric literal separators and leading zero Pre-SIP: Accept numeric literal separators and leading zero Aug 30, 2018
@som-snytt som-snytt removed the WIP label Aug 30, 2018
@lrytz
Copy link
Member

lrytz commented Nov 2, 2018

What's next here? I'd personally prefer supporting only one of the two separators.

@NthPortal
Copy link
Contributor

+1 to Lukas' comment. I'm in favour of _ personally, although I can't think of a good justification for that

@som-snytt som-snytt mentioned this pull request Nov 4, 2018
Closed
@som-snytt som-snytt force-pushed the issue/6124-numeric-underscore branch from e69c40a to d4d5735 Compare November 4, 2018 22:06
@som-snytt
Copy link
Contributor Author

som-snytt commented Nov 4, 2018 •
edited

There was a long discussion elsewhere in which Ichoran expressed a personal preference for 1234'5678. I have no preference except to avoid the foisting of preferences.

Here is the commit rebased and slightly cleaner. The warnings were pushed to the linked PR. It should be easy for someone else to push a tweak once the committee makes up its mind.

@som-snytt som-snytt changed the title Pre-SIP: Accept numeric literal separators and leading zero Pre-SIP: Accept numeric literal separators Nov 4, 2018
@adriaanm
Copy link
Contributor

Our approach here is usually to copy Java, which means just having underscore: https://docs.oracle.com/javase/8/docs/technotes/guides/language/underscores-literals.html

@soronpo
Copy link

soronpo commented Nov 14, 2018

Does Java accept a leading zero?

@som-snytt
Copy link
Contributor Author

@soronpo Java supports octal syntax.

@som-snytt som-snytt changed the title Pre-SIP: Accept numeric literal separators Semi-SIP: Accept underscore as numeric literal separator Nov 14, 2018
@soronpo
Copy link

soronpo commented Dec 4, 2018

desugars to ...

Why is this good?

@SethTisue
Copy link
Member

this is ready for merge, right? I'm certainly in favor of the change.

SethTisue
SethTisue requested changes Dec 4, 2018
View reviewed changes
Copy link
Member

@SethTisue SethTisue left a comment •
edited

Choose a reason for hiding this comment

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

this looks ready to go (post-squash), except that it should include a spec update

@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Dec 4, 2018
@SethTisue
Copy link
Member

as always, our compass is WWGD?

@SethTisue
Copy link
Member

@jvican @darjutak did this go before the SIP committee?

@som-snytt som-snytt force-pushed the issue/6124-numeric-underscore branch from 58aa62e to f8d704d Compare December 5, 2018 01:58
@adriaanm
Copy link
Contributor

adriaanm commented Dec 5, 2018

/cc @olafurpg fyi, small syntax change. We should also think about what this means for round-tripping (normalisation during lexing means we can't print the original source based on the AST.). I'm not saying this has to block the PR, but it's worth documenting.

olafurpg
olafurpg reviewed Dec 5, 2018
View reviewed changes
Copy link
Member

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

This looks great! In Scalameta we already store the original syntax in tokens so the underscores won't be lost during a pretty-printing roundtrip. I recommend to add some position tests (esp. with -Yrangepos) to ensure compiler trees are able recover the original syntax.

test/files/pos/t6124.scala Show resolved Hide resolved
@som-snytt som-snytt changed the title Semi-SIP: Accept underscore as numeric literal separator Accept underscore as numeric literal separator Dec 10, 2018
@som-snytt
Accept numeric literal separators
c359188 
The separator character is underscore
as in Java.  They may be mixed
arbitrarily in a numeric literal, but
only in the interior: the literal, and
each part of double, must begin and
end with a digit.

Add spec for underscore numeric separator.

Test literal positions.

Verifying visually that literals with
underscores are the correct length.
@som-snytt
Copy link
Contributor Author

@SethTisue One, two, three, four, what's the syntax we adore? Numeric underscore!

Fully baked squash pie. This is the rare PR with no thumbs pointed netherward. Even when the mob was all pitchforky about single quote as separator, they dared not jinx it, such was their faith and enthusiasm.

@adriaanm adriaanm merged commit 5753c5e into scala:2.13.x Dec 14, 2018
@adriaanm
Copy link
Contributor

Hmmmm 🥧

@NthPortal
Copy link
Contributor

🎉

@dwijnand
Copy link
Member

Nice!

Particularly well done for not restricting it to groupings of 3, which wouldn't be very nice towards some numbering systems like https://en.wikipedia.org/wiki/Indian_numbering_system

@som-snytt som-snytt deleted the issue/6124-numeric-underscore branch December 14, 2018 13:15
@odersky odersky mentioned this pull request Apr 17, 2019
Closed
@Jentsch Jentsch mentioned this pull request Apr 29, 2019
Merged
@exoego exoego mentioned this pull request Jan 4, 2023
Open
@shailrshah shailrshah mentioned this pull request Apr 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olafurpg olafurpg olafurpg left review comments

@SethTisue SethTisue SethTisue requested changes

Assignees
No one assigned
Labels
release-notes worth highlighting in next release notes
Projects
None yet
Milestone
2.13.0-RC1