Merge pull request scala#8049 from adriaanm/pr7966-followup

ObjectTpeJava is truly unique; bounds; isAnyTpe

Author: retronym

src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala

@@ -672,11 +672,11 @@ abstract class ClassfileParser(reader: ReusableInstance[ReusableDataReader]) {
                         case '+' => TypeBounds.upper(sig2type(tparams, skiptvs))
                         case '-' =>
                           val tp = sig2type(tparams, skiptvs)
-                          // sig2type seems to return AnyClass regardless of the situation:
-                          // we don't want Any as a LOWER bound.
-                          if (tp.typeSymbol == AnyClass) TypeBounds.empty
-                          else TypeBounds.lower(tp)
-                        case '*' => TypeBounds.empty
+                          // Interpret `sig2type` returning `Any` as "no bounds";
+                          // morally equivalent to TypeBounds.empty, but we're representing Java code, so use ObjectTpeJava for AnyTpe.
+                          if (tp.typeSymbol == AnyClass) TypeBounds.upper(definitions.ObjectTpeJava)
+                          else TypeBounds(tp, definitions.ObjectTpeJava)
+                        case '*' => TypeBounds.upper(definitions.ObjectTpeJava)
                       }
                       val newtparam = sym.newExistential(newTypeName("?"+i), sym.pos) setInfo bounds
                       existentials += newtparam

src/reflect/scala/reflect/internal/Definitions.scala

@@ -303,7 +303,7 @@ trait Definitions extends api.StandardDefinitions {
      *
      * We use `ObjectTpeJava`'s identity to equate it, but not `ObjectTpe`, to `AnyTpe` in subtyping and type equality.
      */
-    lazy val ObjectTpeJava   = mkObjectTpeJava
+    lazy val ObjectTpeJava   = new ObjectTpeJavaRef
 
     lazy val SerializableTpe = SerializableClass.tpe
     lazy val StringTpe       = StringClass.tpe

src/reflect/scala/reflect/internal/TypeDebugging.scala

@@ -133,7 +133,7 @@ trait TypeDebugging {
       def refine(defs: Scope): String          = defs.toList.mkString("{", " ;\n ", "}")
       def bounds(lo: Type, hi: Type): String   = {
         val lo_s = if (typeIsNothing(lo)) "" else s" >: $lo"
-        val hi_s = if (typeIsAny(hi)) "" else s" <: $hi"
+        val hi_s = if (typeIsAnyOrJavaObject(hi)) "" else s" <: $hi"
         lo_s + hi_s
       }
     }

src/reflect/scala/reflect/internal/Types.scala

@@ -1542,7 +1542,7 @@ trait Types
       case _                => lo <:< that && that <:< hi
     }
     private def emptyLowerBound = typeIsNothing(lo) || lo.isWildcard
-    private def emptyUpperBound = typeIsAny(hi) || hi.isWildcard
+    private def emptyUpperBound = typeIsAnyOrJavaObject(hi) || hi.isWildcard
     def isEmptyBounds = emptyLowerBound && emptyUpperBound
 
     override def safeToString = scalaNotation(_.toString)
@@ -2409,7 +2409,11 @@ trait Types
       if (this eq other.asInstanceOf[AnyRef]) true
       else other match {
         case otherTypeRef: TypeRef =>
-          Objects.equals(pre, otherTypeRef.pre) && sym.eq(otherTypeRef.sym) && sameElementsEquals(args, otherTypeRef.args)
+          Objects.equals(pre, otherTypeRef.pre) &&
+            sym.eq(otherTypeRef.sym) &&
+            sameElementsEquals(args, otherTypeRef.args) &&
+            // `ObjectTpeJavaRef` is not structurally equal to `ObjectTpe` -- they should not be collapsed by `unique`
+            !(this.isInstanceOf[ObjectTpeJavaRef] || otherTypeRef.isInstanceOf[ObjectTpeJavaRef])
         case _ => false
       }
     }
@@ -2706,18 +2710,21 @@ trait Types
   private final class ClassArgsTypeRef(pre: Type, sym: Symbol, args: List[Type]) extends ArgsTypeRef(pre, sym, args)
   private final class AliasNoArgsTypeRef(pre: Type, sym: Symbol) extends NoArgsTypeRef(pre, sym) with AliasTypeRef
   private final class AbstractNoArgsTypeRef(pre: Type, sym: Symbol) extends NoArgsTypeRef(pre, sym) with AbstractTypeRef
-  private final class ClassNoArgsTypeRef(pre: Type, sym: Symbol) extends NoArgsTypeRef(pre, sym){
+  private final class ClassNoArgsTypeRef(pre: Type, sym: Symbol) extends NoArgsTypeRef(pre, sym) {
     override def contains(sym0: Symbol): Boolean = (sym eq sym0) || pre.contains(sym0)
   }
 
-  /** Expose ClassNoArgsTypeRef so we can create a non-uniqued ObjectTpeJava here and in reflect
-   *
-   * NOTE:
-   *   - definitions.ObjectTpe is forced first, so that it ends up in the unique cache.
-   *   - the created TypeRef is structurally equal to  ObjectTpe, but with its own identity
-   *   - we don't want the TypeRef we create here to be unique'd
-   */
-  private[scala] def mkObjectTpeJava: Type = new ClassNoArgsTypeRef(definitions.ObjectTpe.prefix, definitions.ObjectClass)
+  /** Expose ObjectTpeJavaRef so we can create a non-uniqued ObjectTpeJava
+    * (using a type test rather than `eq`, which causes cycles).
+    *
+    * NOTE:
+    *   - definitions.ObjectTpe is forced first, so that it ends up in the unique cache.
+    *   - the created TypeRef is structurally equal to ObjectTpe, but with its own identity
+    *   - we don't want the TypeRef we create here to be unique'd
+    */
+  private[internal] final class ObjectTpeJavaRef extends NoArgsTypeRef(definitions.ObjectTpe.prefix, definitions.ObjectClass) {
+    override def contains(sym0: Symbol): Boolean = (sym eq sym0) || pre.contains(sym0)
+  }
 
   object TypeRef extends TypeRefExtractor {
     def apply(pre: Type, sym: Symbol, args: List[Type]): Type = unique({
@@ -3534,7 +3541,7 @@ trait Types
         if (typeIsNothing(tp)) { // kind-polymorphic
           addBound(NothingTpe)
           true
-        } else if(typeIsAny(tp)) { // kind-polymorphic
+        } else if(typeIsAnyExactly(tp)) { // kind-polymorphic
           addBound(AnyTpe)
           true
         } else if (params.isEmpty) {
@@ -5203,9 +5210,17 @@ trait Types
     }
 
   @tailrec
-  private[scala] final def typeIsAny(tp: Type): Boolean =
+  private[scala] final def typeIsAnyOrJavaObject(tp: Type): Boolean =
+    tp.dealias match {
+      case PolyType(_, tp) => typeIsAnyOrJavaObject(tp)
+      case TypeRef(_, AnyClass, _) => true
+      case _: ObjectTpeJavaRef => true
+      case _ => false
+    }
+
+  private[scala] final def typeIsAnyExactly(tp: Type): Boolean =
     tp.dealias match {
-      case PolyType(_, tp) => typeIsAny(tp)
+      case PolyType(_, tp) => typeIsAnyExactly(tp)
       case TypeRef(_, AnyClass, _) => true
       case _ => false
     }

src/reflect/scala/reflect/internal/tpe/TypeConstraints.scala

@@ -97,7 +97,7 @@ private[internal] trait TypeConstraints {
       *  only guards against being created with them.]
       */
     private[this] var lobounds = lo0 filterNot typeIsNothing
-    private[this] var hibounds = hi0 filterNot typeIsAny
+    private[this] var hibounds = hi0 filterNot typeIsAnyOrJavaObject
     private[this] var numlo = numlo0
     private[this] var numhi = numhi0
     private[this] var avoidWidening = avoidWidening0
@@ -143,7 +143,7 @@ private[internal] trait TypeConstraints {
     def addHiBound(tp: Type, isNumericBound: Boolean = false): Unit = {
       // My current test case only demonstrates the need to let Nothing through as
       // a lower bound, but I suspect the situation is symmetrical.
-      val mustConsider = typeIsAny(tp) || !(hibounds contains tp)
+      val mustConsider = typeIsAnyOrJavaObject(tp) || !(hibounds contains tp)
       if (mustConsider) {
         checkWidening(tp)
         if (isNumericBound && isNumericValueType(tp)) {
@@ -182,7 +182,7 @@ private[internal] trait TypeConstraints {
           case tp :: Nil => " >: " + tp
           case tps       => tps.mkString(" >: (", ", ", ")")
         }
-        val hi = hiBounds filterNot typeIsAny match {
+        val hi = hiBounds filterNot typeIsAnyOrJavaObject match {
           case Nil       => ""
           case tp :: Nil => " <: " + tp
           case tps       => tps.mkString(" <: (", ", ", ")")

src/reflect/scala/reflect/internal/tpe/TypeMaps.scala

@@ -384,6 +384,7 @@ private[internal] trait TypeMaps {
     def apply(tp: Type): Type =
       tp match {
         case BoundedWildcardType(TypeBounds(lo, AnyTpe)) if variance.isContravariant => lo
+        case BoundedWildcardType(TypeBounds(lo, ObjectTpeJava)) if variance.isContravariant => lo
         case BoundedWildcardType(TypeBounds(NothingTpe, hi)) if variance.isCovariant => hi
         case tp => tp.mapOver(this)
       }

test/files/neg/abstract-class-error.check

@@ -1,6 +1,6 @@
 S.scala:1: error: class S needs to be abstract. Missing implementation for:
   def g(y: Int, z: java.util.List): Int // inherited from class J
-(Note that java.util.List does not match java.util.List[String]. To implement this raw type, use java.util.List[_ <: Object])
+(Note that java.util.List does not match java.util.List[String]. To implement this raw type, use java.util.List[_])
 class S extends J {
       ^
 one error found

test/files/neg/abstract-report2.check

@@ -11,7 +11,7 @@ Missing implementations for 13 members. Stub implementations follow:
   def removeAll(x$1: java.util.Collection[_]): Boolean = ???
   def retainAll(x$1: java.util.Collection[_]): Boolean = ???
   def size(): Int = ???
-  def toArray[T <: Object](x$1: Array[T with Object]): Array[T with Object] = ???
+  def toArray[T](x$1: Array[T with Object]): Array[T with Object] = ???
   def toArray(): Array[Object] = ???
 
 class Foo extends Collection[Int]
@@ -29,7 +29,7 @@ Missing implementations for 13 members. Stub implementations follow:
   def removeAll(x$1: java.util.Collection[_]): Boolean = ???
   def retainAll(x$1: java.util.Collection[_]): Boolean = ???
   def size(): Int = ???
-  def toArray[T <: Object](x$1: Array[T with Object]): Array[T with Object] = ???
+  def toArray[T](x$1: Array[T with Object]): Array[T with Object] = ???
   def toArray(): Array[Object] = ???
 
 class Bar extends Collection[List[_ <: String]]
@@ -47,7 +47,7 @@ Missing implementations for 13 members. Stub implementations follow:
   def removeAll(x$1: java.util.Collection[_]): Boolean = ???
   def retainAll(x$1: java.util.Collection[_]): Boolean = ???
   def size(): Int = ???
-  def toArray[T <: Object](x$1: Array[T with Object]): Array[T with Object] = ???
+  def toArray[T](x$1: Array[T with Object]): Array[T with Object] = ???
   def toArray(): Array[Object] = ???
 
 class Baz[T] extends Collection[T]

test/files/pos/t10418_bounds.scala

@@ -0,0 +1,5 @@
+class Test {
+  def foo(c: java.util.Collection[String]): Unit = {
+    c.removeIf(x => true)
+  }
+}

test/files/pos/t11525.scala

@@ -0,0 +1,63 @@
+// scalac: -Ystop-after:refchecks -verbose -Ydebug -uniqid
+package java.lang
+
+/* This is a pretty random test that very indirectly tests `unique`ing of `ObjectTpeJavaRef`
+It's minimize from scala-js, where CI chanced on a compilation order that would first
+unique `TypeBounds(lo, ObjectTpe)`, and then `TypeBounds(lo, ObjectTpeJava)`,
+which would result in a Java reference to Object being replaced by one that is used
+to represent a Scala occurrence of a reference to Object, which is distinct from Any.
+When Java code refers to Object, it's taken as the same thing as Any, at least when
+it comes to =:= and `... <:< Object-in-java`.
+*/
+import java.util.Iterator
+
+class Class[A](o: Object)
+
+class Comparable[A] { def compareTo(o: A): scala.Int = ??? }
+
+object System {
+  def currentTimeMillis(): scala.Long = ???
+
+  def arraycopy(src: Object, srcPos: scala.Int, dest: Object, destPos: scala.Int, length: scala.Int): Unit = {
+    import scala.{Boolean, Double}
+
+    def mismatch(): Nothing =
+      throw new ArrayStoreException("Incompatible array types")
+
+    def copyPrim[@specialized T](src: Array[T], dest: Array[T]): Unit = {
+        var i = length-1
+        while (i >= 0) {
+          dest(i+destPos) = src(i+srcPos)
+          i -= 1
+        }
+    }
+
+    def copyRef(src: Array[AnyRef], dest: Array[AnyRef]): Unit = {
+      val x = (src.length, dest.length)
+
+      var i = length-1
+      while (i >= 0) {
+        dest(i+destPos) = src(i+srcPos)
+        i -= 1
+      }
+    }
+
+    (src match {
+      case src: Array[Boolean] =>
+        dest match {
+          case dest: Array[Boolean] => copyPrim(src, dest)
+          case _                    => mismatch()
+        }
+
+    })
+  }
+
+  def identityHashCode(x: Object): scala.Int = {
+    x.getClass
+    1
+  }
+}
+
+trait Iterable[T] {
+  def iterator():  java.util.Iterator[T]
+}