Skip to content

Commit

Permalink
Remove range checking during constant folding for constants
Browse files Browse the repository at this point in the history
treated as bit field arguments to shift and bitwise not operators.

Fixes #1852

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=133167719
  • Loading branch information
brad4d authored and blickly committed Sep 15, 2016
1 parent 9d13d6e commit ebba622
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 64 deletions.
1 change: 0 additions & 1 deletion src/com/google/javascript/jscomp/DiagnosticGroups.java
Expand Up @@ -577,7 +577,6 @@ public DiagnosticGroup forName(String name) {
DiagnosticGroups.registerGroup("transitionalSuspiciousCodeWarnings",
PeepholeFoldConstants.INDEX_OUT_OF_BOUNDS_ERROR,
PeepholeFoldConstants.NEGATING_A_NON_NUMBER_ERROR,
PeepholeFoldConstants.BITWISE_OPERAND_OUT_OF_RANGE,
PeepholeFoldConstants.SHIFT_AMOUNT_OUT_OF_BOUNDS,
PeepholeFoldConstants.FRACTIONAL_BITWISE_OPERAND);
}
Expand Down
80 changes: 28 additions & 52 deletions src/com/google/javascript/jscomp/PeepholeFoldConstants.java
Expand Up @@ -46,11 +46,6 @@ class PeepholeFoldConstants extends AbstractPeepholeOptimization {
"JSC_NEGATING_A_NON_NUMBER_ERROR",
"Can''t negate non-numeric value: {0}");

static final DiagnosticType BITWISE_OPERAND_OUT_OF_RANGE =
DiagnosticType.warning(
"JSC_BITWISE_OPERAND_OUT_OF_RANGE",
"Operand out of range, bitwise operation will lose information: {0}");

static final DiagnosticType SHIFT_AMOUNT_OUT_OF_BOUNDS =
DiagnosticType.warning(
"JSC_SHIFT_AMOUNT_OUT_OF_BOUNDS",
Expand Down Expand Up @@ -411,19 +406,14 @@ private Node tryFoldUnaryOperator(Node n) {
case BITNOT:
try {
double val = left.getDouble();
if (val >= Integer.MIN_VALUE && val <= Integer.MAX_VALUE) {
int intVal = (int) val;
if (intVal == val) {
Node notIntValNode = IR.number(~intVal);
parent.replaceChild(n, notIntValNode);
reportCodeChange();
return notIntValNode;
} else {
report(FRACTIONAL_BITWISE_OPERAND, left);
return n;
}
if (Math.floor(val) == val) {
int intVal = jsConvertDoubleToBits(val);
Node notIntValNode = IR.number(~intVal);
parent.replaceChild(n, notIntValNode);
reportCodeChange();
return notIntValNode;
} else {
report(BITWISE_OPERAND_OUT_OF_RANGE, left);
report(FRACTIONAL_BITWISE_OPERAND, left);
return n;
}
} catch (UnsupportedOperationException ex) {
Expand All @@ -437,6 +427,14 @@ private Node tryFoldUnaryOperator(Node n) {
}
}

/**
* Uses a method for treating a double as 32bits that is equivalent to
* how JavaScript would convert a number before applying a bit operation.
*/
private int jsConvertDoubleToBits(double d) {
return (int) (((long) Math.floor(d)) & 0xffffffff);
}

/**
* Try to fold {@code left instanceof right} into {@code true}
* or {@code false}.
Expand Down Expand Up @@ -876,12 +874,6 @@ private Node tryFoldShift(Node n, Node left, Node right) {
double lval = left.getDouble();
double rval = right.getDouble();

// check ranges. We do not do anything that would clip the double to
// a 32-bit range, since the user likely does not intend that.
if (lval < Integer.MIN_VALUE) {
report(BITWISE_OPERAND_OUT_OF_RANGE, left);
return n;
}
// only the lower 5 bits are used when shifting, so don't do anything
// if the shift amount is outside [0,32)
if (!(rval >= 0 && rval < 32)) {
Expand All @@ -895,40 +887,24 @@ private Node tryFoldShift(Node n, Node left, Node right) {
return n;
}

if (Math.floor(lval) != lval) {
report(FRACTIONAL_BITWISE_OPERAND, left);
return n;
}

int bits = jsConvertDoubleToBits(lval);

switch (n.getToken()) {
case LSH:
result = bits << rvalInt;
break;
case RSH:
// Convert the numbers to ints
if (lval > Integer.MAX_VALUE) {
report(BITWISE_OPERAND_OUT_OF_RANGE, left);
return n;
}
int lvalInt = (int) lval;
if (lvalInt != lval) {
report(FRACTIONAL_BITWISE_OPERAND, left);
return n;
}
if (n.getToken() == Token.LSH) {
result = lvalInt << rvalInt;
} else {
result = lvalInt >> rvalInt;
}
result = bits >> rvalInt;
break;
case URSH:
// JavaScript handles zero shifts on signed numbers differently than
// Java as an Java int can not represent the unsigned 32-bit number
// where JavaScript can so use a long here.
long maxUint32 = 0xffffffffL;
if (lval > maxUint32) {
report(BITWISE_OPERAND_OUT_OF_RANGE, left);
return n;
}
long lvalLong = (long) lval;
if (lvalLong != lval) {
report(FRACTIONAL_BITWISE_OPERAND, left);
return n;
}
result = (lvalLong & maxUint32) >>> rvalInt;
// JavaScript always treats the result of >>> as unsigned.
// We must force Java to do the same here.
result = 0xffffffffL & (bits >>> rvalInt);
break;
default:
throw new AssertionError("Unknown shift operator: " + n.getToken());
Expand Down
15 changes: 4 additions & 11 deletions test/com/google/javascript/jscomp/PeepholeFoldConstantsTest.java
Expand Up @@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.javascript.rhino.Node;

import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -478,10 +477,8 @@ public void testUnaryOps() {
fold("a=+-7", "a=-7");
fold("a=+.5", "a=.5");

fold("a=~0x100000000", "a=~0x100000000",
PeepholeFoldConstants.BITWISE_OPERAND_OUT_OF_RANGE);
fold("a=~-0x100000000", "a=~-0x100000000",
PeepholeFoldConstants.BITWISE_OPERAND_OUT_OF_RANGE);
fold("a=~0xffffffff", "a=0");
fold("a=~~0xffffffff", "a=-1");
testSame("a=~.5", PeepholeFoldConstants.FRACTIONAL_BITWISE_OPERAND);
}

Expand Down Expand Up @@ -684,16 +681,12 @@ public void testFoldBitShifts() {
fold("x = -2 >>> 0", "x = 4294967294"); // 0xfffffffe
fold("x = 0x90000000 >>> 28", "x = 9");

testSame("3000000000 << 1",
PeepholeFoldConstants.BITWISE_OPERAND_OUT_OF_RANGE);
fold("x = 0xffffffff << 0", "x = -1");
fold("x = 0xffffffff << 4", "x = -16");
testSame("1 << 32",
PeepholeFoldConstants.SHIFT_AMOUNT_OUT_OF_BOUNDS);
testSame("1 << -1",
PeepholeFoldConstants.SHIFT_AMOUNT_OUT_OF_BOUNDS);
testSame("3000000000 >> 1",
PeepholeFoldConstants.BITWISE_OPERAND_OUT_OF_RANGE);
testSame("0x90000000 >> 28",
PeepholeFoldConstants.BITWISE_OPERAND_OUT_OF_RANGE);
testSame("1 >> 32",
PeepholeFoldConstants.SHIFT_AMOUNT_OUT_OF_BOUNDS);
testSame("1.5 << 0",
Expand Down

0 comments on commit ebba622

Please sign in to comment.