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

VerifyError with separate compilation and method/Object flattening #6493

Open
scabug opened this issue Oct 9, 2012 · 23 comments
Open

VerifyError with separate compilation and method/Object flattening #6493

scabug opened this issue Oct 9, 2012 · 23 comments
Milestone

Comments

@scabug
Copy link

scabug commented Oct 9, 2012

Here is the smallest case I could generate:

scala> def foo = { class Foo { class Bar { val b = 2 }}; val f = new Foo; new f.Bar }

java.lang.AssertionError: assertion failed: f.type
	at scala.tools.nsc.symtab.Types$adaptToNewRunMap$.adaptToNewRun(Types.scala:3945)
	at scala.tools.nsc.symtab.Types$adaptToNewRunMap$.apply(Types.scala:3989)
	at scala.tools.nsc.symtab.Types$adaptToNewRunMap$.apply(Types.scala:3921)
	at scala.tools.nsc.symtab.Types$TypeMap.mapOver(Types.scala:3175)
	at scala.tools.nsc.symtab.Types$adaptToNewRunMap$.apply(Types.scala:4037)
	at scala.tools.nsc.symtab.Types$adaptToNewRunMap$.apply(Types.scala:4010)

I was originally playing around in the interpreter trying to understand the Currency examples in Ordersky Ch.20 and ran into this by doing the following. Note how it accepts the definition once and then break when I attempt to redefine it in exactly the same way. I'm still trying to wrap my head around path dependent types - so I don't have much more useful debugging info.

scala> abstract class Outer { abstract class Inner { val v = 1 } }
defined class Outer

scala> def doSomething(n: Int) = { object O extends Outer { class I extends Inner { val y = n } }; new O.I }
doSomething: (n: Int)O.I forSome { val O: O; type O <: Outer with ScalaObject{type I <: this.Inner with ScalaObject{val y: Int}} }

scala> def doSomething(n: Int) = { object O extends Outer { class I extends Inner { val y = n } }; new O.I }
doSomething: (n: Int)O.I forSome { val O: O; type O <: Outer with ScalaObject{type I <: this.Inner with ScalaObject{val y: Int}} }
java.lang.AssertionError: assertion failed: O.type
	at scala.tools.nsc.symtab.Types$adaptToNewRunMap$.adaptToNewRun(Types.scala:3945)
	at scala.tools.nsc.symtab.Types$adaptToNewRunMap$.apply(Types.scala:3989)
	at scala.tools.nsc.symtab.Types$adaptToNewRunMap$.apply(Types.scala:3921)
	at scala.tools.nsc.symtab.Types$TypeMap.mapOver(Types.scala:3175)
	at scala.tools.nsc.symtab.Types$adaptToNewRunMap$.apply(Types.scala:4037)
@scabug
Copy link
Author

scabug commented Oct 9, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6493?orig=1
Reporter: Kartik Subramanian (selfification)
Affected Versions: 2.9.2
Attachments:

  • diff.png (created on Feb 21, 2013 4:45:00 PM UTC, 300105 bytes)

@scabug
Copy link
Author

scabug commented Oct 9, 2012

@paulp said:
In 2.10 this gives the following, which is pretty cool because it involves multiple, far-flung bug fixes.

scala> def foo = { class Foo { class Bar { val b = 2 }}; val f = new Foo; new f.Bar }
foo: Object{type Bar <: Object{val b: Int}}#Bar

@scabug
Copy link
Author

scabug commented Oct 9, 2012

Kartik Subramanian (selfification) said (edited by @paulp on Feb 1, 2013 6:57:34 AM UTC):
That's cool! I updated to scala 2.10.0-M7 and retried:

scala> def foo() = { object Foo { class Bar { val x = 1} }; new Foo.Bar }
foo: ()Object{type Bar <: Object{val x: Int}}#Bar

scala> foo()
java.lang.NoSuchMethodError: .foo()Ljava/lang/Object;
	at .<init>(<console>:9)
	at .<clinit>(<console>)
	at .<init>(<console>:7)

Now it typechecks.. but doesn't run. Is this expected? Sorry I elided the "new Foo" bit and just made an object Foo. Doing it the long way also raises an exception.

@scabug
Copy link
Author

scabug commented Feb 21, 2013

@paulp said:
As is typically the case with seemingly repl bugs, this is a compiler bug with separate compilation.

// a.scala
object one {
  def foo() = { object Foo { class Bar } ; new Foo.Bar }
}
// b.scala
object Test {
  def main(args: Array[String]): Unit = one.foo
}
% scalac210 a.scala && scalac210 b.scala && scala210 Test
java.lang.NoSuchMethodError: one$.foo()Lone/foo$Foo$Bar;
	at Test$.main(b.scala:2)
	at Test.main(b.scala)

Affects all versions back to at least 2.8.

@scabug
Copy link
Author

scabug commented Feb 21, 2013

@paulp said:
I'm taking the extreme measure of attaching a screenshot because it's so much more effective at showing what is happening than is anything I can accomplish via jira. The parts in red are the separately compiled bytecode; the green is the together compiled (working) version.

@scabug
Copy link
Author

scabug commented May 30, 2013

@retronym said:
I'd point the finger at erasure.

    // end of uncurry
    <method> def foo(): lang.this.Object{type Bar <: lang.this.Object}#Bar = {
    // end of erasure
    <method> def foo(): Foo.Bar = {

I'd expect the return type to erase to Object.

Seems like we get away with this under joint compilation.

@scabug
Copy link
Author

scabug commented May 30, 2013

@retronym said:
I was too quick to blame erasure. A corrupt TypeRef is being passed all the way from typer.

<method> def foo#7445(): scala#21.this.AnyRef#2222{type Bar#12153 <: scala#21.this.AnyRef#2222}#Bar#12125

It should be:

<method> def foo#7445(): scala#21.this.AnyRef#2222{type Bar#12153 <: scala#21.this.AnyRef#2222}#Bar#12123

I believe that this assumption is incorrect:

    // only need to rebind type aliases, as typeRef already handles abstract types
    // (they are allowed to be rebound more liberally)
    def coevolveSym(pre1: Type): Symbol = sym

Compare with the override in AliasTypeRef:

    // #3731: return sym1 for which holds: pre bound sym.name to sym and
    // pre1 now binds sym.name to sym1, conceptually exactly the same
    // symbol as sym.  The selection of sym on pre must be updated to the
    // selection of sym1 on pre1, since sym's info was probably updated
    // by the TypeMap to yield a new symbol, sym1 with transformed info.
    // @returns sym1
    override def coevolveSym(pre1: Type): Symbol =
      if (pre eq pre1) sym else (pre, pre1) match {
        // don't look at parents -- it would be an error to override alias types anyway
        case (RefinedType(_, _), RefinedType(_, decls1)) => decls1 lookup sym.name
        // TODO: is there another way a typeref's symbol can refer to a symbol defined in its pre?
        case _                                           => sym
      }

@scabug
Copy link
Author

scabug commented May 30, 2013

@retronym said (edited on May 31, 2013 12:12:23 PM UTC):
Here's an attempted fix:

https://github.com/retronym/scala/compare/ticket/6493

@scabug
Copy link
Author

scabug commented Sep 25, 2013

@retronym said:
scala/scala#2986

@scabug
Copy link
Author

scabug commented Feb 11, 2014

@adriaanm said:
Part of the problem is existentialTransform's use of subst instead of substSym:

    def doSubst(info: Type) = info.substSym(rawSyms, typeParams) // don't use subst, as it won't update pre.sym[args]  to pre.newSym[args]  unless pre == NoPrefix!

@scabug
Copy link
Author

scabug commented Feb 11, 2014

@adriaanm said:
the type symbols are still out of whack after this, but at least now we select an abstract type rather than a class symbol off of the new existential, so that we avoid the NoSuchMethodError

@scabug
Copy link
Author

scabug commented Feb 11, 2014

@adriaanm said (edited on Feb 11, 2014 10:14:56 PM UTC):
scala/scala#3510

@scabug
Copy link
Author

scabug commented Feb 12, 2014

@adriaanm said:
Vaguely related problem:

scala> class Test {
     |   def c: x.X forSome { val x : {type X <: C}; type C } = ???
     |   lazy val x = c // lazy vals are borked when dealing with existentials
     | }
<console>:9: error: type mismatch;
 found   : x.type(in lazy value x)#X(in <refinement of AnyRef>)(in <refinement of AnyRef>) where type x.type(in lazy value x) <: AnyRef{type X(in <refinement of AnyRef>)(in <refinement of AnyRef>) <: C} with Singleton
 required: (some other)x.type(in lazy value x)#X(in <refinement of AnyRef>)(in <refinement of AnyRef>) forSome { type (some other)x.type(in lazy value x) <: AnyRef{type X(in <refinement of AnyRef>)(in <refinement of AnyRef>) <: C} with Singleton; type C }
         lazy val x = c // lazy vals are borked when dealing with existentials
                      ^
<console>:9: error: type mismatch;
 found   : x.type(in value x)#X(in <refinement of AnyRef>)(in <refinement of AnyRef>) where type x.type(in value x) <: AnyRef{type X(in <refinement of AnyRef>)(in <refinement of AnyRef>) <: C} with Singleton
 required: x.type(in lazy value x)#X(in <refinement of AnyRef>)(in <refinement of AnyRef>) forSome { type x.type(in lazy value x) <: AnyRef{type X(in <refinement of AnyRef>)(in <refinement of AnyRef>) <: C} with Singleton; type C }
         lazy val x = c // lazy vals are borked when dealing with existentials
                  ^

@scabug
Copy link
Author

scabug commented Feb 12, 2014

@paulp said (edited on Feb 12, 2014 7:29:55 AM UTC):
lazy vals are really an innocent victim, their mistake is using a var in the implementation.
Here is another, free of lazy vals.

class Test {
  val c: x.X forSome { val x : { type X <: AnyRef with C }; type C } = ???
}

@scabug
Copy link
Author

scabug commented Feb 12, 2014

@paulp said:
That compiles if it's private[this], which is how you can tell it's the correspondence between getter and field. With vars it's even more booched thanks to the setter. See also the old comment of mine which retronym brought up in some other ticket recently.

    case class Getter(tree: ValDef) extends BaseGetter(tree) {
      override def derivedSym = if (mods.isDeferred) basisSym else basisSym.getter(enclClass)
      private def derivedRhs  = if (mods.isDeferred) EmptyTree else fieldSelection
      private def derivedTpt = {
        // For existentials, don't specify a type for the getter, even one derived
        // from the symbol! This leads to incompatible existentials for the field and
        // the getter. Let the typer do all the work. You might think "why only for
        // existentials, why not always," and you would be right, except: a single test
        // fails, but it looked like some work to deal with it. Test neg/t0606.scala
        // starts compiling (instead of failing like it's supposed to) because the typer
        // expects to be able to identify escaping locals in typedDefDef, and fails to
        // spot that brand of them. In other words it's an artifact of the implementation.
        val tpt = derivedSym.tpe_*.finalResultType.widen match {
          // Range position errors ensue if we don't duplicate this in some
          // circumstances (at least: concrete vals with existential types.)
          case ExistentialType(_, _)  => TypeTree() setOriginal (tree.tpt.duplicate setPos tree.tpt.pos.focus)
          case _ if mods.isDeferred   => TypeTree() setOriginal tree.tpt // keep type tree of original abstract field
          case tp                     => TypeTree(tp)
        }
        tpt setPos tree.tpt.pos.focus
      }
      override def derivedTree: DefDef = newDefDef(derivedSym, derivedRhs)(tpt = derivedTpt)
    }

@scabug
Copy link
Author

scabug commented Feb 12, 2014

@adriaanm said:
I debugged it to subtyping for existentials being broken.

      val underlying1 = underlying.instantiateTypeParams(quantified, tvars) // fuse subst quantified -> quantifiedFresh -> tvars

does not go into symbol infos, thus the C in X's bound is not replaced by a type var

@scabug
Copy link
Author

scabug commented Feb 12, 2014

@paulp said:
You mean "substitution" not "subtyping" right? Not that subtyping for existentials isn't broken.

@scabug
Copy link
Author

scabug commented Feb 13, 2014

@adriaanm said:
subtyping is broken because substitution is -- you can't tell, because existentialTransform transitively chases all bounds so it doesn't matter, but that's not the right fix (as that rewrite is not universally valid, and we shouldn't be changing these user-defined types like that anyway)

scala> type T = X forSome { type X <: U ; type U <: String }
scala> typeOf[T].dealias.skolemizeExistential.typeSymbol.info
res7: $r.intp.global.Type =  <: String

If you disable that bound rewriting (as I'm doing), this will yield false:

scala> :power
scala> class C { type T = X.T forSome { val X : U ; type U <: {type T} } }
scala> typeOf[C].member("T": TypeName)
scala> res0.info.skolemizeExistential <:< res0.info

@scabug
Copy link
Author

scabug commented Feb 13, 2014

@adriaanm said:
Note that another benefit of not rewriting these bounds is that you don't have to copy refined types in existentialTransform, which is what complicates a proper fix for this bug.

@scabug
Copy link
Author

scabug commented Feb 13, 2014

@adriaanm said:
scala/scala#3522

the remainder of my saga turned out to be #2071, which I hope to fix in 2.12 (https://github.com/adriaanm/scala/compare/t2071

@scabug
Copy link
Author

scabug commented Feb 23, 2014

@adriaanm said:
I'll submit a fix under -Xexperimental for RC2, a full fix is blocked by #2071, which won't make it into 2.11.0. Probably for 2.12.

@scabug
Copy link
Author

scabug commented Feb 24, 2014

@adriaanm said:
-Xexperimental fix at scala/scala#3578

@scabug
Copy link
Author

scabug commented Mar 17, 2014

@adriaanm said:
Thus turned out to be much harder than I thought, and we're out of time for 2.11. As this has a good chance of affecting source compatibility, I'll target 2.12.0-M1 for now. Maybe we'll be able to backport if we're confident the fix won't cause regressions.

@scabug scabug added this to the 2.13.0-RC1 milestone Apr 7, 2017
@adriaanm adriaanm removed their assignment Sep 28, 2018
@adriaanm adriaanm modified the milestones: 2.13.0-RC1, Backlog Nov 13, 2018
@scala scala deleted a comment from scabug Feb 14, 2024
@scala scala deleted a comment from scabug Feb 14, 2024
@scala scala deleted a comment from scabug Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants