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

Scala 2.13.x compiler stackoverflows (at scala.reflect.internal.Types$TypeVar.registerBound) #11474

Closed
thesamet opened this Issue Apr 6, 2019 · 17 comments

Comments

Projects
None yet
5 participants
@thesamet
Copy link

commented Apr 6, 2019

I have the following Java code, this is a simplified version of a class hierarchy from protobuf-java:

package foo;

public class Outer {
    public interface BaseBuilder { }

    public abstract static class Container<B> { }

    public abstract static class Builder<
          M extends Container<B>,
          B extends Builder<M, B>>
      implements BaseBuilder { }

    public abstract static class Builder1<T extends Builder1<T>>
            extends Builder { }

    public abstract class Builder2 extends Builder1<Builder2> { }

    interface MyBase {
        BaseBuilder newBuilder();
    }

    public static abstract class M1 implements MyBase  {
        public Outer.Builder2 newBuilder() { return null; }
    }

    public static abstract class M2 implements MyBase {
        public Builder newBuilder() { return null; }
    }
}

And this scala code that references it:

package foo;

object O {
  Seq(
    new Outer.M2 {},
    new Outer.M1 {} 
  )
}

The following gets the compiler to go into infinite recursion into:

[error] java.lang.StackOverflowError
[error]      at scala.collection.mutable.HashMap.apply(HashMap.scala:84)
[error]      at scala.reflect.internal.tpe.TypeMaps$ExistentialExtrapolation.$anonfun$countOccs$1(TypeMaps.scala:317)
[error]      at scala.reflect.internal.tpe.TypeMaps$ExistentialExtrapolation.$anonfun$countOccs$1$adapted(TypeMaps.scala:314)
[error]      at scala.reflect.internal.tpe.TypeMaps$ForEachTypeTraverser.traverse(TypeMaps.scala:1097)
[error]      at scala.reflect.internal.tpe.TypeMaps$TypeTraverser.apply(TypeMaps.scala:257)
[error]      at scala.reflect.internal.tpe.TypeMaps$TypeTraverser.apply(TypeMaps.scala:255)
[error]      at scala.reflect.internal.Types$TypeRef.mapOver(Types.scala:2353)
[error]      at scala.reflect.internal.tpe.TypeMaps$ForEachTypeTraverser.traverse(TypeMaps.scala:1098)
[error]      at scala.reflect.internal.tpe.TypeMaps$TypeTraverser.apply(TypeMaps.scala:257)
[error]      at scala.reflect.internal.Types$TypeBounds.mapOver(Types.scala:1569)
[error]      at scala.reflect.internal.tpe.TypeMaps$ForEachTypeTraverser.traverse(TypeMaps.scala:1098)
[error]      at scala.reflect.internal.tpe.TypeMaps$TypeTraverser.apply(TypeMaps.scala:257)
[error]      at scala.reflect.internal.tpe.TypeMaps$TypeMap.applyToSymbolInfo(TypeMaps.scala:122)
[error]      at scala.reflect.internal.tpe.TypeMaps$TypeMap.loop$1(TypeMaps.scala:116)
[error]      at scala.reflect.internal.tpe.TypeMaps$TypeMap.firstChangedSymbol(TypeMaps.scala:120)
[error]      at scala.reflect.internal.tpe.TypeMaps$TypeMap.mapOver(TypeMaps.scala:134)
[error]      at scala.reflect.internal.Types$ExistentialType.mapOver(Types.scala:3165)
[error]      at scala.reflect.internal.tpe.TypeMaps$ForEachTypeTraverser.traverse(TypeMaps.scala:1098)
[error]      at scala.reflect.internal.Types$Type.foreach(Types.scala:793)
[error]      at scala.reflect.internal.tpe.TypeMaps$ExistentialExtrapolation.countOccs(TypeMaps.scala:314)
[error]      at scala.reflect.internal.tpe.TypeMaps$ExistentialExtrapolation.extrapolate(TypeMaps.scala:323)
[error]      at scala.reflect.internal.Types.existentialAbstraction(Types.scala:4211)
[error]      at scala.reflect.internal.Types.existentialAbstraction$(Types.scala:4165)
[error]      at scala.reflect.internal.SymbolTable.existentialAbstraction(SymbolTable.scala:28)
[error]      at scala.reflect.internal.Types$TypeVar.$anonfun$registerBound$7(Types.scala:3640)
[error]      at scala.reflect.internal.ExistentialsAndSkolems.existentialTransform(ExistentialsAndSkolems.scala:116)
[error]      at scala.reflect.internal.ExistentialsAndSkolems.existentialTransform$(ExistentialsAndSkolems.scala:95)
[error]      at scala.reflect.internal.Types$TypeVar.registerBound(Types.scala:3640)
[error]      at scala.reflect.internal.Types$TypeVar.registerBound(Types.scala:3641)
[error]      at scala.reflect.internal.Types$TypeVar.registerBound(Types.scala:3641)
[error]      at scala.reflect.internal.Types$TypeVar.registerBound(Types.scala:3641)
[error]      at scala.reflect.internal.Types$TypeVar.registerBound(Types.scala:3641)
[error]      at scala.reflect.internal.Types$TypeVar.registerBound(Types.scala:3641)
[error]      at scala.reflect.internal.Types$TypeVar.registerBound(Types.scala:3641)
[error]      at scala.reflect.internal.Types$TypeVar.registerBound(Types.scala:3641)
[error]      at scala.reflect.internal.Types$TypeVar.registerBound(Types.scala:3641)
[error]      at scala.reflect.internal.Types$TypeVar.registerBound(Types.scala:3641)
...

This happens when I compile JavaThenScala on 2.13.0-M5 and beyond (including the latest nightly 2.13.0-pre-63769af), but works correctly in 2.12.x.

For context, I encountered this bug in ScalaPB unit tests. The Java code is simplified protobuf-java code and M1 and M2 code generated by the Java protobuf generator. The Scala code is pretty trivial, so I assume this may be an issue for any Java protobuf users who work with Scala (regardless of whether they use ScalaPB).

For convenience, I created a mininal repo that reproduces this problem at https://github.com/thesamet/scalac-stackoverflow

@hrhino

This comment has been minimized.

Copy link
Member

commented Apr 6, 2019

Looks like it's having trouble computing the LUB of M1 and M2 (because they both refine the type of newBuilder.

You should be able to work around this by explicitly passing the type argument to Seq.apply: Seq[MyBase](new M1{}, new M2{}), assuming that's what you intend.

thesamet added a commit to scalapb/ScalaPB that referenced this issue Apr 7, 2019

@joroKr21

This comment has been minimized.

Copy link

commented Apr 7, 2019

Hmm, looks like JavaThenScala is required to reproduce the bug. What's the difference?

@joroKr21

This comment has been minimized.

Copy link

commented Apr 7, 2019

Yikes, looks like F-bounded existential problems. This is my fault: scala/scala#6421. An easy solution might be - try to do this only once. I.e. if the existential type is still not relatable, give up. Need to find a mimization in one Scala file though.

@joroKr21

This comment has been minimized.

Copy link

commented Apr 7, 2019

public Builder newBuilder() { return null; }

What does that even mean? Builder takes F-bounded type parameters with complex bounds. Java is so gross sometimes...

@smarter

This comment has been minimized.

Copy link

commented Apr 7, 2019

Hmm, looks like JavaThenScala is required to reproduce the bug. What's the difference?

JavaThenScala means that scalac won't see the .java files, instead it'll see classfiles which will be parsed with https://github.com/scala/scala/blob/2.13.x/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala. If JavaThenScala isn't set then the .java sources will be seen by scalac (this allow mutual references between .java and .scala sources), the .java sources are parsed using https://github.com/scala/scala/blob/2.13.x/src/compiler/scala/tools/nsc/javac/JavaParsers.scala.

What does that even mean? Builder takes F-bounded type parameters with complex bounds. Java is so gross sometimes...

It's a raw type, a legacy feature from pre-generics Java: https://docs.oracle.com/javase/tutorial/java/generics/rawTypes.html

@joroKr21

This comment has been minimized.

Copy link

commented Apr 7, 2019

this allow mutual references between .java and .scala sources

This isn't the case here so it must be some other difference in the parsers.

From the linked doc:

When using raw types, you essentially get pre-generics behavior — a Box gives you Objects

That was my understanding so far, but Object doesn't satisfy the bounds.
You can't use the upper bounds either because they are F-bounded.

@smarter

This comment has been minimized.

Copy link

commented Apr 7, 2019

That was my understanding so far, but Object doesn't satisfy the bounds.

Yeah, javac doesn't care.

@SethTisue

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

:honey-badger:

@hrhino

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

Wait, no, that doesn't make sense.

If you have

class Foo<T extends Bar> {
  T meh();
}

then meh will have descriptor ()LBar; and signature ()TT;, but java <1.5 sees only the descriptor and should see meh returning Bar, not Object.

That said, trying to think of what the Scala equivalent of the raw type Builder is hurts my head. It's 2019, and we still need to deal with this...?

@smarter

This comment has been minimized.

Copy link

commented Apr 7, 2019

@hrhino

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

I guess some horror like

type RawBuilder = Builder[M, B] forSome {
  type M <: Container[B]
  type B <: Builder[M, B] 
}

although I recall there being subtleties to this that I'm probably forgetting.

(That forSome thing is an existential type that cannot be expressed by wildcards, for those Dotty folks sufficiently blessed not to have to know about them :P )

@hrhino

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

@smarter do I understand you as saying that Dotty considers this as just Builder[_, _] and it works out? If so, that's both surprising (to me) and relieving (to all, I'd guess). If I were at a computer I'd go poke at the type-cooking code in scalac, but right now I'm just going off of memory.

@smarter

This comment has been minimized.

Copy link

commented Apr 7, 2019

@hrhino Yes, I think so.

@joroKr21

This comment has been minimized.

Copy link

commented Apr 8, 2019

Confusing 🤔

typeOf[foo.Outer#Builder2]
BTS(foo.Outer#Builder2,foo.Outer.Builder1[foo.Outer#Builder2],foo.Outer.Builder[M,B] forSome { type M <: foo.Outer.Container[B]; type B <: foo.Outer.Builder[M,B] },foo.Outer.BaseBuilder,Object,Any)
abstract class Builder3
    extends (Outer.Builder[M, B] forSome { type M <: Outer.Container[B]; type B <: Outer.Builder[M, B] })

[error] class type required but foo.Outer.Builder[M,B] forSome { type M <: foo.Outer.Container[B]; type B <: foo.Outer.Builder[M,B] } found
[error]     extends (Outer.Builder[M, B] forSome { type M <: Outer.Container[B]; type B <: Outer.Builder[M, B] })

so which one is it?

@joroKr21

This comment has been minimized.

Copy link

commented Apr 16, 2019

Ok how can I write a test that does the JavaThenScala compilation order from the original example?

I couldn't concoct a pure Scala reproduction.

@smarter

This comment has been minimized.

Copy link

commented Apr 16, 2019

Put the Java code in a file ending with _1.java and the Scala code in a file ending with _2.scala

@joroKr21

This comment has been minimized.

Copy link

commented Apr 16, 2019

Put the Java code in a file ending with _1.java and the Scala code in a file ending with _2.scala

This doesn't help. It still compiles fine. It looks like it's parsing the source and not the class file.

Edit: Oh God, I was testing Seq(new M1, new M2) instead of Seq(new M2, new M1) 😆

It works thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.