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

Implement SIP-42 - Support for binary integer literals #19405

Merged
merged 1 commit into from
Jan 20, 2024

Conversation

Sporarum
Copy link
Contributor

@Sporarum Sporarum commented Jan 9, 2024

Implement, test and document SIP-42

(Implement is a strong word, I uncommented a single line)

@odersky
Copy link
Contributor

odersky commented Jan 9, 2024

Is it in the Scala 2 spec? If not we need a doc page for the change. As often docs are harder than code.

@som-snytt
Copy link
Contributor

I had forgotten that I forward-ported the commented code with other fixes.

The old NthPortal PR was https://github.com/scala/scala/pull/8275/files which had only the syntax summary tweak. It has some more tests.

Nth's comment:

what I actually should have tested is 0b1 // Kenobi

That effort was in the six months leading up to the pandemic. My opinion is that it was unfortunate to put Scala 3 uptake in the path of Scala 2. That is, 0b syntax could have benignly coexisted. It would have blocked no one's migration and confused no beginners. And we would have had four years to come up with a literal syntax pun on n00b.

@bishabosha
Copy link
Member

bishabosha commented Jan 10, 2024

here is the relevant doc section to update: https://scala-lang.org/files/archive/spec/3.4/01-lexical-syntax.html#integer-literals

@odersky
Copy link
Contributor

odersky commented Jan 10, 2024

Plus, we should have a short doc page in the docs/_docs/reference pages explaining the feature with a short example. We do that for all features that are new in Scala 3.

@Sporarum
Copy link
Contributor Author

I was expecting it to get backported to Scala 2, since there is already an implementation targetting it, will this not be the case ?

@som-snytt
Copy link
Contributor

@Sporarum that is likely. scala/scala#10656

@odersky
Copy link
Contributor

odersky commented Jan 11, 2024

But in any case we need a doc page to explain the new feature, whether it will be in Scala 2 or not. It should not be hard...

@Sporarum
Copy link
Contributor Author

@odersky
Totally agree, and I'll try to do it this weekend, it's more a question of where, and thus in which format ?

@odersky
Copy link
Contributor

odersky commented Jan 11, 2024

I think it should have an entry here:

https://dotty.epfl.ch/docs/reference/index.html

Local directory is docs/_docs/reference.

@SethTisue
Copy link
Member

Note that the Scala 2 PR is now merged (for release in Scala 2.13.13).

@Sporarum
Copy link
Contributor Author

I have added a short reference page, let me know if something is missing

@bishabosha bishabosha added the needs-minor-release This PR cannot be merged until the next minor release label Jan 14, 2024
@Sporarum Sporarum requested a review from bishabosha January 16, 2024 17:38
@Sporarum
Copy link
Contributor Author

I added the line to the second sidebar, and updated the Scala 3 Spec (I mistook the earlier message with one speaking about the Scala 2 Spec)

If it's good like that, let me know and I'll squash my commits together

Implement, test and document SIP-42

(Implement is a strong word, I uncommented a single line)
@Sporarum
Copy link
Contributor Author

@bishabosha, @sjrd I wasn't sure if I could merge it, so I'll leave you to it

@sjrd sjrd enabled auto-merge January 20, 2024 21:06
@sjrd sjrd merged commit a9a75c1 into scala:main Jan 20, 2024
16 checks passed
@Sporarum Sporarum deleted the binaryLiterals branch January 20, 2024 22:25
@Kordyjan Kordyjan added this to the 3.4.1 milestone Feb 14, 2024
@nicolasstucki nicolasstucki modified the milestones: 3.4.1, 3.5.0 Feb 15, 2024
Kordyjan added a commit that referenced this pull request Feb 15, 2024
Reverts #19405 as it was marked as needing minor release.
@nicolasstucki
Copy link
Contributor

nicolasstucki commented Feb 16, 2024

I noticed we are missing a test for 0b1010l (Long with lower case).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-minor-release This PR cannot be merged until the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants