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

Bodies of non-private type aliases must be invariant #6566

Closed
scabug opened this issue Oct 24, 2012 · 18 comments
Closed

Bodies of non-private type aliases must be invariant #6566

scabug opened this issue Oct 24, 2012 · 18 comments

Comments

@scabug
Copy link

scabug commented Oct 24, 2012

Minimization is due to paulp,

object WhatsYourTypeIsMyType {
  class TypeCheat[+T] { type MyType = T }

  class Foo {
    val tc = new TypeCheat[Foo]
    var x: tc.MyType = _
    def setX() = x = new Foo
  }
  class Bar extends Foo {
    override val tc = new TypeCheat[Bar]
    def unsound = this

    setX()
    println(x.unsound)
  }
  def main(args: Array[String]): Unit = new Bar
}

Compiles and when run produces a CCE,

iles@lewis:scala$ scala-master -classpath . WhatsYourTypeIsMyType
java.lang.ClassCastException: WhatsYourTypeIsMyType$Foo cannot be cast to WhatsYourTypeIsMyType$Bar
        at WhatsYourTypeIsMyType$Bar.<init>(bug.scala:14)
        at WhatsYourTypeIsMyType$.main(bug.scala:16)
        at WhatsYourTypeIsMyType.main(bug.scala)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:601)
        at scala.tools.nsc.util.ScalaClassLoader$$anonfun$run$1.apply(ScalaClassLoader.scala:71)
        at scala.tools.nsc.util.ScalaClassLoader$class.asContext(ScalaClassLoader.scala:31)
        at scala.tools.nsc.util.ScalaClassLoader$URLClassLoader.asContext(ScalaClassLoader.scala:139)
        at scala.tools.nsc.util.ScalaClassLoader$class.run(ScalaClassLoader.scala:71)
        at scala.tools.nsc.util.ScalaClassLoader$URLClassLoader.run(ScalaClassLoader.scala:139)
        at scala.tools.nsc.CommonRunner$class.run(ObjectRunner.scala:28)
        at scala.tools.nsc.ObjectRunner$.run(ObjectRunner.scala:45)
        at scala.tools.nsc.CommonRunner$class.runAndCatch(ObjectRunner.scala:35)
        at scala.tools.nsc.ObjectRunner$.runAndCatch(ObjectRunner.scala:45)
        at scala.tools.nsc.MainGenericRunner.runTarget$1(MainGenericRunner.scala:74)
        at scala.tools.nsc.MainGenericRunner.process(MainGenericRunner.scala:96)
        at scala.tools.nsc.MainGenericRunner$.main(MainGenericRunner.scala:105)
        at scala.tools.nsc.MainGenericRunner.main(MainGenericRunner.scala)
@scabug
Copy link
Author

scabug commented Oct 24, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6566?orig=1
Reporter: @milessabin
Affected Versions: 2.9.2, 2.10.0-RC1

@scabug
Copy link
Author

scabug commented Oct 24, 2012

@milessabin said:
Related, and simpler,

object WhatsYourTypeIsMyType {
  trait WithMyType[+T] {
    type MyType = T
  }
  
  class Foo extends WithMyType[Foo] {
    var x: MyType = _
    def setX() = x = new Foo  
  }
  
  class Bar extends Foo with WithMyType[Bar] {
    def unsound { println("iAmABar") }
    
    setX()
    println(x.unsound)
  }

  def main(args: Array[String]): Unit = new Bar
}

@scabug
Copy link
Author

scabug commented Oct 24, 2012

@paulp said:
Here's another way of looking at it:

scala> class In[+T] { type MyType = T }
defined class In

scala> trait Out[T] { val tc: In[T] ; def f(x: tc.MyType): Unit }
defined trait Out

scala> val o1: Out[String] { val tc: In[AnyRef] } = new Out[String] { val tc = new In[String] ; def f(s: String) = () }
o1: Out[String]{val tc: In[AnyRef]} = $anon$1@42929c90

scala> o1.f('hi)
java.lang.ClassCastException: scala.Symbol cannot be cast to java.lang.String
	at $anon$1.f(<console>:9)
	at .<init>(<console>:11)
	at .<clinit>(<console>)

I was amazed to see this had gone undiscovered for so long, but then I saw it's a regression... from scala 2.7. Same failure in 2.8 and beyond, but in 2.7 it makes the seemingly correct observation that

scala> class In[+T] { type MyType = T }
<console>:4: error: covariant type T occurs in invariant position in type T of type MyType
       class In[+T] { type MyType = T }
                           ^

@scabug
Copy link
Author

scabug commented Dec 10, 2012

@retronym said:

2.10.x ~/code/scala git diff
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scal
index fbeb401..9d05412 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -894,7 +894,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.int
               // However, if `sym` does override a type in a base class
               // we have to assume NoVariance, as there might then be
               // references to the type parameter that are not variance checked.
-              state = if (sym.isOverridingSymbol) NoVariance else AnyVariance
+              state = NoVariance
             }

2.10.x ~/code/scala  squalac sandbox/t6566.scala
sandbox/t6566.scala:1: error: covariant type T occurs in invariant position in type T of type MyType
class In[+T] { type MyType = T }
                    ^
one error found

scala/scala@68bcc9e7

Suppresses variance checking in type aliases; exapnds all type aliases
instead.

@scabug
Copy link
Author

scabug commented Dec 10, 2012

@retronym said:
Only one failure in partest --pos --neg after that patch.

testing: [...]/files/neg/variances.scala                              [FAILED]
16,18d15
< variances.scala:89: error: covariant type T occurs in invariant position in type T of type A
<     type A = T
<          ^
22c19
< 7 errors found
---
> 6 errors found

See also:

scala/scala@8a6e20ce

Which introduced the test that now reports two errors rather than one:

object TestAlias {
  class B[-T]
  trait C[+T] {
    type A = T
    def foo: B[A]
  }
}

@scabug
Copy link
Author

scabug commented Dec 10, 2012

@paulp said:
I notice just now this is related to #5120. In both tickets there is an invariant type parameter, and an abstract type which is derived from the type parameter. A loss of fidelity in that derivation allows the "invariant" type to vary, and unsoundness results. In #5120 the lever is how existentials are simplified, and here the lever is variance checking. Since abstract types can be used in many more ways than type parameters (by virtue of being members, and because they can be referenced from subclasses and from the outside) and because they are considerably less fully exercised than type parameters, I think it likely we have more variations on this theme to discover.

@scabug
Copy link
Author

scabug commented Dec 10, 2012

@odersky said:
I am not 100% sure what caused the change from 2.7 to 2.8. There must have been a reason, but I don't remember it. The reversal that Jason proposes looks certainly safe. So I'd say let's do it now, and watch out whether something major stops compiling.

@scabug
Copy link
Author

scabug commented Dec 11, 2012

@retronym said:
retronym/scala@scala:2.10.x...retronym:ticket/6656
https://scala-webapps.epfl.ch/jenkins/job/scala-checkin-manual/696/console

If that goes well, I'll ask Josh to run a community build. We can then choose whether we want to ship the fix on master or 2.10.x.

@scabug
Copy link
Author

scabug commented Dec 11, 2012

@retronym said:
I retract my unfounded optimism.

quick.lib:
    [mkdir] Created dir: /localhome/jenkins/a/workspace/scala-checkin-manual/build/quick/classes/library
    [javac] Compiling 32 source files to /localhome/jenkins/a/workspace/scala-checkin-manual/build/quick/classes/library
    [javac] Note: /localhome/jenkins/a/workspace/scala-checkin-manual/src/library/scala/collection/concurrent/MainNode.java uses unchecked or unsafe operations.
    [javac] Note: Recompile with -Xlint:unchecked for details.
    [javac] Compiling 41 source files to /localhome/jenkins/a/workspace/scala-checkin-manual/build/quick/classes/library
    [javac] Note: Some input files use unchecked or unsafe operations.
    [javac] Note: Recompile with -Xlint:unchecked for details.
[scalacfork] Compiling 719 files to /localhome/jenkins/a/workspace/scala-checkin-manual/build/quick/classes/library
[scalacfork] /localhome/jenkins/a/workspace/scala-checkin-manual/src/library/scala/collection/TraversableLike.scala:80: error: covariant type Repr occurs in invariant position in type Repr of type Self
[scalacfork]   protected type Self = Repr
[scalacfork]                  ^
[scalacfork] /localhome/jenkins/a/workspace/scala-checkin-manual/src/library/scala/collection/generic/GenericClassTagCompanion.scala:22: error: covariant type CC occurs in invariant position in type CC[_] of type Coll
[scalacfork]   type Coll = CC[_]
[scalacfork]        ^
[scalacfork] /localhome/jenkins/a/workspace/scala-checkin-manual/src/library/scala/collection/generic/GenericCompanion.scala:27: error: covariant type CC occurs in invariant position in type CC[_] of type Coll
[scalacfork]   type Coll = CC[_]
[scalacfork]        ^
[scalacfork] /localhome/jenkins/a/workspace/scala-checkin-manual/src/library/scala/collection/generic/GenericOrderedCompanion.scala:22: error: covariant type CC occurs in invariant position in type CC[_] of type Coll
[scalacfork]   type Coll = CC[_]
[scalacfork]        ^
[scalacfork] /localhome/jenkins/a/workspace/scala-checkin-manual/src/library/scala/collection/immutable/TrieIterator.scala:49: error: covariant type T occurs in invariant position in type ((Iterator[T], Int), Iterator[T]) of type SplitIterators
[scalacfork]   private type SplitIterators = ((Iterator[T], Int), Iterator[T])
[scalacfork]                ^
[scalacfork] /localhome/jenkins/a/workspace/scala-checkin-manual/src/library/scala/collection/mutable/IndexedSeqView.scala:36: error: covariant type Coll occurs in invariant position in type scala.collection.mutable.IndexedSeqView[A,Coll] of type This
[scalacfork]   private[this] type This = IndexedSeqView[A, Coll]
[scalacfork]                      ^
[scalacfork] /localhome/jenkins/a/workspace/scala-checkin-manual/src/library/scala/collection/parallel/ParSeqLike.scala:48: error: covariant type T occurs in invariant position in type scala.collection.parallel.IterableSplitter[T] of type SuperParIterator
[scalacfork]   type SuperParIterator = IterableSplitter[T]
[scalacfork]        ^
trait ParSeqLike[+T, +Repr <: ParSeq[T], +Sequential <: Seq[T] with SeqLike[T, Sequential]]
extends scala.collection.GenSeqLike[T, Repr]
   with ParIterableLike[T, Repr, Sequential] {
self =>
  
  type SuperParIterator = IterableSplitter[T]

@scabug
Copy link
Author

scabug commented Jan 2, 2013

@paulp said:
Patch pending.

@scabug
Copy link
Author

scabug commented Jan 22, 2013

@adriaanm said:
fixed for 2.11.0-M1 in a419799f87

@scabug
Copy link
Author

scabug commented Feb 1, 2013

@paulp said:
Can't see how to fix it for 2.10. Here's the commit message I just wrote for the partial backport.

    SI-6566, unsoundness with alias variance [backport]
    
    This is only a partial backport, and does not include the
    most important part, but it is the part which I could reasonably
    backport. The rest relies on my overhaul of variance which
    takes it out of reach for 2.10.x.
    
    What is included here are the type soundness fixes in
    the collections library. Which, come to think of it, won't
    be binary compatible so this whole effort is doomed.

@scabug scabug closed this as completed Feb 1, 2013
@scabug
Copy link
Author

scabug commented Feb 16, 2014

@Blaisorblade said:
To be sure, the issue is that S and T in Fun must be invariant, right? But not in Fun0.

trait Fun[-S, +T] {type Eval = S => T}

trait Fun0[-S, +T] {
  private[this] type Eval = S => T
}

I hope this bug is linked in the release notes.
I proposed the following spec update — scala/scala-dist#131. It seems the issue was not discussed at all.

@scabug
Copy link
Author

scabug commented Feb 16, 2014

@Blaisorblade said:
I also updated the comment in scala/scala#3537.

I was also trying to refine the spec documenting the current behavior (which is questionable, but I hope it approximates what should be documented to guarantee soundness), and/or comparing what's implemented with the current spec, but I had to give up on that.

@scabug
Copy link
Author

scabug commented Feb 17, 2014

@retronym said:
@paolo: The release notes will contain a list of all fixed bugs. But to highlight particular changes, we can also add a section to: https://github.com/scala/make-release-notes/blob/master/hand-written.md. Would you like to submit a PR for that?

@scabug
Copy link
Author

scabug commented Feb 17, 2014

@Blaisorblade said:
Sounds good. I will look for a suitable phrasing.

@scabug
Copy link
Author

scabug commented Feb 17, 2014

@Blaisorblade said:
scala/make-release-notes#46

I also added "specification" as component, since scala/scala-dist#131 is still open. (I'm not sure whether to reopen the bug, but I'll better leave the judgment call to you guys).

@scabug
Copy link
Author

scabug commented Mar 21, 2014

@adriaanm said:
See also #7093 on an important note regarding protected[this].

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