Skip to content

export interacting weirdly with scala.Conversion #12700

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

Closed
erikerlandson opened this issue Jun 3, 2021 · 16 comments
Closed

export interacting weirdly with scala.Conversion #12700

erikerlandson opened this issue Jun 3, 2021 · 16 comments

Comments

@erikerlandson
Copy link

Compiler version

3.0.0

Minimized code

package repro

export wrap.Wrap as Wrap
export wrap.lift as lift

// separate issue: trying to define this at top-level errors out in other ways.
object wrap:
    opaque type Wrap[V, T] = V

    extension[V, T](w: Wrap[V, T])
        def value: V = w

    extension[V](v: V)
        def lift[T]: Wrap[V, T] = v
end wrap

object conversions:
    given g[T]: scala.Conversion[Wrap[Double, T], Wrap[Double, String]] =
        new scala.Conversion[Wrap[Double, T], Wrap[Double, String]]:
            def apply(w: Wrap[Double, T]): Wrap[Double, String] =
                (w.value * 2).lift[String]

Output

scala> val t: Wrap[Double, String] = 1D.lift[Int]                                                                                        
1 |val t: Wrap[Double, String] = 1D.lift[Int]
  |                              ^^^^^^^^^^^^
  |Found:    repro.wrap.Wrap[Double, Int]
  |Required: repro.Wrap²[Double, String]
  |
  |where:    Wrap  is a type in object wrap with bounds <: [V, T] =>> Any
  |          Wrap² is a type in package object wrap$package which is an alias of [V, T] =>> repro.wrap.Wrap[V, T]

Expectation

I would expect scala to understand that repro.wrap.Wrap and repro.Wrap are the same thing, and not error out.

@erikerlandson
Copy link
Author

So far, I have not seen any failures of this kind pertaining to other given types that I have defined. This may be specific to scala.Conversion although I don't understand why it would be special with respect to this

@som-snytt
Copy link
Contributor

som-snytt commented Jun 3, 2021

In the same compilation unit, the error is

   |                                Found:    repro.wrap.Wrap[Double, Int]
   |                                Required: repro.wrap.Wrap[Double, String]

It works anywhere when importing from conversions. Maybe I don't understand the issue.

Is #12591 a similar question, to reconcile two paths through an export, either for behavior or just for messaging?

@erikerlandson
Copy link
Author

I believe #12591 is getting at the same idea: In my repro above, the scala.Conversion should automatically convert a Wrap[Double, Int] into a Wrap[Double, String], but the compiler is erroring out because it believes repro.Wrap is different than repro.wrap.Wrap - in other words, it doesn't understand the paths are equivalent, and so it concludes no Conversion object can be created.

@som-snytt
Copy link
Contributor

@erikerlandson it works if you import repro.conversions.given. Is there a reason that shouldn't be required? This is what I tried:

package repro2

@main def test(): Unit =
  import language.implicitConversions
  import repro.{given, *}
  import repro.conversions.given
  val t: Wrap[Double, String] = 1D.lift[Int]
  println(t)

@erikerlandson
Copy link
Author

I also imported that in my testing. However, I will play with it some more and see if I can find something else wrong in my code

@erikerlandson
Copy link
Author

Out of curiosity, have you tested in a REPL? generally these things work either way but occasionally a repl causes subtle differences

@som-snytt
Copy link
Contributor

I was going to say maybe REPL is not doing your import. I didn't try in REPL, just run class from CLI.

@som-snytt
Copy link
Contributor

Oh right, scala -classpath /tmp doesn't work because of broken option handling. Bummer. Maybe you tried in sbt console?

@erikerlandson
Copy link
Author

yes, I've been running with ./mill -i coulomb.console
I've been doing all my testing that way for a while now, with pretty complicated givens and metaprogramming, and it's been working fine. Either I have an error in this code that I haven't found yet (entirely possible) or there's something different about scala.Conversion

@som-snytt
Copy link
Contributor

Aha. For the record

➜  snips ~/projects/dotty/bin/scala
scala> @main def test(): Unit =
     |   import language.implicitConversions
     |   import repro.{given, *}
     |   import repro.conversions.given
     |   val t: Wrap[Double, String] = 1D.lift[Int]
     |   println(t)
     |
def test(): Unit

scala> test()
2.0

scala> import language.implicitConversions

scala> import repro.{given, *}

scala> import repro.conversions.given

scala> val t: Wrap[Double, String] = 1D.lift[Int]
val t: repro.Wrap[Double, String] = 2.0

@erikerlandson
Copy link
Author

weird - well I'll have to double check my local reproducer then

@erikerlandson
Copy link
Author

so... my reproducer above also works for me, I might have missed the .given on import

However, I did figure out what was different about scala.Conversion that makes it fail. My "real" definition is:

given g1[U1, U2](using conv: Coefficient[U1, U2]):
        scala.Conversion[Quantity[Double, U1], Quantity[Double, U2]] =
    new scala.Conversion[Quantity[Double, U1], Quantity[Double, U2]]:
        val c = conv.coef.toDouble
        def apply(q: Quantity[Double, U1]): Quantity[Double, U2] =
            (q.value * c).withUnit[U2]

And what's happening is that scala.Conversion's signature is Conversion[-T, +U], and it's those covariance and contravariance modifiers that are causing my chained using conv: Coefficient[U1, U2] to fail down inside the metaprogramming. All my other definitions are invariant - I can reproduce similar failures in other types if I add covariance modifiers like for Conversion

@erikerlandson
Copy link
Author

actual error looks like this:

scala> summon[scala.Conversion[Quantity[Double, Yard], Quantity[Double, Meter]]]
1 |summon[scala.Conversion[Quantity[Double, Yard], Quantity[Double, Meter]]]
  |                                                                         ^
  |units are not convertable: (TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),object scala),Any)) (TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),object scala),Nothing))

almost like scala is forcing the types to Any, Nothing somewhere, which is interesting since Any => Nothing is the subclass of all functions

@som-snytt
Copy link
Contributor

The old joke for Scala 2 was to switch variances until the code compiles.

I think for Scala 3, it's add transparent inline. Not sure if this comment is relevant to your code.

@erikerlandson
Copy link
Author

Yes, that other issue looks very similar.

Hilarious, it was the definition for Coefficient that needed to be transparent inline - I had deliberately removed that since the result didn't seem to require any dependent typing "and why make the compiler work harder" 😆

    transparent inline given [U1, U2]: Coefficient[U1, U2] =
        ${ coulomb.infra.meta.coefficient[U1, U2] }

@erikerlandson
Copy link
Author

Works now:

scala> val t: Quantity[Double, Meter] = (1D.withUnit[Yard])
val t: coulomb.Quantity[Double, coulomb.si.Meter] = 0.9144

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

2 participants