Skip to content

Commit ec95e53

Browse files
committed
SI-9422 Fix incorrect constant propagation
The ConstantOptimization phase uses abstract interpretation to track what is known about values, and then to use this information to optimize away tests with a statically known result. Constant propagation was added under -optimize in Scala 2.11.0-M3, in PR scala#2214. For example, we might know that a variable must hold one of a set of values (`Possible`). Or, we could track that it must *not* be of of a set of value (`Impossible`). The test case in the bug report was enough to create comparison: v1 == v2 // where V1 = Possible(Set(true, false)) // V2 = Possible(Set(true, false)) This test was considered to be always true, due to a bug in `Possible#mightNotEqual`. The only time we can be sure that `Possible(p1) mightNotEquals Possible(p2)` is if `p1` and `p2` are the same singleton set. This commit changes this method to implement this logic. The starting assumption for all values is currently `Impossible(Set())`, although it would also be reasonable to represent an unknown boolean variable as `Possible(Set(true, false))`, given the finite and small domain. I tried to change the starting assumption for boolean locals in exactly this manner, and that brings the bug into sharp focus. Under this patch: retronym@e564fe522d This code: def test(a: Boolean, b: Boolean) = a == b Compiles to: public boolean test(boolean, boolean); Code: 0: iconst_1 1: ireturn Note: the enclosed test case does not list `-optimize` in a `.flags` file, I'm relying on that being passed in by the validation build. I've tested locally with that option, though.
1 parent 0e9525a commit ec95e53

File tree

2 files changed

+16
-3
lines changed

2 files changed

+16
-3
lines changed

src/compiler/scala/tools/nsc/backend/opt/ConstantOptimization.scala

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,11 @@ abstract class ConstantOptimization extends SubComponent {
170170
// out all the possibilities
171171
case Impossible(possible2) => (possible -- possible2).nonEmpty
172172
})
173-
def mightNotEqual(other: Contents): Boolean = (this ne other) && (other match {
174-
// two Possibles might not be equal if either has possible members that the other doesn't
175-
case Possible(possible2) => (possible -- possible2).nonEmpty || (possible2 -- possible).nonEmpty
173+
def mightNotEqual(other: Contents): Boolean = (other match {
174+
case Possible(possible2) =>
175+
// two Possibles must equal if each is known to be of the same, single value
176+
val mustEqual = possible.size == 1 && possible == possible2
177+
!mustEqual
176178
case Impossible(_) => true
177179
})
178180
}

test/files/run/t9422.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
class Test(val x: Long) {
2+
def sameDirection(y: Long): Boolean =
3+
(y == 0 || x == 0 || ((y > 0) == (x > 0)))
4+
}
5+
6+
object Test {
7+
def main(args: Array[String]) {
8+
val b = new Test(1L)
9+
assert(!b.sameDirection(-1L))
10+
}
11+
}

0 commit comments

Comments
 (0)