Skip to content

Commit

Permalink
Deprecate numeric conversions that lose precision
Browse files Browse the repository at this point in the history
Int to Float, Long to Float and Long to Double are dangerous conversions
and should never be done automatically.
  • Loading branch information
smarter authored and som-snytt committed Feb 1, 2020
1 parent 46a8156 commit c8ba3c4
Show file tree
Hide file tree
Showing 11 changed files with 75 additions and 24 deletions.
24 changes: 14 additions & 10 deletions project/GenerateAnyVals.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,26 @@ trait GenerateAnyValReps {

case class Op(op : String, doc : String)

private def companionCoercions(tos: AnyValRep*) = {
tos.toList map (to =>
s"implicit def @javaequiv@2${to.javaEquiv}(x: @name@): ${to.name} = x.to${to.name}"
)
}
private def companionCoercions(deprecated: Boolean, tos: AnyValRep*): List[String] =
tos.toList.flatMap { to =>
val code = s"implicit def @javaequiv@2${to.javaEquiv}(x: @name@): ${to.name} = x.to${to.name}"
if (deprecated)
List(s"""@deprecated("Implicit conversion from @name@ to ${to.name} is dangerous because it loses precision. Write `.to${to.name}` instead.", "2.13.1")""", code)
else
List(code)
}

def coercionComment =
"""/** Language mandated coercions from @name@ to "wider" types. */
import scala.language.implicitConversions"""

def implicitCoercions: List[String] = {
val coercions = this match {
case B => companionCoercions(S, I, L, F, D)
case S | C => companionCoercions(I, L, F, D)
case I => companionCoercions(L, F, D)
case L => companionCoercions(F, D)
case F => companionCoercions(D)
case B => companionCoercions(deprecated = false, S, I, L, F, D)
case S | C => companionCoercions(deprecated = false, I, L, F, D)
case I => companionCoercions(deprecated = true, F) ++ companionCoercions(deprecated = false, L, D)
case L => companionCoercions(deprecated = true, F, D)
case F => companionCoercions(deprecated = false, D)
case _ => Nil
}
if (coercions.isEmpty) Nil
Expand Down
13 changes: 10 additions & 3 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1136,8 +1136,15 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
}
if (!isThisTypeResult && !explicitlyUnit(tree)) context.warning(tree.pos, "discarded non-Unit value")
}
@inline def warnNumericWiden(): Unit =
if (!isPastTyper && settings.warnNumericWiden) context.warning(tree.pos, "implicit numeric widening")
@inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit =
if (!isPastTyper) {
if (tpSym == IntClass && ptSym == FloatClass ||
tpSym == LongClass && (ptSym == FloatClass || ptSym == DoubleClass))
context.deprecationWarning(tree.pos, NoSymbol,
s"Automatic conversion from ${tpSym.name} to ${ptSym.name} is deprecated (since 2.13.1) because it loses precision. " +
s"Write `.to${ptSym.name}` instead.".stripMargin, "2.13.1")
else if (settings.warnNumericWiden) context.warning(tree.pos, "implicit numeric widening")
}

// The <: Any requirement inhibits attempts to adapt continuation types to non-continuation types.
val anyTyped = tree.tpe <:< AnyTpe
Expand All @@ -1146,7 +1153,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
case TypeRef(_, UnitClass, _) if anyTyped => // (12)
warnValueDiscard() ; tpdPos(gen.mkUnitBlock(tree))
case TypeRef(_, numValueCls, _) if anyTyped && isNumericValueClass(numValueCls) && isNumericSubType(tree.tpe, pt) => // (10) (11)
warnNumericWiden() ; tpdPos(Select(tree, s"to${numValueCls.name}"))
warnNumericWiden(tree.tpe.widen.typeSymbol, numValueCls) ; tpdPos(Select(tree, s"to${numValueCls.name}"))
case dealiased if dealiased.annotations.nonEmpty && canAdaptAnnotations(tree, this, mode, pt) => // (13)
tpd(adaptAnnotations(tree, this, mode, pt))
case _ =>
Expand Down
3 changes: 2 additions & 1 deletion src/library/scala/Int.scala
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,9 @@ object Int extends AnyValCompanion {
override def toString = "object scala.Int"
/** Language mandated coercions from Int to "wider" types. */
import scala.language.implicitConversions
implicit def int2long(x: Int): Long = x.toLong
@deprecated("Implicit conversion from Int to Float is dangerous because it loses precision. Write `.toFloat` instead.", "2.13.1")
implicit def int2float(x: Int): Float = x.toFloat
implicit def int2long(x: Int): Long = x.toLong
implicit def int2double(x: Int): Double = x.toDouble
}

2 changes: 2 additions & 0 deletions src/library/scala/Long.scala
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,9 @@ object Long extends AnyValCompanion {
override def toString = "object scala.Long"
/** Language mandated coercions from Long to "wider" types. */
import scala.language.implicitConversions
@deprecated("Implicit conversion from Long to Float is dangerous because it loses precision. Write `.toFloat` instead.", "2.13.1")
implicit def long2float(x: Long): Float = x.toFloat
@deprecated("Implicit conversion from Long to Double is dangerous because it loses precision. Write `.toDouble` instead.", "2.13.1")
implicit def long2double(x: Long): Double = x.toDouble
}

21 changes: 21 additions & 0 deletions test/files/neg/deprecated_widening.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
deprecated_widening.scala:3: warning: Automatic conversion from Int to Float is deprecated (since 2.13.1) because it loses precision. Write `.toFloat` instead.
val i_f: Float = i // deprecated
^
deprecated_widening.scala:5: warning: Automatic conversion from Long to Float is deprecated (since 2.13.1) because it loses precision. Write `.toFloat` instead.
val l_f: Float = l // deprecated
^
deprecated_widening.scala:6: warning: Automatic conversion from Long to Double is deprecated (since 2.13.1) because it loses precision. Write `.toDouble` instead.
val l_d: Double = l // deprecated
^
deprecated_widening.scala:10: warning: method int2float in object Int is deprecated (since 2.13.1): Implicit conversion from Int to Float is dangerous because it loses precision. Write `.toFloat` instead.
implicitly[Int => Float] // deprecated
^
deprecated_widening.scala:12: warning: method long2float in object Long is deprecated (since 2.13.1): Implicit conversion from Long to Float is dangerous because it loses precision. Write `.toFloat` instead.
implicitly[Long => Float] // deprecated
^
deprecated_widening.scala:13: warning: method long2double in object Long is deprecated (since 2.13.1): Implicit conversion from Long to Double is dangerous because it loses precision. Write `.toDouble` instead.
implicitly[Long => Double] // deprecated
^
error: No warnings can be incurred under -Werror.
6 warnings found
one error found
1 change: 1 addition & 0 deletions test/files/neg/deprecated_widening.flags
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-deprecation -Xfatal-warnings
15 changes: 15 additions & 0 deletions test/files/neg/deprecated_widening.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
object Test {
def foo(i: Int, l: Long): Unit = {
val i_f: Float = i // deprecated
val i_d: Double = i // OK
val l_f: Float = l // deprecated
val l_d: Double = l // deprecated
}

def imp: Unit = {
implicitly[Int => Float] // deprecated
implicitly[Int => Double] // OK
implicitly[Long => Float] // deprecated
implicitly[Long => Double] // deprecated
}
}
4 changes: 2 additions & 2 deletions test/files/neg/t8450.check
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
t8450.scala:7: warning: implicit numeric widening
def elapsed: Foo = (System.nanoTime - 100L).foo
^
def elapsed: Foo = (System.nanoTime.toInt - 100).foo
^
error: No warnings can be incurred under -Werror.
1 warning
1 error
6 changes: 3 additions & 3 deletions test/files/neg/t8450.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ trait Foo

class WarnWidening {
implicit class FooDouble(d: Double) { def foo = new Foo {} }
def elapsed: Foo = (System.nanoTime - 100L).foo
def elapsed: Foo = (System.nanoTime.toInt - 100).foo
}

class NoWarnWidening {
implicit class FooLong(l: Long) { def foo = new Foo {} }
implicit class FooInt(i: Int) { def foo = new Foo {} }
implicit class FooDouble(d: Double) { def foo = new Foo {} }
def elapsed: Foo = (System.nanoTime - 100L).foo
def elapsed: Foo = (System.nanoTime.toInt - 100).foo
}
4 changes: 2 additions & 2 deletions test/files/run/arrays.scala
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ object Test {
def fcheck(xs: Array[Float ]): Unit = {
check(xs.length == 3, xs.length, 3);
check(xs(0) == f0, xs(0), f0);
check(xs(1) == f1, xs(1), f1: Float); // !!! : Float
check(xs(1) == f1, xs(1), f1.toFloat); // !!! : Float
check(xs(2) == f2, xs(2), f2);
}

Expand Down Expand Up @@ -363,7 +363,7 @@ object Test {
val carray: Array[Char ] = Array(c0, c1, c2);
val iarray: Array[Int ] = Array(i0, i1, i2);
val larray: Array[Long ] = Array(l0, l1, l2);
val farray: Array[Float ] = Array(f0, f1, f2);
val farray: Array[Float ] = Array(f0, f1.toFloat, f2);
val darray: Array[Double ] = Array(d0, d1, d2);
val rarray: Array[AnyRef ] = Array(r0, r1, r2, r4, r4, r5);
val oarray: Array[Object ] = Array(o0, o1, o2, o4, o4, o5);
Expand Down
6 changes: 3 additions & 3 deletions test/files/run/hashCodeStatics.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ object Test

def allSame[T](xs: List[T]) = assert(xs.distinct.size == 1, "failed: " + xs)

def mkNumbers(x: Int): List[Number] =
List(x.toByte, x.toShort, x, x.toLong, x.toFloat, x.toDouble)
def mkNumbers(x: Int): List[Any] =
List[Any](x.toByte, x.toShort, x, x.toLong, x.toFloat, x.toDouble)

def testLDF(x: Long) = allSame(List[Number](x, x.toDouble, x.toFloat) map anyHash)
def testLDF(x: Long) = allSame(List[Any](x, x.toDouble, x.toFloat) map anyHash)

def main(args: Array[String]): Unit = {
List(Byte.MinValue, -1, 0, 1, Byte.MaxValue) foreach { n =>
Expand Down

0 comments on commit c8ba3c4

Please sign in to comment.