MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

DFA: support unary ++/-- (IDEA-221657)

Browse Source 
Also: define binop widening based on CFG, not on PSI (so loops generated via inliners are also supported)
Better squashing

GitOrigin-RevId: e1e15652b0363357f6d8dd40c6048e09ae436d09
This commit is contained in:
Tagir Valeev
2019-09-11 07:33:10 +00:00
committed by intellij-monorepo-bot
parent 72cc341a78
commit 9c1be97480
12 changed files with 242 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java
+38 -45
View File
@@ -14,7 +14,6 @@ import com.intellij.codeInspection.dataFlow.inliner.*;
import com.intellij.codeInspection.dataFlow.instructions.*;
import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet;
import com.intellij.codeInspection.dataFlow.value.*;
import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
@@ -221,8 +220,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
lExpr.accept(this);
addInstruction(new DupInstruction());
rExpr.accept(this);
addInstruction(new BinopInstruction(
isAcceptableContextForMathOperation(expression) ? JavaTokenType.PLUS : BinopInstruction.STRING_CONCAT_IN_LOOP, null, type));
addInstruction(new BinopInstruction(BinopInstruction.STRING_CONCAT_IN_LOOP, null, type));
}
else {
IElementType sign = TypeConversionUtil.convertEQtoOperation(op);
@@ -232,7 +230,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
generateBoxingUnboxingInstructionFor(lExpr, resType);
rExpr.accept(this);
generateBoxingUnboxingInstructionFor(rExpr, resType);
sign = substituteBinaryOperation(rExpr, sign);
if (isAssignmentDivision(op) && resType != null && PsiType.LONG.isAssignableFrom(resType)) {
checkZeroDivisor();
}
@@ -679,7 +676,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
addInstruction(new PushInstruction(loopVar, null, true));
addInstruction(new PushInstruction(loopVar, null));
addInstruction(new PushInstruction(myFactory.getConstFactory().createFromValue(1, PsiType.INT), null));
addInstruction(new BinopInstruction(JavaTokenType.PLUS, null, loopVar.getType()));
addInstruction(new BinopInstruction(JavaTokenType.PLUS, null, loopVar.getType(), -1, true));
addInstruction(new AssignInstruction(null, null));
addInstruction(new PopInstruction());
}
@@ -1400,8 +1397,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
private void generateOther(PsiPolyadicExpression expression, IElementType op, PsiExpression[] operands, PsiType type) {
op = substituteBinaryOperation(expression, op);
PsiExpression lExpr = operands[0];
lExpr.accept(this);
PsiType lType = lExpr.getType();
@@ -1418,38 +1413,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
}
@Nullable
private IElementType substituteBinaryOperation(PsiExpression expression, IElementType op) {
if (JavaTokenType.PLUS == op) {
if (isAcceptableContextForMathOperation(expression)) return op;
if (TypeUtils.isJavaLangString(expression.getType())) return BinopInstruction.STRING_CONCAT_IN_LOOP;
return null;
}
if ((JavaTokenType.MINUS == op || JavaTokenType.ASTERISK == op) && !isAcceptableContextForMathOperation(expression)) return null;
return op;
}
private boolean isAcceptableContextForMathOperation(PsiExpression expression) {
PsiElement parent = expression.getParent();
while (parent != null && parent != myCodeFragment) {
if ((parent instanceof PsiExpressionList && parent.getParent() instanceof PsiCallExpression) ||
parent instanceof PsiArrayInitializerExpression ||
parent instanceof PsiArrayAccessExpression) {
return true;
}
if (parent instanceof PsiBinaryExpression && RelationType.fromElementType(((PsiBinaryExpression)parent).getOperationTokenType()) != null) {
return true;
}
if (parent instanceof PsiLoopStatement &&
!(parent instanceof PsiForStatement &&
PsiTreeUtil.isAncestor(((PsiForStatement)parent).getInitialization(), expression, false))) {
return false;
}
parent = parent.getParent();
}
return true;
}
private void acceptBinaryRightOperand(@Nullable IElementType op, PsiType type,
PsiExpression lExpr, @Nullable PsiType lType,
PsiExpression rExpr, @Nullable PsiType rType) {
@@ -1949,16 +1912,44 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
PsiExpression operand = PsiUtil.skipParenthesizedExprDown(expression.getOperand());
if (operand != null) {
operand.accept(this);
generateBoxingUnboxingInstructionFor(operand, PsiType.INT);
pushUnknown();
addInstruction(new AssignInstruction(operand, null, myFactory.createValue(operand)));
addInstruction(new DupInstruction());
processIncrementDecrement(expression, operand);
addInstruction(new PopInstruction());
} else {
pushUnknown();
}
pushUnknown();
finishElement(expression);
}
private boolean processIncrementDecrement(PsiUnaryExpression expression, PsiExpression operand) {
IElementType token;
if (expression.getOperationTokenType().equals(JavaTokenType.MINUSMINUS)) {
token = JavaTokenType.MINUS;
}
else if (expression.getOperationTokenType().equals(JavaTokenType.PLUSPLUS)) {
token = JavaTokenType.PLUS;
}
else {
return false;
}
PsiPrimitiveType unboxedType = PsiPrimitiveType.getOptionallyUnboxedType(operand.getType());
if (unboxedType == null) return false;
addInstruction(new DupInstruction());
generateBoxingUnboxingInstructionFor(operand, unboxedType);
PsiType resultType = TypeConversionUtil.binaryNumericPromotion(unboxedType, PsiType.INT);
addInstruction(new PushInstruction(myFactory.getConstFactory().createFromValue(1, PsiType.INT), null));
addInstruction(new BinopInstruction(token, null, resultType));
if (!unboxedType.equals(resultType)) {
addInstruction(new PrimitiveConversionInstruction(unboxedType, null));
}
if (!(operand.getType() instanceof PsiPrimitiveType)) {
addInstruction(new BoxingInstruction(operand.getType()));
}
addInstruction(new AssignInstruction(operand, null, myFactory.createValue(operand)));
return true;
}
@Override public void visitPrefixExpression(PsiPrefixExpression expression) {
startElement(expression);
@@ -1979,8 +1970,10 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
PsiPrimitiveType unboxed = PsiPrimitiveType.getUnboxedType(type);
generateBoxingUnboxingInstructionFor(operand, unboxed == null ? type : unboxed);
if (PsiUtil.isIncrementDecrementOperation(expression)) {
pushUnknown();
addInstruction(new AssignInstruction(operand, null, myFactory.createValue(operand)));
if (!processIncrementDecrement(expression, operand)) {
pushUnknown();
addInstruction(new AssignInstruction(operand, null, myFactory.createValue(operand)));
}
}
else if (expression.getOperationTokenType() == JavaTokenType.EXCL) {
addInstruction(new NotInstruction(expression));
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java
+11
View File
@@ -168,6 +168,13 @@ public class DataFlowRunner {
int endOffset = flow.getInstructionCount();
myInstructions = flow.getInstructions();
for (int i = 0; i < endOffset; i++) {
if (loopNumber[i] > 0 && myInstructions[i] instanceof BinopInstruction) {
((BinopInstruction)myInstructions[i]).widenOperationInLoop();
}
}
myNestedClosures.clear();
myWasForciblyMerged = false;
@@ -337,6 +344,10 @@ public class DataFlowRunner {
} else if (instruction instanceof MethodCallInstruction && !((MethodCallInstruction)instruction).getContracts().isEmpty()) {
joinInstructions.add(myInstructions[index + 1]);
}
else if (instruction instanceof FinishElementInstruction && !((FinishElementInstruction)instruction).getVarsToFlush().isEmpty()) {
// Good chances to squash something after some vars are flushed
joinInstructions.add(myInstructions[index + 1]);
}
}
return joinInstructions;
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaInstructionState.java
+16 -7
View File
@@ -88,14 +88,23 @@ class StateQueue {
}
if (memoryStates.size() > 1 && joinInstructions.contains(instruction)) {
MultiMap<Object, DfaMemoryStateImpl> groups = MultiMap.create();
for (DfaMemoryStateImpl memoryState : memoryStates) {
groups.putValue(memoryState.getSuperficialKey(), memoryState);
}
while (true) {
int beforeSize = memoryStates.size();
MultiMap<Object, DfaMemoryStateImpl> groups = MultiMap.create();
for (DfaMemoryStateImpl memoryState : memoryStates) {
groups.putValue(memoryState.getSuperficialKey(), memoryState);
}
memoryStates = new ArrayList<>();
for (Map.Entry<Object, Collection<DfaMemoryStateImpl>> entry : groups.entrySet()) {
memoryStates.addAll(mergeGroup((List<DfaMemoryStateImpl>)entry.getValue()));
memoryStates = new ArrayList<>();
for (Map.Entry<Object, Collection<DfaMemoryStateImpl>> entry : groups.entrySet()) {
memoryStates.addAll(mergeGroup((List<DfaMemoryStateImpl>)entry.getValue()));
}
if (memoryStates.size() == beforeSize) break;
beforeSize = memoryStates.size();
if (beforeSize == 1) break;
// If some states were merged it's possible that they could be further squashed
memoryStates = squash(memoryStates);
if (memoryStates.size() == beforeSize || memoryStates.size() == 1) break;
}
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java
+3
View File
@@ -259,6 +259,9 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
DfaVariableValue target = replaceQualifier((DfaVariableValue)value, flushed, replacement);
if (target != value) return target;
}
if (value.getType() instanceof PsiPrimitiveType) {
return myFactory.getFactValue(DfaFactType.RANGE, getValueFact(value, DfaFactType.RANGE));
}
DfaNullability dfaNullability = isNotNull(value) ? DfaNullability.NOT_NULL : getValueFact(value, DfaFactType.NULLABILITY);
if (dfaNullability == null) {
dfaNullability = DfaNullability.fromNullability(((DfaVariableValue)value).getInherentNullability());
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java
+1
View File
@@ -254,6 +254,7 @@ class StateMerger {
DfaMemoryStateImpl copy = map.get(var);
if (copy == null) {
copy = state.createCopy();
copy.setVariableState(var, copy.createVariableState(var));
copy.flushVariable(var);
map.put(var, copy);
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java
+52 -4
View File
@@ -20,12 +20,13 @@ import com.intellij.codeInspection.dataFlow.DataFlowRunner;
import com.intellij.codeInspection.dataFlow.DfaInstructionState;
import com.intellij.codeInspection.dataFlow.DfaMemoryState;
import com.intellij.codeInspection.dataFlow.InstructionVisitor;
import com.intellij.codeInspection.dataFlow.value.DfaRelationValue;
import com.intellij.openapi.util.TextRange;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.PsiPolyadicExpression;
import com.intellij.psi.PsiType;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.tree.TokenSet;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.ig.psiutils.TypeUtils;
import org.jetbrains.annotations.Nullable;
import static com.intellij.psi.JavaTokenType.*;
@@ -47,19 +48,66 @@ public class BinopInstruction extends BranchingInstruction implements Expression
*/
public static final IElementType STRING_EQUALITY_BY_CONTENT = EQ;
private final IElementType myOperationSign;
private IElementType myOperationSign;
private final @Nullable PsiType myResultType;
private final int myLastOperand;
private final boolean myUnrolledLoop;
public BinopInstruction(IElementType opSign, @Nullable PsiExpression psiAnchor, @Nullable PsiType resultType) {
this(opSign, psiAnchor, resultType, -1);
}
public BinopInstruction(IElementType opSign, @Nullable PsiExpression psiAnchor, @Nullable PsiType resultType, int lastOperand) {
this(opSign, psiAnchor, resultType, lastOperand, false);
}
/**
* @param opSign sign of the operation
* @param psiAnchor PSI element to bind the instruction to
* @param resultType result of the operation
* @param lastOperand number of last operand if anchor is a {@link PsiPolyadicExpression} and this instruction is the result of
* part of that expression; -1 if not applicable
* @param unrolledLoop true means that this instruction is executed inside an unrolled loop; in this case it will never be widened
*/
public BinopInstruction(IElementType opSign,
@Nullable PsiExpression psiAnchor,
@Nullable PsiType resultType,
int lastOperand,
boolean unrolledLoop) {
super(psiAnchor);
myResultType = resultType;
myOperationSign = ourSignificantOperations.contains(opSign) ? opSign : null;
myLastOperand = lastOperand;
myUnrolledLoop = unrolledLoop;
}
/**
* Make operation wide (less precise) if necessary (called for the operations inside loops only)
*/
public void widenOperationInLoop() {
// these operations usually produce non-converging states
if (!myUnrolledLoop && (myOperationSign == PLUS || myOperationSign == MINUS || myOperationSign == ASTERISK) &&
mayProduceDivergedState()) {
myOperationSign = TypeUtils.isJavaLangString(myResultType) ? STRING_CONCAT_IN_LOOP : null;
}
}
private boolean mayProduceDivergedState() {
PsiElement anchor = getExpression();
if (anchor instanceof PsiUnaryExpression) {
return PsiUtil.isIncrementDecrementOperation(anchor);
}
while (anchor != null && !(anchor instanceof PsiAssignmentExpression) && !(anchor instanceof PsiVariable)) {
if (anchor instanceof PsiStatement ||
anchor instanceof PsiExpressionList && anchor.getParent() instanceof PsiCallExpression ||
anchor instanceof PsiArrayInitializerExpression || anchor instanceof PsiArrayAccessExpression ||
anchor instanceof PsiBinaryExpression &&
DfaRelationValue.RelationType.fromElementType(((PsiBinaryExpression)anchor).getOperationTokenType()) != null) {
return false;
}
anchor = anchor.getParent();
}
return true;
}
/**
java/java-tests/testData/inspection/dataFlow/fixture/CellsComplex.java
+42
View File
@@ -0,0 +1,42 @@
class Foo {
// TODO: make not complex
public static int[] <weak_warning descr="Method 'cells' is complex: data flow results could be imprecise">cells</weak_warning>(int[] start, int[] end) {
int overlap = 0;
int gaps = 0;
for (int i = 0, j = 0; j < end.length; ) {
if (i < start.length && start[i] < end[j]) {
overlap++;
i++;
} else {
j++;
overlap--;
}
if (overlap == 0) {
gaps++;
}
}
int[] cells = new int[gaps * 2];
overlap = 0;
gaps = 0;
int previousOverlap = 0;
for (int i = 0, j = 0; j < end.length; ) {
if (i < start.length && start[i] < end[j]) {
overlap++;
if (previousOverlap == 0) {
cells[gaps++] = start[i];
}
i++;
} else {
overlap--;
if (overlap == 0) {
cells[gaps++] = end[j];
}
j++;
}
previousOverlap = overlap;
}
return cells;
}
}
java/java-tests/testData/inspection/dataFlow/fixture/ConstantConditionsWithAssignmentsInside.java
+2 -2
View File
@@ -4,8 +4,8 @@ class Contracts {
if (flag == (flag = true)) System.out.println();
int x = 1;
boolean y = x == (x +=1); // returns false
if (y) System.out.println();
boolean y = <warning descr="Condition 'x == (x +=1)' is always 'false'">x == (x +=1)</warning>; // returns false
if (<warning descr="Condition 'y' is always 'false'">y</warning>) System.out.println();
int k = 1;
boolean z = <warning descr="Condition '(k +=1) == k' is always 'true'">(k +=1) == k</warning>; // returns true
java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java
+6
View File
@@ -301,4 +301,10 @@ public class StreamInlining {
<error descr="Cannot resolve symbol 'bb'">bb</error>,
<error descr="Cannot resolve symbol 'cc'">cc</error>));
}
void testNotTooComplexForEach(List<String> list) {
int[] count = {0};
list.stream().forEach(l -> count[0]++);
System.out.println(count[0]);
}
}
java/java-tests/testData/inspection/dataFlow/fixture/UnaryPlusMinus.java
+69
View File
@@ -0,0 +1,69 @@
import java.util.*;
public class UnaryPlusMinus {
void test() {
int x = 0;
if (<warning descr="Condition 'x == 0' is always 'true'">x == 0</warning>) { }
x += 1;
if (<warning descr="Condition 'x == 1' is always 'true'">x == 1</warning>) { }
x++;
if (<warning descr="Condition 'x == 3' is always 'false'">x == 3</warning>) { }
++x;
if (<warning descr="Condition 'x == 3' is always 'true'">x == 3</warning>) {}
if (<warning descr="Condition '--x == 2' is always 'true'">--x == 2</warning>) {}
x--;
if (<warning descr="Condition 'x == 1' is always 'true'">x == 1</warning>) {}
}
void testChar() {
char c = 0;
if (<warning descr="Condition '--c == '\uFFFF'' is always 'true'">--c == '\uFFFF'</warning>) {}
}
void testLong() {
long l = Integer.MAX_VALUE;
l++;
if (<warning descr="Condition 'l == Integer.MAX_VALUE+1L' is always 'true'">l == Integer.MAX_VALUE+1L</warning>) {}
}
void testDouble() {
// Not supported
double x = 0;
x++;
if (x == 1) {}
x = 1e15;
x++;
if (x == 1e15) {}
}
void testArray() {
int[] x = new int[3];
int index = 0;
x[index++] = 1;
x[index++] = 2;
x[index++] = 3;
x[<warning descr="Array index is out of bounds">index++</warning>] = 4;
}
int testInForCondition(int[] _data, int _pos) {
int max = _data[_pos - 1];
for (int i = _pos - 1; i-- > 0;) {
max = Math.max(max, _data[_pos]);
}
return max;
}
void testNotComplexInLoop() {
int x = 0;
while(true) {
int y = x + 1;
if (y > 10000) break;
x = y;
}
System.out.println(x);
}
}
java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java
+1
View File
@@ -668,4 +668,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
public void testClassCastExceptionDispatch() { doTest(); }
public void testInstanceQualifiedStaticMember() { doTest(); }
public void testClassEqualityCornerCase() { doTest(); }
public void testCellsComplex() { doTest(); }
}
java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java
+1
View File
@@ -62,4 +62,5 @@ public class DataFlowRangeAnalysisTest extends DataFlowInspectionTestCase {
public void testBackPropagationMod() { doTest(); }
public void testArithmeticNoOp() { doTest(); }
public void testStringConcat() { doTest(); }
public void testUnaryPlusMinus() { doTest(); }
}