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

type aliases + variance checks: to dealias or not to dealias (and: dealiasing loses annotations on type arguments) #8079

Closed
scabug opened this issue Dec 13, 2013 · 12 comments

Comments

@scabug
Copy link

scabug commented Dec 13, 2013

There's no reason CovTrait should be afforded any privilege above CovAlias.

object Bug {
  import scala.annotation.unchecked.{ uncheckedVariance => uV }

  trait Base[T] { def f: java.util.Iterator[T] }
  trait CovTrait[+T] extends java.util.Iterator[T @uV]
  type CovAlias[+T] = java.util.Iterator[T @uV]

  // compiles
  class A[+T] extends Base[T @uV] { def f: CovTrait[T] = null }
  // fails
  class B[+T] extends Base[T @uV] { def f: CovAlias[T] = null }
  // <console>:17: error: covariant type T occurs in invariant position in type => Bug.CovAlias[T] of method f
  //          class B[+T] extends Base[T @uV] { def f: CovAlias[T] = null }
  //                                                ^
}
@scabug
Copy link
Author

scabug commented Dec 13, 2013

Imported From: https://issues.scala-lang.org/browse/SI-8079?orig=1
Reporter: @paulp
See #8071

@scabug
Copy link
Author

scabug commented Dec 13, 2013

@paulp said:
That's not always an option:

illegal inheritance from final class Optional
[error] trait Optional[+T] extends java.util.Optional[T @uV]
[error]                                      ^
[error] one error found

Without fixing this there is no way to treat java.util.Optional (for instance - as one example among endless) as the covariant type it should be without adding an uncheckedVariance annotation at every single use site until you die. Of course there's no point in that, you can write _ <: A at every single use site until you die, and that's probably shorter.

scala> import scala.annotation.unchecked.{ uncheckedVariance => uV }
import scala.annotation.unchecked.{uncheckedVariance=>uV}

scala> type Opt[+A] = java.util.Optional[A @uV]
defined type alias Opt

scala> class A[+T](val x: Opt[T])
<console>:9: error: covariant type T occurs in invariant position in type => Opt[T] of value x
       class A[+T](val x: Opt[T])
                       ^

scala> class A[+T](val x: java.util.Optional[T])
<console>:8: error: covariant type T occurs in invariant position in type => java.util.Optional[T] of value x
       class A[+T](val x: java.util.Optional[T])
                       ^

scala> class A[+T](val x: Option[T])
defined class A

scala> class A[+T](val x: java.util.Optional[_ <: T])
defined class A

scala> class A[+T](val x: Opt[T @uV])
defined class A

@scabug
Copy link
Author

scabug commented Dec 14, 2013

@retronym said:
Relatedly, I've found that the way that AsSeenFrom discards (most) annotations is the root cause of #8071. I asked for the history of this, and Martin said that originally no annotations where preserved through typemaps. When CPS needed it, rather than passing them all through, only the CPS related annotation was passed through. Why not pass them all? He wasn't sure, but a plausible theory was it was an attempt at a minimal change to solve the problem at hand.

I'm going to try to pass through all annotations to see if anything breaks or gets slower. Hopefully that change will also sort out this ticket.

@scabug
Copy link
Author

scabug commented Dec 14, 2013

@paulp said:
I spotted #8071 last night on my scan of recent tickets. Funny timing.

Annotations have all kinds of different and not easily categorized influence. Discarding them selectively without anyone even knowing the reasoning why... I guess that's where I check out of this comment.

@scabug
Copy link
Author

scabug commented Dec 14, 2013

@paulp said (edited on Dec 14, 2013 4:47:56 PM UTC):
If that doesn't work for some reason - and of course things will depend on exactly what typemaps currently happen to do - maybe another route is for uncheckedVariance to extend TypeConstraint. I can't understand the comment on TypeConstraint, and uncheckedVariance is more of a TypeUnconstraint, but in the spirit of addressing all things symptomatically.

@scabug
Copy link
Author

scabug commented Dec 16, 2013

@paulp said:

I'm going to try to pass through all annotations to see if anything breaks or gets slower. Hopefully that change will also sort out this ticket.

In case you haven't tried that yet, I did. Here you go.

https://github.com/paulp/scala/tree/pr/keep-type-annotations

Although the branch is prefixed with "pr/" I won't be sending a pull request because I'm not interested in dealing with the two failures. But the branch is some evidence the change is easily makable.

@scabug
Copy link
Author

scabug commented Sep 25, 2014

@scabug
Copy link
Author

scabug commented Sep 25, 2014

@retronym said:
Aside from the question of honouring uncheckedVariance in alias RHS, which can be done by preserving annotations through type maps as Paul's experiment has confirmed, we should also pause to ask the broader question: after the fix for #6566, which restores variance checking of the RHS of type aliases, is checking expansions redundant or somehow still material to prevent unsoundness.

% cat sandbox/test.scala
trait C[-I] {
  type X = C[I]
  def f2(b: X): Unit
}
2.11.x /code/scala for v in 7 8 9 10 11; do version=2.$v; echo $version; ~/scala/$version/bin/scalac sandbox/test.scala; done
2.7
sandbox/test.scala:2: error: contravariant type I occurs in invariant position in type C[I] of type X
  type X = C[I]
       ^
one error found
2.8
sandbox/test.scala:3: error: contravariant type I occurs in covariant position in type C.this.X of value b
  def f2(b: X): Unit
         ^
one error found
2.9
sandbox/test.scala:3: error: contravariant type I occurs in covariant position in type C.this.X of value b
  def f2(b: X): Unit
         ^
one error found
2.10
sandbox/test.scala:3: error: contravariant type I occurs in covariant position in type C.this.X of value b
  def f2(b: X): Unit
         ^
one error found
2.11
sandbox/test.scala:2: error: contravariant type I occurs in invariant position in type C[I] of type X
  type X = C[I]
       ^
sandbox/test.scala:3: error: contravariant type I occurs in covariant position in type C.this.X of value b
  def f2(b: X): Unit
         ^
two errors found

If we omit the checks on the RHS because the alias is sufficiently private, we clearly must check expansions. For example, the following is unsound but was allowed in Scala 2.7:

trait C[-I] {
  private[this] type X = C[I]
  def f2(b: X): Unit
}

Intuitively, I think we only need to check in one place. But I'll give it some more thought.

@scabug
Copy link
Author

scabug commented Sep 26, 2014

@scabug
Copy link
Author

scabug commented Apr 18, 2016

@samskivert said:
With the improved SAM support coming in 2.12, one is tempted to make heavier use of the java.util.function types in code that must interop with Java. Being able to tell scalac that these types have their natural variance via aliases is one of the last remaining pain points in making this interop pleasant. I'm currently working around this with custom traits, but it would be awesome if I didn't have to weird my APIs with these types.

I'm happy to provide what support I can in getting this resolved, but it seems like the right fix here is something that can only be decided by someone with intimate knowledge of the nefarious machinations of the compiler.

@scabug
Copy link
Author

scabug commented Apr 18, 2016

@paulp said (edited on Apr 18, 2016 2:54:15 PM UTC):
See also my last comment on #8071, written two years ago tomorrow. "But #8079 doesn't. Just a reminder that it is easy to fix. Java 8 is out and has a lot going for it. One thing scala can do to assist in its survival is offer nicer wrappers around java 8. It has to be possible to write them though."

@scabug
Copy link
Author

scabug commented Aug 19, 2016

@SethTisue said:
scala/scala#5280

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