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

Lint for integral divisions that are widened to a float [forward port from 2.12] #10369

Merged
merged 2 commits into from Apr 11, 2023
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
1 change: 1 addition & 0 deletions src/compiler/scala/tools/nsc/Reporting.scala
Expand Up @@ -408,6 +408,7 @@ object Reporting {
object LintUnitSpecialization extends Lint; add(LintUnitSpecialization)
object LintMultiargInfix extends Lint; add(LintMultiargInfix)
object LintPerformance extends Lint; add(LintPerformance)
object LintIntDivToFloat extends Lint; add(LintIntDivToFloat)

sealed trait Feature extends WarningCategory { override def summaryCategory: WarningCategory = Feature }
object Feature extends Feature { override def includes(o: WarningCategory): Boolean = o.isInstanceOf[Feature] }; add(Feature)
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/scala/tools/nsc/settings/Warnings.scala
Expand Up @@ -214,6 +214,7 @@ trait Warnings {
val ImplicitRecursion = LintWarning("implicit-recursion", "Implicit resolves to an enclosing definition.")
val UniversalMethods = LintWarning("universal-methods", "Require arg to is/asInstanceOf.")
val ArgDiscard = LintWarning("arg-discard", "-Wvalue-discard for adapted arguments.")
val IntDivToFloat = LintWarning("int-div-to-float", "Warn when an integer division is converted (widened) to floating point: `(someInt / 2): Double`.")

def allLintWarnings = values.toSeq.asInstanceOf[Seq[LintWarning]]
}
Expand Down Expand Up @@ -248,6 +249,7 @@ trait Warnings {
def lintImplicitRecursion = lint.contains(ImplicitRecursion) || (warnSelfImplicit.value: @nowarn("cat=deprecation"))
def lintUniversalMethods = lint.contains(UniversalMethods)
def lintArgDiscard = lint.contains(ArgDiscard)
def lintIntDivToFloat = lint.contains(IntDivToFloat)

// The Xlint warning group.
val lint = MultiChoiceSetting(
Expand Down
35 changes: 24 additions & 11 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -837,7 +837,6 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
// refuses to re-attempt typechecking, and presumes that someone
// else was responsible for issuing the related type error!
fun.setSymbol(NoSymbol)
case _ =>
}
debuglog(s"fallback on implicits: $tree/$resetTree")
// scala/bug#10066 Need to patch the enclosing tree in the context to make translation of Dynamic
Expand Down Expand Up @@ -1124,17 +1123,31 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
@inline def warnValueDiscard(): Unit =
if (!isPastTyper && settings.warnValueDiscard.value && !treeInfo.isThisTypeResult(tree) && !treeInfo.hasExplicitUnit(tree))
context.warning(tree.pos, s"discarded non-Unit value of type ${tree.tpe}", WarningCategory.WFlagValueDiscard)
@inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit =
if (!isPastTyper) {
val isInharmonic = (
(tpSym == IntClass && ptSym == FloatClass)
|| (tpSym == LongClass && (ptSym == FloatClass || ptSym == DoubleClass))
)
if (isInharmonic)
// not `context.deprecationWarning` because they are not buffered in silent mode
context.warning(tree.pos, s"Widening conversion from ${tpSym.name} to ${ptSym.name} is deprecated because it loses precision. Write `.to${ptSym.name}` instead.", WarningCategory.Deprecation)
else if (settings.warnNumericWiden.value) context.warning(tree.pos, "implicit numeric widening", WarningCategory.WFlagNumericWiden)
@inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit = if (!isPastTyper) {
val targetIsWide = ptSym == FloatClass || ptSym == DoubleClass
val isInharmonic = {
def intWidened = tpSym == IntClass && ptSym == FloatClass
def longWidened = tpSym == LongClass && targetIsWide
intWidened || longWidened
}
if (isInharmonic)
// not `context.deprecationWarning` because they are not buffered in silent mode
context.warning(tree.pos, s"Widening conversion from ${tpSym.name} to ${ptSym.name} is deprecated because it loses precision. Write `.to${ptSym.name}` instead.", WarningCategory.Deprecation)
else {
object warnIntDiv extends Traverser {
def isInt(t: Tree) = ScalaIntegralValueClasses(t.tpe.typeSymbol)
override def traverse(tree: Tree): Unit = tree match {
case Apply(Select(q, nme.DIV), _) if isInt(q) =>
context.warning(tree.pos, s"integral division is implicitly converted (widened) to floating point. Add an explicit `.to${ptSym.name}`.", WarningCategory.LintIntDivToFloat)
case Apply(Select(a1, _), List(a2)) if isInt(tree) && isInt(a1) && isInt(a2) => traverse(a1); traverse(a2)
case Select(q, _) if isInt(tree) && isInt(q) => traverse(q)
case _ =>
}
}
if (targetIsWide && settings.lintIntDivToFloat) warnIntDiv(tree)
if (settings.warnNumericWiden.value) context.warning(tree.pos, "implicit numeric widening", WarningCategory.WFlagNumericWiden)
}
}

// The <: Any requirement inhibits attempts to adapt continuation types to non-continuation types.
val anyTyped = tree.tpe <:< AnyTpe
Expand Down
1 change: 1 addition & 0 deletions src/library/scala/math/BigInt.scala
Expand Up @@ -278,6 +278,7 @@ final class BigInt private (private var _bigInteger: BigInteger, private val _lo
(shifted.signum != 0) && !(shifted equals BigInt.minusOne)
}

@deprecated("isWhole on an integer type is always true", "2.12.15")
def isWhole: Boolean = true
def underlying: BigInteger = bigInteger

Expand Down
1 change: 1 addition & 0 deletions src/library/scala/runtime/RichInt.scala
Expand Up @@ -31,6 +31,7 @@ final class RichInt(val self: Int) extends AnyVal with ScalaNumberProxy[Int] wit
/** Returns `'''true'''` if this number has no decimal component.
* Always `'''true'''` for `RichInt`.
*/
@deprecated("isWhole on an integer type is always true", "2.12.15")
def isWhole = true

override def isValidInt = true
Expand Down
1 change: 1 addition & 0 deletions src/library/scala/runtime/ScalaNumberProxy.scala
Expand Up @@ -50,6 +50,7 @@ trait ScalaNumberProxy[T] extends Any with ScalaNumericAnyConversions with Typed
@deprecated("use `sign` method instead", since = "2.13.0") def signum: Int = num.signum(self)
}
trait ScalaWholeNumberProxy[T] extends Any with ScalaNumberProxy[T] {
@deprecated("isWhole on an integer type is always true", "2.12.15")
def isWhole = true
}
trait IntegralProxy[T] extends Any with ScalaWholeNumberProxy[T] with RangedProxy[T] {
Expand Down
2 changes: 2 additions & 0 deletions src/reflect/scala/reflect/internal/Definitions.scala
Expand Up @@ -182,6 +182,8 @@ trait Definitions extends api.StandardDefinitions {
}
def ScalaPrimitiveValueClasses: List[ClassSymbol] = ScalaValueClasses

lazy val ScalaIntegralValueClasses: Set[Symbol] = Set(CharClass, ByteClass, ShortClass, IntClass, LongClass)

def underlyingOfValueClass(clazz: Symbol): Type =
clazz.derivedValueClassUnbox.tpe.resultType

Expand Down
1 change: 1 addition & 0 deletions src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
Expand Up @@ -530,6 +530,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse =>
definitions.ScalaValueClasses
definitions.ScalaValueClassesSet
definitions.ScalaNumericValueClassesSet
definitions.ScalaIntegralValueClasses

uncurry.VarargsSymbolAttachment
uncurry.DesugaredParameterType
Expand Down
18 changes: 18 additions & 0 deletions test/files/neg/lint-int-div-to-float.check
@@ -0,0 +1,18 @@
lint-int-div-to-float.scala:6: warning: integral division is implicitly converted (widened) to floating point. Add an explicit `.toDouble`.
def w1: Double = f / 2
^
lint-int-div-to-float.scala:7: warning: integral division is implicitly converted (widened) to floating point. Add an explicit `.toDouble`.
def w2: Double = (f / 2) * 3
^
lint-int-div-to-float.scala:8: warning: integral division is implicitly converted (widened) to floating point. Add an explicit `.toDouble`.
def w3: Double = -(f / 2)
^
lint-int-div-to-float.scala:9: warning: integral division is implicitly converted (widened) to floating point. Add an explicit `.toDouble`.
def w4: Double = (new C).f / (new C).f * 3
^
lint-int-div-to-float.scala:10: warning: integral division is implicitly converted (widened) to floating point. Add an explicit `.toDouble`.
def w5: Double = f - f.abs / 2
^
error: No warnings can be incurred under -Werror.
5 warnings
1 error
18 changes: 18 additions & 0 deletions test/files/neg/lint-int-div-to-float.scala
@@ -0,0 +1,18 @@
// scalac: -Xlint -Xfatal-warnings

class C {
def f = 1

def w1: Double = f / 2
def w2: Double = (f / 2) * 3
def w3: Double = -(f / 2)
def w4: Double = (new C).f / (new C).f * 3
def w5: Double = f - f.abs / 2

def o1: Double = (f / 2).toDouble
def o2: Double = f.toDouble / 2
def o3: Double = f / 2.toDouble
def o4: Double = f / 2d
def o5: Double = (new C).f.toDouble / (new C).f * 3
def o6: Long = f / 2 + 3 // only warn if widening to a floating point, not when widening int to long
}
11 changes: 5 additions & 6 deletions test/files/run/is-valid-num.scala
@@ -1,6 +1,5 @@
/*
* filter: inliner warnings; re-run with
*/
// scalac: -Xlint -Werror
@annotation.nowarn("cat=deprecation&msg=isWhole")
object Test {
def x = BigInt("10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")
def y = BigDecimal("" + (Short.MaxValue + 1) + ".0")
Expand All @@ -11,7 +10,7 @@ object Test {
def l2 = Int.MinValue.toLong - 1

def main(args: Array[String]): Unit = {
// assert(x.isWhole, x)
assert(x.isWhole, x)
assert(!x.isValidDouble, x)
assert(!x.isValidFloat, x)
assert(!x.isValidLong, x)
Expand Down Expand Up @@ -171,7 +170,7 @@ object Test {

if (!d.isInfinity) {
val bd = BigDecimal(new java.math.BigDecimal(d))
// assert(!bd.isWhole, bd)
assert(!bd.isWhole, bd)
assert(bd.isExactDouble, bd)
assert(bd.isExactFloat == isFloat, bd)
assert(!bd.isValidLong, bd)
Expand Down Expand Up @@ -221,7 +220,7 @@ object Test {
assert(bd.isValidShort == isShort, bd)
assert(bd.isValidByte == isByte, bd)

// assert(bi.isWhole, bi)
assert(bi.isWhole, bi)
assert(bi.isValidDouble == isDouble, bi)
assert(bi.isValidFloat == isFloat, bi)
assert(bi.isValidLong == isLong, bi)
Expand Down
3 changes: 2 additions & 1 deletion test/scalacheck/scala/math/BigIntProperties.scala
Expand Up @@ -5,6 +5,7 @@ import Arbitrary.arbitrary
import org.scalacheck.Prop._
import java.math.BigInteger
import java.lang.Character
import scala.annotation.nowarn
import scala.util.Random

object BigIntProperties extends Properties("BigInt") {
Expand Down Expand Up @@ -95,7 +96,7 @@ object BigIntProperties extends Properties("BigInt") {
property("isValidLong") = forAll { (l: Long) => BigInt(l).isValidLong }
property("isValidLong") = !BigInt("9223372036854775808").isValidLong
property("isValidLong") = !BigInt("-9223372036854775809").isValidLong
property("isWhole") = forAll { (bi: BigInt) => bi.isWhole }
property("isWhole") = forAll { (bi: BigInt) => bi.isWhole: @nowarn }
property("underlying") = forAll(bigInteger) { bi => BigInt(bi).underlying ?= bi }
property("equals") = forAll(bigInteger, bigInteger) { (x, y) => (x == y) ?= (BigInt(x) equals BigInt(y)) }
property("compare") = forAll(bigInteger, bigInteger) { (x, y) => x.compareTo(y) ?= BigInt(x).compare(y) }
Expand Down