Skip to content

Commit

Permalink
Fix for Checked failing in a border case in scala 3
Browse files Browse the repository at this point in the history
Signed-off-by: Carlos Quiroz <[email protected]>
  • Loading branch information
cquiroz committed Oct 1, 2021
1 parent bc17356 commit ac301a4
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 17 deletions.
2 changes: 1 addition & 1 deletion core/src/main/scala/spire/math/Algebraic.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ object Algebraic extends AlgebraicInstances {

@nowarn
// TODO Restore the checked call
def apply(expr: Algebraic.Expr): Bound = {
def apply(expr: Algebraic.Expr): Bound = checked {
// Unfortunately, we must call degreeBound early, to avoid many redundant
// traversals of the Expr tree. Getting this out of the way early on
// means that we will traverse the tree once and populate the degreeBound
Expand Down
22 changes: 11 additions & 11 deletions macros/src/main/scala-3/spire/macros/Checked.scala
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ object Checked:
'{
val z = ${toInt(checkedImpl(x.asExprOf[Any], fallback))}
if (z == ${numLimit}) $fallback else -z
}.asExprOf[A].asTerm
}.asExprOf[A].asTerm.changeOwner(owner)
else if (isLong)
'{
val z = ${toLong(checkedImpl(x.asExprOf[Any], fallback))}
if (z == ${numLimit}) $fallback else -z
}.asTerm
}.asTerm.changeOwner(owner)
else super.transformTerm(tree)(owner)
// NOTE I couldn't find a way to unify the long and int branches. Suggestions are welcome
case Apply(Select(x, "*"), List(y)) =>
Expand All @@ -93,14 +93,14 @@ object Checked:
val yt = ${toInt(checkedImpl(y.asExprOf[Any], fallback))}
val z = xt * yt
if (xt == 0 || (yt == z / xt && !(xt == -1 && yt == $numLimit))) z else $fallback
}.asTerm
}.asTerm.changeOwner(owner)
} else if (isLong) {
'{
val xt = ${toLong(checkedImpl(x.asExprOf[Any], fallback))}
val yt = ${toLong(checkedImpl(y.asExprOf[Any], fallback))}
val z = xt * yt
if (xt == 0 || (yt == z / xt && !(xt == -1 && yt == $numLimit))) z else $fallback
}.asTerm
}.asTerm.changeOwner(owner)
} else
super.transformTerm(tree)(owner)
case Apply(Select(x, "+"), List(y)) =>
Expand All @@ -112,14 +112,14 @@ object Checked:
val yt = ${toInt(checkedImpl(y.asExprOf[Any], fallback))}
val z = xt + yt
if ((~(xt ^ yt) & (xt ^ z)) < 0) $fallback else z
}.asTerm
}.asTerm.changeOwner(owner)
else if (isLong)
'{
val xt = ${toLong(checkedImpl(x.asExprOf[Any], fallback))}
val yt = ${toLong(checkedImpl(y.asExprOf[Any], fallback))}
val z = xt + yt
if ((~(xt ^ yt) & (xt ^ z)) < 0) $fallback else z
}.asTerm
}.asTerm.changeOwner(owner)
else super.transformTerm(tree)(owner)
case Apply(Select(x, "-"), List(y)) =>
val isInt = isIntType(x.asExpr) && isIntType(y.asExpr)
Expand All @@ -130,14 +130,14 @@ object Checked:
val yt = ${toInt(checkedImpl(y.asExprOf[Any], fallback))}
val z = xt - yt
if (((xt ^ yt) & (xt ^ z)) < 0) $fallback else z
}.asTerm
}.asTerm.changeOwner(owner)
else if (isLong)
'{
val xt = ${toLong(checkedImpl(x.asExprOf[Any], fallback))}
val yt = ${toLong(checkedImpl(y.asExprOf[Any], fallback))}
val z = xt - yt
if (((xt ^ yt) & (xt ^ z)) < 0) $fallback else z
}.asTerm
}.asTerm.changeOwner(owner)
else super.transformTerm(tree)(owner)
case Apply(Select(x, "/"), List(y)) =>
val isInt = isIntType(x.asExpr) && isIntType(y.asExpr)
Expand All @@ -148,19 +148,19 @@ object Checked:
val yt = ${toInt(checkedImpl(y.asExprOf[Any], fallback))}
val z = xt / yt
if (yt == -1 && xt == $numLimit) $fallback else z
}.asTerm
}.asTerm.changeOwner(owner)
else if (isLong)
'{
val xt = ${toLong(checkedImpl(x.asExprOf[Any], fallback))}
val yt = ${toLong(checkedImpl(y.asExprOf[Any], fallback))}
val z = xt / yt
if (yt == -1 && xt == $numLimit) $fallback else z
}.asTerm
}.asTerm.changeOwner(owner)
else super.transformTerm(tree)(owner)
case _ =>
super.transformTerm(tree)(owner)

val result = acc.transformTerm(tree)(tree.symbol).asExprOf[A]
val result = acc.transformTerm(tree)(Symbol.spliceOwner).asExprOf[A]
// report.info(result.show)
result

Expand Down
10 changes: 5 additions & 5 deletions macros/src/test/scala/spire/macros/CheckedScalaCheckSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,11 @@ class CheckedScalaCheckSuite extends munit.ScalaCheckSuite {
assertEquals(3L, c5)
val ag = A(Long.MaxValue, Long.MaxValue)
intercept[ArithmeticException] { checked(ag.p * 2L) }
// Border case failing in scala 3
// intercept[ArithmeticException] { checked(List(1L, 2L).map{ k =>
// ag.p * k
// })
// }
// Border case failing in earlier versions of the scala 3 macro
intercept[ArithmeticException] { checked(List(1L, 2L).map{ k =>
ag.p * k
})
}
}

property("Long negate overflow throws arithmetic exception") {
Expand Down

0 comments on commit ac301a4

Please sign in to comment.