Skip to content

Update extension syntax in scodec #9534

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

Merged

Conversation

nicolasstucki
Copy link
Contributor

This should unblock #9476.

The trick was to invert the arguments of the :: extension and give one of them a DummyImplicit.
Not sure if the inversion of arguments was intentional, we should double-check this behavior.

@nicolasstucki nicolasstucki self-assigned this Aug 11, 2020
@nicolasstucki nicolasstucki requested a review from odersky August 11, 2020 10:54
@nicolasstucki nicolasstucki force-pushed the update-extension-syntax-in-scodec branch from 59e2ef9 to d9ebd3a Compare August 11, 2020 11:30
@nicolasstucki nicolasstucki force-pushed the update-extension-syntax-in-scodec branch from d9ebd3a to f4e7362 Compare August 11, 2020 11:43
@nicolasstucki nicolasstucki marked this pull request as ready for review August 11, 2020 12:45
Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

Yes, in fact the inversion of arguments is necessary. The old syntax used flipped parameters for
single extension methods relative to collective ones. So it was:

extension on (xs: List[Int]):
  def :: (x: Int): List[Int]

but

def (x: Int) :: (xs::List[Int]): List[Int]

The new extension syntax uses the natural x then xs order throughout. I.e.

extension (x: Int)
  def (xs: List[Int]): List[Int]

Great that you spotted this!

@odersky odersky merged commit 15a6c8b into scala:master Aug 16, 2020
@odersky odersky deleted the update-extension-syntax-in-scodec branch August 16, 2020 09:58
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