Skip to content

Commit

Permalink
Allow bit-twiddling with constants of larger integral types if all th…
Browse files Browse the repository at this point in the history
…e bits fit.

Example:
```
byte b = ...;
b |= 1;
```

Context: google/auto#1384
PiperOrigin-RevId: 481963331
  • Loading branch information
eamonnmcmanus authored and Error Prone Team committed Oct 18, 2022
1 parent 081f67e commit 6d82c80
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.errorprone.fixes.Fix;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.OperatorPrecedence;
import com.sun.source.tree.CompoundAssignmentTree;
import com.sun.source.tree.ConditionalExpressionTree;
Expand Down Expand Up @@ -149,6 +150,13 @@ private static Optional<Fix> rewriteCompoundAssignment(
case RIGHT_SHIFT_ASSIGNMENT:
// narrowing the result of a signed right shift does not lose information
return Optional.absent();
case AND_ASSIGNMENT:
case XOR_ASSIGNMENT:
case OR_ASSIGNMENT:
if (twiddlingConstantBitsOk(tree)) {
return Optional.absent();
}
break;
default:
break;
}
Expand All @@ -175,4 +183,25 @@ private static Optional<Fix> rewriteCompoundAssignment(
String replacement = String.format("%s = (%s) (%s %s %s)", var, castType, var, op, expr);
return Optional.of(SuggestedFix.replace(tree, replacement));
}

private static boolean twiddlingConstantBitsOk(CompoundAssignmentTree tree) {
int shift;
switch (ASTHelpers.getType(tree.getVariable()).getKind()) {
case BYTE:
shift = 8;
break;
case SHORT:
shift = 16;
break;
default:
return false;
}
Object constValue = ASTHelpers.constValue(tree.getExpression());
if (!(constValue instanceof Integer || constValue instanceof Long)) {
return false;
}
long constLong = ((Number) constValue).longValue();
long shifted = constLong >> shift;
return shifted == 0 || shifted == ~0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,43 @@ public void testBitTwiddle() {
.doTest();
}

// bit twiddling deficient types with constant masks of the same width is fine
@Test
public void testBitTwiddleConstant() {
compilationHelper
.addSourceLines(
"Test.java",
"class Test {",
" void m() {",
" short s = 0;",
" byte b = 0;",
"",
" s &= ~1;",
" s |= 1;",
" s ^= 1;",
"",
" b &= ~1;",
" b |= 1;",
" b ^= 1;",
"",
" b |= 128;",
" b &= 128;",
" b ^= 128;",
" b |= 1L;",
" b &= 1L;",
" b ^= 1L;",
"",
" // BUG: Diagnostic contains: b = (byte) (b | 256)",
" b |= 256;",
" // BUG: Diagnostic contains: b = (byte) (b & ~256)",
" b &= ~256;",
" // BUG: Diagnostic contains: b = (byte) (b ^ 256)",
" b ^= 256;",
" }",
"}")
.doTest();
}

@Test
public void allowsBinopsOfDeficientTypes() {
compilationHelper
Expand Down Expand Up @@ -366,13 +403,13 @@ public void testPreservePrecedenceExhaustive_orOr() throws Exception {
}

private void testPrecedence(String opA, String opB, boolean parens) {
String rhs = String.format("1 %s 2", opB);
String rhs = String.format("100_000 %s 200_000", opB);
if (parens) {
rhs = "(" + rhs + ")";
}
String expect = String.format("s = (short) (s %s %s", opA, rhs);

String compoundAssignment = String.format(" s %s= 1 %s 2;", opA, opB);
String compoundAssignment = String.format(" s %s= 100_000 %s 200_000;", opA, opB);

compilationHelper
.addSourceLines(
Expand Down

0 comments on commit 6d82c80

Please sign in to comment.