-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Cannot define a path-dependent Conversion instance #7412
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
Comments
can I pick this up? |
@neko-kai can you assign this to me? |
@ashwinbhaskar I can't, i'm not a dotty developer and don't have any permissions. Maybe @smarter could? (sorry for bothering) |
@ashwinbhaskar I assigned you |
@neko-kai @bishabosha thank you:) |
This syntax does not make sense in general (what happens if the second type parameter appears in a context where given Conversion[(s: String), Named[_]] {
def apply(s: String): Named[s.type] = new Named(s)
} But that would also require some special-casing of Conversion. Since we want to discourage people from using conversions, this might not be worth it. If it is, then a different syntax (like |
IMHO it can conceivably make sense for all subtypes of Function* where these type parameters appear in appropriate positions with appropriate variance.
AFAIU the special case would appear only in Implicits.scala? (i.e. use refined apply signature for search & result, not type parameters) That seems completely fine to me, Conversion is already special-cased there.
shapeless, akka, singleton-ops, spire etc. afaik use dependent conversions. An even more absolutely massive amount of libraries use implicit def macro conversions. IMHO this will come up again and it will be easier to support earlier before the codebase becomes more mature. Also scalafix rules to rewrite conversions cannot be made comprehensive until Conversion reaches parity with implicit def. |
Yes, it's not too bad.
Good point, could you link to some examples ? |
Off the top: shapeless:
sbt: My company uses both dependent and macro conversions in open-source and closed-source as well. I can find more usages if you ask again for more proofs of usage. |
If the second type parameter appears in a context where But I think this syntax does make sense, in general. We already have it for |
@julienrf The syntax does not work for
Did you mean that if it is made to work for |
Yes, we could maybe generalize the path-dependent syntax to other types than |
@neko-kai Do you remember where Akka used these? I'd be interested to check that out. |
implicit def applicativeInstance[A[_]](implicit ap: Applicative[A]): Instance.Aux[A] Most of the implicit |
That doesn't look like a dependent method type to me, there's no parameter or result type which refers to a previous term parameter |
@dwijnand @smarter akka-http's Directive.scala has two 1 2 For a total of 7 in akka-http.
I mentioned implicit def macros for sbt. Why did I intentionally conflate dependent conversions and macro conversions here?
trait Fixture0[A] extends Conversion[Int, A] {
def a: A
override def apply(i: Int): A = i match {
case 0 => a
case _ => throw new AssertionError("Runtime error: access only from 0 not any other number!")
}
}
trait Fixture[A] extends Fixture0[A] {
override inline def apply(i: Int): A = inline i match {
case 0 => a
case _ => error("Compile error: access only from 0 not any other number!")
}
} |
Instead of writing a Conversion[T, U] we could use Conversion[T => U] which also allows dependent conversion Conversion[(x:T) => x.U] Here, opaque type Conversion[+F <: Nothing => Any] = F See #8523 for more details |
#8523 was merged — was this ticket left open on purpose (because of @neko-kai's caveat at https://contributors.scala-lang.org/t/proposal-changes-to-implicit-conversions/4101/15), or just by accident? |
#8523 does not change the current implementation, it's a test file showing a prototype of what a changed implementation could look like. |
minimized code
expectation
Expected being able to define an instance of
Conversion
equivalent toimplicit def convert(s: String): Named[s.type]
. However the definition above fails to parse!The text was updated successfully, but these errors were encountered: