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 4 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
26 changes: 21 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 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 +1444,27 @@ 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
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 {
Expand All @@ -1460,9 +1475,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
78 changes: 78 additions & 0 deletions docs/docs/reference/dropped/weak-conformance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
---
layout: doc-page
title: Dropped: Weak Conformance
---

In some situations, Scala used a {\em weak conformance} relation when
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you can use latex code in our documentation. You should be able to write *weak conformance* instead which should be rendered as weak conformance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. That comes from switching context from writing a paper with LaTex...

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing closing bracket: List[AnyVal]

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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this rule have a name? Is it a new Dotty feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it so say that it is new. It does not have a name (yet).


- If a list of expressions `Es` appears as one of

- the elements of a vararg parameter, or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when it's not a vararg parameter list?

def add[T](a: T, b: T): List[T] = List(a, b)
add(1.0f, 1L) // Double?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, AnyVal.

- 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good if there was some explanation of how the rules differ between val and inline val.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if this is final instead of inline?

inline val s: Short = 33
def f(): Int = b + s
List(b, s, 'a') : List[Int]
List(b, s, 'a', f()): List[Int]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to note here that the type is List[Int] because Int is the LUB, not because of the new rule.

List(1.0f, 'a', 0) : List[Float]
List(1.0f, 1L) : List[Double]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a 1: Int here to highlight the difference between f() and 1: Int?

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.


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
23 changes: 19 additions & 4 deletions tests/pos/harmonize.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down