Merge pull request #2843 from dotty-staging/fix-homogenize

Fix #2833: Fixes to harmonize

Author: odersky

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -29,6 +29,7 @@ import config.Printers.{typr, unapp, overload}
 import TypeApplications._
 import language.implicitConversions
 import reporting.diagnostic.Message
+import Constants.{Constant, IntTag, LongTag}
 
 object Applications {
   import tpd._
@@ -441,6 +442,9 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
                 val harmonizedArgs = harmonizeArgs(typedArgs)
                 if (harmonizedArgs ne typedArgs) {
                   ctx.typerState.constraint = origConstraint
+                    // reset constraint, we will re-establish constraint anyway when we
+                    // compare against the seqliteral. The reset is needed
+                    // otherwise pos/harmonize.scala would fail on line 40.
                   typedArgs = harmonizedArgs
                 }
                 typedArgs.foreach(addArg(_, elemFormal))
@@ -1437,15 +1441,35 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
     }
     val clss = numericClasses(ts, Set())
     if (clss.size > 1) {
+      def isCompatible(cls: Symbol, sup: TypeRef) =
+        defn.isValueSubType(cls.typeRef, sup) &&
+        !(cls == defn.LongClass && sup.isRef(defn.FloatClass))
+          // exclude Long <: Float from list of allowable widenings
+          // TODO: should we do this everywhere we ask for isValueSubType?
+
       val lub = defn.ScalaNumericValueTypeList.find(lubTpe =>
-        clss.forall(cls => defn.isValueSubType(cls.typeRef, lubTpe))).get
-      ts.mapConserve(adapt(_, lub))
+        clss.forall(cls => isCompatible(cls, lubTpe))).get
+
+      def lossOfPrecision(ct: Constant): Boolean =
+        ct.tag == IntTag && lub.isRef(defn.FloatClass) &&
+          ct.intValue.toFloat.toInt != ct.intValue ||
+        ct.tag == LongTag && lub.isRef(defn.DoubleClass) &&
+          ct.longValue.toDouble.toLong != ct.longValue
+
+      val ts1 = ts.mapConserve { t =>
+        tpe(t).widenTermRefExpr match {
+          case ct: ConstantType if !lossOfPrecision(ct.value) => adapt(t, lub)
+          case _ => t
+        }
+      }
+      if (numericClasses(ts1, Set()).size == 1) ts1 else ts
     }
     else ts
   }
 
   /** If `trees` all have numeric value types, and they do not have all the same type,
-   *  pick a common numeric supertype and convert all trees to this type.
+   *  pick a common numeric supertype and convert all constant trees to this type.
+   *  If the resulting trees all have the same type, return them instead of the original ones.
    */
   def harmonize(trees: List[Tree])(implicit ctx: Context): List[Tree] = {
     def adapt(tree: Tree, pt: Type): Tree = tree match {
@@ -1456,9 +1480,10 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
   }
 
   /** If all `types` are numeric value types, and they are not all the same type,
-   *  pick a common numeric supertype and return it instead of every original type.
+   *  pick a common numeric supertype and widen any constant types in `tpes` to it.
+   *  If the resulting types are all the same, return them instead of the original ones.
    */
-  def harmonizeTypes(tpes: List[Type])(implicit ctx: Context): List[Type] =
+  private def harmonizeTypes(tpes: List[Type])(implicit ctx: Context): List[Type] =
     harmonizeWith(tpes)(identity, (tp, pt) => pt)
 }
 

compiler/src/dotty/tools/dotc/typer/Checking.scala

@@ -557,7 +557,8 @@ trait Checking {
       case tp => tp.widenTermRefExpr match {
         case tp: ConstantType if isPureExpr(tree) => // ok
         case tp if defn.isFunctionType(tp) && isPureExpr(tree) => // ok
-        case _ => ctx.error(em"$what must be a constant expression or a function", tree.pos)
+        case _ =>
+          if (!ctx.erasedTypes) ctx.error(em"$what must be a constant expression or a function", tree.pos)
       }
     }
 

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -2098,8 +2098,11 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
             case _: RefTree | _: Literal
             if !isVarPattern(tree) &&
                !(tree.tpe <:< pt)(ctx.addMode(Mode.GADTflexible)) =>
-              val tp1 :: tp2 :: Nil = harmonizeTypes(pt :: wtp :: Nil)
-              checkCanEqual(tp1, tp2, tree.pos)(ctx.retractMode(Mode.Pattern))
+              val cmp =
+                untpd.Apply(
+                  untpd.Select(untpd.TypedSplice(tree), nme.EQ),
+                  untpd.TypedSplice(dummyTreeOfType(pt)))
+              typedExpr(cmp, defn.BooleanType)(ctx.retractMode(Mode.Pattern))
             case _ =>
           }
           tree

docs/docs/reference/dropped/weak-conformance.md

@@ -0,0 +1,87 @@
+---
+layout: doc-page
+title: Dropped: Weak Conformance
+---
+
+In some situations, Scala used a _weak conformance_ relation when
+testing type compatibility or computing the least upper bound of a set
+of types.  The principal motivation behind weak conformance was to
+make as expression like this have type `List[Double]`:
+
+    List(1.0, math.sqrt(3.0), 0, -3.3) // : List[Double]
+
+It's "obvious" that this should be a `List[Double]`. However, without
+some special provision, the least upper bound of the lists's element
+types `(Double, Double, Int, Double)` would be `AnyVal`, hence the list
+expression would be given type `List[AnyVal]`.
+
+A less obvious example is the following one, which was also typed as a
+`List[Double]`, using the weak conformance relation.
+
+    val n: Int = 3
+    val c: Char = 'X'
+    val n: Double = math.sqrt(3.0)
+    List(n, c, d) // used to be: List[Double], now: List[AnyVal]
+
+Here, it is less clear why the type should be widened to
+`List[Double]`, a `List[AnyVal]` seems to be an equally valid -- and
+more principled -- choice.
+
+To simplify the underlying type theory, Dotty drops the notion of weak
+conformance altogether. Instead, it provides more flexibility when
+assigning a type to a constant expression. The new rule is:
+
+ - If a list of expressions `Es` appears as one of
+
+     - the elements of a vararg parameter, or
+     - the alternatives of an if-then-else or match expression, or
+     - the body and catch results of a try expression,
+
+   and all expressions have primitive numeric types, but they do not
+   all have the same type, then the following is attempted: Every
+   constant expression `E` in `Es` is widened to the least primitive
+   numeric value type equal to or above the types of all expressions in `Es`,
+   if that can be done without a loss of precision. Here
+   _above_ and _least_ are interpreted according to the ordering given
+   below.
+
+
+                  Double
+                 /      \
+               Long    Float
+                 \     /
+                   Int
+                 /    \
+              Short   Char
+                |
+              Byte
+
+    A loss of precision occurs for an `Int -> Float` conversion of a constant
+    `c` if `c.toFloat.toInt != c`. For a `Long -> Double` conversion it occurs
+    if `c.toDouble.toLong != c`.
+
+    If these widenings lead to all widened expressions having the same type,
+    we use the widened expressions instead of `Es`, otherwise we use `Es` unchanged.
+
+__Examples:__
+
+    inline val b = 33
+    def f(): Int = b + 1
+    List(b, 33, 'a')      : List[Int]
+    List(b, 33, 'a', f()) : List[Int]
+    List(1.0f, 'a', 0)    : List[Float]
+    List(1.0f, 1L)        : List[Double]
+    List(1.0f, 1L, f())   : List[AnyVal]
+    List(1.0f, 1234567890): List[AnyVal]
+
+The expression on the second-to-last line has type `List[AnyVal]`,
+since widenings only affect constants. Hence, `1.0f` and `1L` are
+widened to `Double`, but `f()` still has type `Int`. The elements
+don't agree on a type after widening, hence the elements are left
+unchanged.
+
+The expression on the last line has type `List[AnyVal]` because
+`1234567890` cannot be converted to a `Float` without a loss of
+precision.
+
+

docs/sidebar.yml

@@ -95,6 +95,8 @@ sidebar:
               url: docs/reference/dropped/xml.html
             - title: Auto-Application
               url: docs/reference/dropped/auto-apply.html
+            - title: Weak Conformance
+              url: docs/reference/dropped/weak-conformance.html
     - title: Contributing
       subsection:
         - title: Getting Started

tests/pos/harmonize.scala

@@ -3,26 +3,56 @@ object Test {
   def main(args: Array[String]) = {
     val x = true
     val n = 1
+    inline val nn = 2
     val y = if (x) 'A' else n
     val z: Int = y
 
-    val yy = n match {
+    val yy1 = n match {
       case 1 => 'A'
       case 2 => n
       case 3 => 1.0
     }
-    val zz: Double = yy
+    val zz1: AnyVal = yy1 // no widening
+
+    val yy2 = n match {
+      case 1 => 'A'
+      case 2 => nn
+      case 3 => 1.0f
+    }
+    val zz2: Float = yy2 // widening to Float
+
+    val yy3 = n match {
+      case 1 => 'A'
+      case 2 => 3L
+      case 3 => 1.0f
+    }
+    val zz3: Double = yy3 // widening to Double
 
     val a = try {
       'A'
     } catch {
-      case ex: Exception => n
+      case ex: Exception => nn
       case ex: Error => 3L
     }
     val b: Long = a
 
-    val xs = List(1.0, n, 'c')
+    val xs = List(1.0, nn, 'c')
     val ys: List[Double] = xs
+
   }
 
+  inline val b = 33
+  def f(): Int = b + 1
+  val a1 = Array(b, 33, 'a')
+  val b1: Array[Int] = a1
+  val a2 = Array(b, 33, 'a', f())
+  val b2: Array[Int] = a2
+  val a3 = Array(1.0f, 'a', 0)
+  val b3: Array[Float] = a3
+  val a4 = Array(1.0f, 1L)
+  val b4: Array[Double] = a4
+  val a5 = Array(1.0f, 1L, f())
+  val b5: Array[AnyVal] = a5
+  val a6 = Array(1.0f, 1234567890)
+  val b6: Array[AnyVal] = a6
 }

tests/pos/i2833.scala

@@ -0,0 +1,9 @@
+object a {
+  val x: scala.Any =
+    if (true)
+      0L
+    else if (false)
+      (0: Int)
+    else
+      null
+}