Skip to content

Implement into modifier on parameter types #14514

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
merged 4 commits into from
Nov 21, 2022

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Feb 18, 2022

in the thread https://contributors.scala-lang.org/t/proposed-changes-and-restrictions-for-implicit-conversions/4923
we discussed two changes that would make implicit conversions largely redundant. One proposed change was implemented in #14497. The other is implemented here. It's based on #14497.

The idea of this PR is to have some way to specify that a method parameter accepts implicit conversions on its arguments. Then the feature warning on implicit conversion use would be suppressed in this case. In the previous thread I proposed
~ to mark convertible arguments but I now feel this is too cryptic. Instead there is a into prefix in the
parameter type. E.g.

  def f(x: into T) = ...

For Scala 2, we introduce an annotation on the parameter that expresses the same thing:

  // proposed Scala-2 syntax:
  def f(@allowConversions x: T) = 

A larger example:

given Conversion[String, Text] = Text(_)

@main def Test =

  def f(x: into Text, y: => into Text, zs: into Text*) =
    println(s"${x.str} ${y.str} ${zs.map(_.str).mkString(" ")}")

  f("abc", "def")  // ok, no feature warning
  f("abc", "def", "xyz", "uvw")  // also ok
  f("abc", "def", "xyz", Text("uvw"))  // also ok

A larger example is a parser combinator library that works without unrestricted implicit conversions:
tests/run/Parser.scala

@odersky
Copy link
Contributor Author

odersky commented Feb 18, 2022

Open questions

  1. Is this the right way to express implicit convertibility in source?
  2. Internally we mark convertible parameters with the annotation @scala.annotation.internal.$convertible. Is this the right way to do it, or do we prefer a more robust scheme?

@odersky
Copy link
Contributor Author

odersky commented Feb 18, 2022

The last commit bases the implementation on an internal alias type instead of an annotation. This makes
convertibleTo handling unforgeable.

@odersky odersky force-pushed the add-convertible-2 branch 2 times, most recently from 415cb86 to 35710f6 Compare February 18, 2022 19:58
@odersky odersky changed the title Implement convertibleTo modifier on parameter types Implement into modifier on parameter types Feb 19, 2022
@odersky odersky marked this pull request as draft February 20, 2022 15:18
@nicolasstucki
Copy link
Contributor

Will we need to extend this concept at some point to lambdas?

@odersky odersky force-pushed the add-convertible-2 branch 3 times, most recently from e47a34a to 6525fd3 Compare September 28, 2022 10:22
@odersky odersky marked this pull request as ready for review September 29, 2022 11:49
@odersky
Copy link
Contributor Author

odersky commented Sep 29, 2022

I went ahead and made this an experimental feature so that people can try it out.

Points to note:

  • When Scala 3 conversions are applied implicitly they currently require an implicitConversions language import at the use site. (Old style Scala 2 implicit conversions required the language import at definition site, but that turned out to be toothless).
  • With this PR there is now an exception: No language import is required if the implicit conversion is on a method argument where the corresponding parameter is marked into.

This technique addresses the situations where implicit conversions were found to be essential and hard to replace in the discussion thread cited above.

@deusaquilus
Copy link
Contributor

@odersky I'm so sorry to keep pestering you about this but after you closed #16002 I really don't know what else to do. For Quill to work I need to have both implicit conversions Quoted[T] => T and T => Quoted[T]. The into capability might help with the T => Quoted[T] part but it's the Quoted[T] => T that is much more important (I mentioned that on the original thread here).

At this point, the only thing that can save Quill from an eternity of needing implicit-conversions is some kind of injectable Quoted[T] => T conversion that would be injected into quote { ... } blocks. Without something like that, every other kind of feature like this one is completely pointless for Quill.

@odersky
Copy link
Contributor Author

odersky commented Sep 30, 2022

@deusaquilus I don't know a solution either. Maybe Quill won't profit from into. I am not sure what else can be done. For the moment, the language import might be the only way to do it.

@odersky odersky removed their assignment Sep 30, 2022
@odersky odersky force-pushed the add-convertible-2 branch 4 times, most recently from db10040 to a7c6680 Compare November 17, 2022 20:22
Source input:

From Scala 3: `(x: into T)`
From Scala 2: `(@allowConversions x: T)`

Gets translated to

    (x: <into>[T])

where `<into>` is a new synthetic alias marker type defined as

    type <into>[T] = T

`into` is not accessible from user programs.

Parameters labeled `into` allow implicit argument conversions without a

    import language.implicitConversions

import.
@odersky odersky requested a review from dwijnand November 18, 2022 13:32
@odersky odersky assigned dwijnand and unassigned dwijnand and KacperFKorban Nov 18, 2022
@odersky
Copy link
Contributor Author

odersky commented Nov 18, 2022

@dwijnand Can you give this a review?

@odersky
Copy link
Contributor Author

odersky commented Nov 18, 2022

The reason to merge this now is that I'd like to experiment with the compiler to see whether we can get rid of implicit conversions. We need to make progress on this since old style implicit conversions will be deprecated at some point and new style implicit conversions require a language import at the use site.

To make this work, we need to have alternative mechanisms in place that make implicit conversions necessary only in exceptional cases. into is such a mechanism, and right now it is not clear whether it is already sufficient or we need something else as well. Now, the compiler contains thousands of occurrences of implicit conversions. So it is an ideal testing ground to see how far we get in getting rid of them. And before we start we need into.

Co-authored-by: Jamie Thompson <bishbashboshjt@gmail.com>
Co-authored-by: Jamie Thompson <bishbashboshjt@gmail.com>
Copy link
Member

@KacperFKorban KacperFKorban left a comment

Choose a reason for hiding this comment

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

LGTM overall. Just added some minor comments.

Also sorry for causing this PR to hang for so long, I must've dismissed the notification by accident.

@dwijnand dwijnand assigned KacperFKorban and unassigned dwijnand Nov 21, 2022
@dwijnand dwijnand removed their request for review November 21, 2022 09:44
@odersky
Copy link
Contributor Author

odersky commented Nov 21, 2022

@KacperFKorban Thanks for the review!

@KacperFKorban KacperFKorban merged commit b4f8eef into scala:main Nov 21, 2022
@KacperFKorban KacperFKorban deleted the add-convertible-2 branch November 21, 2022 22:38
@Kordyjan Kordyjan added this to the 3.3.0 milestone Aug 1, 2023
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.

7 participants