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

given failing to resolve when parameter is curried prior to using #19523

Closed
erikerlandson opened this issue Jan 24, 2024 · 7 comments
Closed

Comments

@erikerlandson
Copy link

Compiler version

scala 3.3.1

Minimized code

object reprodefs:
    trait Addable[V]:
        def add(x: V, y: V): V

    case class R[V](value: V):
        // this works
        def add(using alg: Addable[V])(r: R[V]): R[V] =
            R(alg.add(this.value, r.value))

        // putting 'r' before 'using alg' will fail
        def addFails(r: R[V])(using alg: Addable[V]): R[V] =
            R(alg.add(this.value, r.value))

    given scala.Conversion[R[Int], R[Double]] with
        def apply(r: R[Int]): R[Double] = R(r.value.toDouble)

    given Addable[Double] with
        def add(x: Double, y: Double): Double = x + y

object repro:
    import reprodefs.{*, given}
    import scala.language.implicitConversions

    // works fine
    val w = R(1.0).add(R(1))

    // this will cause compiler error where it incorrectly types
    // Addable and fails to find it
    val f = R(1.0).addFails(R(1))

Output

It seems to be incorrectly assigning type Addable[Double | Int] and so not finding it.

[error] 706 |    val f = R(1.0).addFails(R(1))
[error]     |                                 ^
[error]     |No given instance of type reprodefs.Addable[Double | Int] was found for parameter alg of method addFails in class R
[error] one error found
[error] (coreJVM / Compile / compileIncremental) Compilation failed

Expectation

I would expect the compiler to properly type Addable[] regardless of whether r was curried before or after the using alg parameter.

@erikerlandson erikerlandson added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 24, 2024
@WojciechMazur WojciechMazur added area:typer and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 24, 2024
@WojciechMazur
Copy link
Contributor

AFAIK this is treated as a feature, and not a bug, and is a difference between Scala 2 and Scala 3 type system.
The first version def add(using alg: Addable[V])(r: R[V]): R[V] would materialize the type V to Double based on Addable instance, this way it requires r to be also an instance of R[Double] which allows for triggering Conversion[Int, Double]
The second version def addFails(r: R[V])(using alg: Addable[V]): R[V] would first resolve/widen to the common type, which happens to be Double | Int and only based on that would try to search for implicit instance.

@erikerlandson
Copy link
Author

erikerlandson commented Jan 24, 2024

What concerns me (maybe) is that it means scala.Conversion only works if I curry the using parameters first. Since the type V is fixed at creation time of R[V] I'm confused about why this doesn't work both ways.

On the other hand, I think it will be possible to tweak the signatures of my methods to curry the using first, so if that is the way it has to work, it should be feasible for me.

@odersky
Copy link
Contributor

odersky commented Jan 27, 2024

Yes, type inference is tricky. Before implicit arguments are searched type variables appearing in preceding arguments are instantiated. If the preceding argument is 1.0 it's going to be Double, but if there are two arguments 1.0 and 1, it's going to be Double | Int. So this works as specced.

@erikerlandson
Copy link
Author

Thank you for looking at it! In the case of methods on a class, I find this a bit unintuitive, only because once I create an R[Double] I'd expect V to be unambiguously fixed as Double, and if I tried to give it Int, it would either find some valid Conversion to R[Double] or fail.

I also tried this with "free" functions, and in that case the Double | Int typing made sense to me, since it has no other reference point.

Regardless, if this isn't a bug I will proceed with figuring out how to make my signatures work with both my using params and Conversion

@odersky odersky closed this as not planned Won't fix, can't repro, duplicate, stale Jan 27, 2024
@erikerlandson
Copy link
Author

Just for posterity, doing this such that Conversion works makes some of my method signatures extra complex, for example this where I have three curried parameter blocks.
However, it does appear to work the way I need it to.

        transparent inline def *[UR](using
            alg: MultiplicativeSemigroup[VL]
        )(qr: Quantity[VL, UR])(using
            su: SimplifiedUnit[UL * UR]
        ): Quantity[VL, su.UO] =
            alg.times(ql.value, qr.value).withUnit[su.UO]

@odersky
Copy link
Contributor

odersky commented Jan 28, 2024

The mid-term goal should be to get rid of implicit conversions entirely. We should now have better way to do things.

@erikerlandson
Copy link
Author

What is the better way? I'm happy to explore another design approach if implicit conversions are something scala would like to make obsolete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants