Skip to content
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

Deprecate numeric conversions that lose precision (e.g., Long to Double) #8679

Merged
merged 2 commits into from Feb 3, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 14 additions & 10 deletions project/GenerateAnyVals.scala
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
14 changes: 11 additions & 3 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -1136,8 +1136,16 @@ 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) {
val isInharmonic = (
tpSym == IntClass && ptSym == FloatClass ||
tpSym == LongClass && (ptSym == FloatClass || ptSym == DoubleClass)
)
if (isInharmonic)
context.warning(tree.pos, s"Automatic conversion from ${tpSym.name} to ${ptSym.name} is deprecated (since 2.13.1) because it loses precision. Write `.to${ptSym.name}` instead.")
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 +1154,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
2 changes: 1 addition & 1 deletion src/library/scala/Function0.scala
Expand Up @@ -11,7 +11,7 @@
*/

// GENERATED CODE: DO NOT EDIT.
// genprod generated these sources at: 2019-06-18T20:49:11.955Z
// genprod generated these sources at: 2020-02-01T04:13:22.795Z

package scala

Expand Down
3 changes: 2 additions & 1 deletion src/library/scala/Int.scala
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
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
@@ -0,0 +1,21 @@
deprecated_widening.scala:5: 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:7: 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:8: 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:12: 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:14: 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:15: 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
1 error
21 changes: 21 additions & 0 deletions test/files/neg/deprecated_widening.scala
@@ -0,0 +1,21 @@
// scalac: -deprecation -Werror
//
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
}

// don't leak silent warning from float conversion
val n = 42
def clean = n max 27
}
4 changes: 2 additions & 2 deletions test/files/neg/t8450.check
@@ -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
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
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
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
12 changes: 6 additions & 6 deletions test/files/run/number-parsing.scala
Expand Up @@ -8,12 +8,12 @@ object Test {
assert((MinusZero: scala.Float) == (PlusZero: scala.Float))
assert(!(MinusZero equals PlusZero))

List(
-5f.max(2) ,
-5f max 2 ,
-5.max(2) ,
-5 max 2
) foreach (num => assert(num == 2))
assert(List[AnyVal](
-5f.max(2),
-5f max 2,
-5.max(2),
-5 max 2,
).forall(_ == 2))
}

case class Foo(val x: Double) {
Expand Down