typelevel / scala

Typelevel Scala, a fork of Scala
http://typelevel.org/scala/
372 stars 21 forks source link

Implicit resolution (with literal types) missed in type aliasing. #162

Closed soronpo closed 6 years ago

soronpo commented 7 years ago

See comments in code

 object TT {
    trait Foo {
      type T0
    }

    object Foo {
      type Aux[T] = Foo {type T0 = T}
      implicit def apply[T](implicit v : ValueOf[T]) : Aux[T] = new Foo {
        type T0 = T
      }
    }
  }
  type Foo[T] = TT.Foo.Aux[T]
  val Foo = TT.Foo

  Foo[5] //OK
  implicitly[Foo.Aux[5]] //OK!
  implicitly[Foo[5]] //implicit not found error!
soronpo commented 7 years ago

Actually, this turns out to be a scalac issue, if we use Witness:

  import shapeless.Witness
  val three = Witness(3)
  type Three = three.T
  type Something[T] = shapeless.Witness.Aux[T]
  implicitly[shapeless.Witness.Aux[Three]] //works
  implicitly[Something[Three]] //implicit not found
soronpo commented 6 years ago

@milessabin Please notice that this is actually a LBS issue https://github.com/scala/bug/issues/10453, that TLS suffers from. So perhaps consider closing this bug.

milessabin commented 6 years ago

It is, but it's in a near enough neighbourhood that I'd like to convince myself whether or not this is an issue which should be addressed here.

milessabin commented 6 years ago

The problem here seems to be that the type parameter of ValueOf doesn't have a <: Singleton bound which in turn allows the type parameter of Foo.apply to not have a <: Singleton bound which means that the default widening behaviour is applicable in the third case.

IIRC the motivation for not having a <: Singleton bound on the type parameter of ValueOf was to allow for ValueOf[Unit]. I have to confess I no longer find that particularly compelling, particularly seeing as ().type isn't supported either here or in Dotty.

So my proposed fix for this issue is to add the bound and drop the instance for Unit ... @soronpo would you like to chime in before I go ahead with that?

soronpo commented 6 years ago

If I understand correctly, then the Witness example fails for scalac because it too does not have an upper-bound and widening occurs. This solution means that all users would need to update their type parameters and add the upper bound everywhere. Somewhat annoying 😟 . Maybe you should get feedback from the community, and not just me. Can't we change the widening behavior for literal types (and add it to SIP23)?

soronpo commented 6 years ago

Rethinking this, why does type aliasing cause widening? This really feels like a bug, and if this is indeed the expected behavior, then perhaps a new SIP is required to prevent this.

milessabin commented 6 years ago

It's not that type aliases cause widening. It's that without a <: Singleton bound singleton types won't be inferred ... that's behaviour we can't change without breaking much too much existing code.

There does appear to be an undesirable interaction with aliasing here though, I agree ... I'll take another look.

milessabin commented 6 years ago

The following is fine in Dotty,

object Test {
  def main(args: Array[String]) = println("")

  final class ValueOf[T](val value: T) extends AnyVal
  object ValueOf {
    implicit val valueOf5: ValueOf[5] = new ValueOf(5)
  }

  trait Foo0 {
    type T0
  }

  object Foo0 {
    type Aux[T] = Foo0 { type T0 = T}
    implicit def apply[T](implicit v : ValueOf[T]) : Aux[T] = new Foo0 {
      type T0 = T
    }
  }

  type Foo[T] = Foo0.Aux[T]
  val Foo = Foo0

  Foo[5] //OK
  implicitly[Foo.Aux[5]] //OK!
  implicitly[Foo[5]] // also OK
}

So I'll make it work here as well.

soronpo commented 6 years ago

I wanted to test in Dotty, but didn't have a ValueOf. Nice trick 😄

milessabin commented 6 years ago

FWIW, using the <: Singleton bound to prevent widening doesn't appear to be implemented in Dotty,

object Test {
  case class Widened[T](value: Boolean)
  object Widened {
    implicit def notWidened[T <: Singleton]: Widened[T] = Widened(false)
    implicit def widened[T]: Widened[T] = Widened(true)
  }

  def widened[T](t: T)(implicit w: Widened[T]): Boolean = w.value

  def boundedWidened[T <: Singleton](t: T)(implicit w: Widened[T]): Boolean = w.value

  def main(args: Array[String]) = {
    assert(widened(23))
    assert(!boundedWidened(23)) // Type argument Int does not conform to upper bound Singleton (only fails in Dotty)
    assert(widened(()))
    assert(boundedWidened(())) // found: Unit required: Nothing (fails in both Dotty and Scala)
  }
}
milessabin commented 6 years ago

Fixed in https://github.com/milessabin/scala/commit/e76d2483cb4864882943959208a9b5b09457f82b.

soronpo commented 6 years ago

I see you included the commit as part of the literal types branch. Shouldn't this be a separate PR against LBS to fix https://github.com/scala/bug/issues/10453 ?

milessabin commented 6 years ago

No, I don't think it should be. The issue can't be fixed without the widening-suppression behaviour which is part of SIP-23.

soronpo commented 6 years ago

Oh, I see. So this also a Spec change and not just an implementation. I will close the LBS bug. Many thanks!