diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2015-07-29 13:29:57 +1000 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2015-07-29 14:13:22 +1000 |
commit | ec95e534a213a6ea760aa31c507d122ce449890a (patch) | |
tree | 26683be8f4c387085d7ce7a2ddf8586ee609087c /src/compiler | |
parent | 0e9525aa618a2eca143a1c7379ff1e6efd23b86e (diff) | |
download | scala-ec95e534a213a6ea760aa31c507d122ce449890a.tar.gz scala-ec95e534a213a6ea760aa31c507d122ce449890a.tar.bz2 scala-ec95e534a213a6ea760aa31c507d122ce449890a.zip |
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 #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:
https://github.com/retronym/scala/commit/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.
Diffstat (limited to 'src/compiler')
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/opt/ConstantOptimization.scala | 8 |
1 files changed, 5 insertions, 3 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/opt/ConstantOptimization.scala b/src/compiler/scala/tools/nsc/backend/opt/ConstantOptimization.scala index 0e6ee76eb2..fb1799e092 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/ConstantOptimization.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/ConstantOptimization.scala @@ -170,9 +170,11 @@ abstract class ConstantOptimization extends SubComponent { // out all the possibilities case Impossible(possible2) => (possible -- possible2).nonEmpty }) - def mightNotEqual(other: Contents): Boolean = (this ne other) && (other match { - // two Possibles might not be equal if either has possible members that the other doesn't - case Possible(possible2) => (possible -- possible2).nonEmpty || (possible2 -- possible).nonEmpty + def mightNotEqual(other: Contents): Boolean = (other match { + case Possible(possible2) => + // two Possibles must equal if each is known to be of the same, single value + val mustEqual = possible.size == 1 && possible == possible2 + !mustEqual case Impossible(_) => true }) } |