From 7953479c96524959af7c13914b31c0907173061b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Jul 2017 09:33:53 +0200 Subject: [PATCH 1/7] Make pattern matching typecheck more principled When checking a pattern `p` against a selector `s` we should check that `p == s` is typable. So far we did this in a roundabout way by calling homogenizeType and canCheckEquals diretcly. But that is correct only if there are no user-defined definitions of `==`. --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 77baa1fe3931..3044613bbdbd 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/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 From 28481bfbea1319a01422393d22918253c0dfbb8f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Jul 2017 11:02:56 +0200 Subject: [PATCH 2/7] Harmonize only for constants We now widen a vararg parameter or alternative of an If or Match only to another numeric value type only if the original type is a constant type, and the type can be widened without loss of precision (here we deem it acceptable that Ints are widened to Floats, but Longs can only be widened to Doubles). --- .../dotty/tools/dotc/typer/Applications.scala | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index f4f1e57875cc..a0e78f47c53d 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -445,6 +445,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 test 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)) @@ -1441,9 +1444,18 @@ 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 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 + ts.mapConserve { t => + tpe(t).widenTermRefExpr match { + case ct: ConstantType => adapt(t, lub) + case _ => t + } + } } else ts } @@ -1462,7 +1474,7 @@ 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. */ - 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) } From 45b7f14af88798fdfb7fdb0f5d48d5fa07e03b23 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Jul 2017 11:14:31 +0200 Subject: [PATCH 3/7] Suppress inline checking after erasure After erasure, the rhs of an inline val like inline val n = 3 no longer has a constant type. Therefore, we need to disable the check for it after erasure. Test case (discovered by accident) in harmonize.scala. --- .../src/dotty/tools/dotc/typer/Checking.scala | 3 ++- tests/pos/harmonize.scala | 23 +++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 797a5e6f3e5f..c402f4aafccf 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/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) } } diff --git a/tests/pos/harmonize.scala b/tests/pos/harmonize.scala index 212fb81a8df0..668cdd0343dc 100644 --- a/tests/pos/harmonize.scala +++ b/tests/pos/harmonize.scala @@ -3,25 +3,40 @@ 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 } From 41c442857895ead150dac602f76c9a733b5b2710 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Jul 2017 12:03:56 +0200 Subject: [PATCH 4/7] Add reference section Also, adapt the algorithm to the reference (i.e. convert only if the resulting types all agree). --- .../dotty/tools/dotc/typer/Applications.scala | 12 ++- .../reference/dropped/weak-conformance.md | 78 +++++++++++++++++++ docs/sidebar.yml | 2 + 3 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 docs/docs/reference/dropped/weak-conformance.md diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index a0e78f47c53d..14618d998889 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -445,7 +445,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic => val harmonizedArgs = harmonizeArgs(typedArgs) if (harmonizedArgs ne typedArgs) { ctx.typerState.constraint = origConstraint - // reset constraint, we will test constraint anyway when we + // 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 @@ -1448,20 +1448,23 @@ trait Applications extends Compatibility { self: Typer with Dynamic => 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 => isCompatible(cls, lubTpe))).get - ts.mapConserve { t => + val ts1 = ts.mapConserve { t => tpe(t).widenTermRefExpr match { case ct: ConstantType => 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 { @@ -1472,7 +1475,8 @@ 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. */ private def harmonizeTypes(tpes: List[Type])(implicit ctx: Context): List[Type] = harmonizeWith(tpes)(identity, (tp, pt) => pt) diff --git a/docs/docs/reference/dropped/weak-conformance.md b/docs/docs/reference/dropped/weak-conformance.md new file mode 100644 index 000000000000..ecf382aba56c --- /dev/null +++ b/docs/docs/reference/dropped/weak-conformance.md @@ -0,0 +1,78 @@ +--- +layout: doc-page +title: Dropped: Weak Conformance +--- + +In some situations, Scala used a {\em 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 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 above the types of all expressions in `Es`. Here + _above_ and _least_ are interpreted according to the ordering given + below. + + + Double + / \ + Long Float + \ / + Int + / \ + Short Char + | + Byte + + If these widenings lead to all expressions `Es` having the same type, + we use the transformed list of expressions instead of `Es`, otherwise + we use `Es` unchanged. + +__Examples:__ + + inline val b: Byte = 3 + inline val s: Short = 33 + def f(): Int = b + s + List(b, s, 'a') : List[Int] + List(b, s, 'a', f()): List[Int] + List(1.0f, 'a', 0) : List[Float] + List(1.0f, 1L) : List[Double] + List(1.0f, 1L, f()) : List[AnyVal] + +The expression on the 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 expressions are left unchanged. + + diff --git a/docs/sidebar.yml b/docs/sidebar.yml index 8e62d484b0ec..35a807c25705 100644 --- a/docs/sidebar.yml +++ b/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 From af81f87071e076e39a57b840ef015e9b3a6555c3 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Jul 2017 12:07:07 +0200 Subject: [PATCH 5/7] Add original test case --- tests/pos/i2833.scala | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/pos/i2833.scala diff --git a/tests/pos/i2833.scala b/tests/pos/i2833.scala new file mode 100644 index 000000000000..59cc405e627b --- /dev/null +++ b/tests/pos/i2833.scala @@ -0,0 +1,9 @@ +object a { + val x: scala.Any = + if (true) + 0L + else if (false) + (0: Int) + else + null +} From 6d208c5d285df8b0b43cdbe51cbcd08367907501 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Jul 2017 14:27:16 +0200 Subject: [PATCH 6/7] Don't widen constants if that causes a loss of precision --- .../dotty/tools/dotc/typer/Applications.scala | 11 ++++- .../reference/dropped/weak-conformance.md | 43 +++++++++++-------- tests/pos/harmonize.scala | 15 +++++++ 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 14618d998889..65378e1cecd7 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/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._ @@ -1449,11 +1450,19 @@ trait Applications extends Compatibility { self: Typer with Dynamic => !(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 => 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 => adapt(t, lub) + case ct: ConstantType if !lossOfPrecision(ct.value) => adapt(t, lub) case _ => t } } diff --git a/docs/docs/reference/dropped/weak-conformance.md b/docs/docs/reference/dropped/weak-conformance.md index ecf382aba56c..4b97f34bdb82 100644 --- a/docs/docs/reference/dropped/weak-conformance.md +++ b/docs/docs/reference/dropped/weak-conformance.md @@ -40,7 +40,8 @@ assigning a type to a constant expression. The rule is: 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 above the types of all expressions in `Es`. Here + 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. @@ -55,24 +56,32 @@ assigning a type to a constant expression. The rule is: | Byte - If these widenings lead to all expressions `Es` having the same type, - we use the transformed list of expressions instead of `Es`, otherwise - we use `Es` unchanged. + 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: Byte = 3 - inline val s: Short = 33 - def f(): Int = b + s - List(b, s, 'a') : List[Int] - List(b, s, 'a', f()): List[Int] - List(1.0f, 'a', 0) : List[Float] - List(1.0f, 1L) : List[Double] - List(1.0f, 1L, f()) : List[AnyVal] - -The expression on the 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 expressions are left unchanged. + 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. diff --git a/tests/pos/harmonize.scala b/tests/pos/harmonize.scala index 668cdd0343dc..ecf6aa132fb7 100644 --- a/tests/pos/harmonize.scala +++ b/tests/pos/harmonize.scala @@ -38,6 +38,21 @@ object Test { 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 } From 196769f8bce124832eeaa2085ae5b10bfa8f5aba Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 10 Jul 2017 14:48:31 +0200 Subject: [PATCH 7/7] Fix docs. --- docs/docs/reference/dropped/weak-conformance.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/docs/reference/dropped/weak-conformance.md b/docs/docs/reference/dropped/weak-conformance.md index 4b97f34bdb82..8b592c1af76b 100644 --- a/docs/docs/reference/dropped/weak-conformance.md +++ b/docs/docs/reference/dropped/weak-conformance.md @@ -3,7 +3,7 @@ layout: doc-page title: Dropped: Weak Conformance --- -In some situations, Scala used a {\em weak conformance} relation when +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]`: @@ -24,12 +24,12 @@ A less obvious example is the following one, which was also typed as a 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 +`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 rule is: +assigning a type to a constant expression. The new rule is: - If a list of expressions `Es` appears as one of