Skip to content

Fix #2833: Fixes to harmonize #2843

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

Merged
merged 7 commits into from
Jul 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -445,6 +446,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))
Expand Down Expand Up @@ -1441,15 +1445,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 {
Expand All @@ -1460,9 +1484,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)
}

3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
7 changes: 5 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
87 changes: 87 additions & 0 deletions docs/docs/reference/dropped/weak-conformance.md
Original file line number Diff line number Diff line change
@@ -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.


2 changes: 2 additions & 0 deletions docs/sidebar.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 34 additions & 4 deletions tests/pos/harmonize.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
9 changes: 9 additions & 0 deletions tests/pos/i2833.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
object a {
val x: scala.Any =
if (true)
0L
else if (false)
(0: Int)
else
null
}