From 6a2e1e39c2bb7fb0468133c366e38933df3de85a Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 26 Mar 2018 13:21:05 +0700 Subject: [PATCH] DFA: initialize small primitive arrays; unroll small counting loops --- .../codeInspection/dataFlow/CFGBuilder.java | 4 +- .../dataFlow/ControlFlowAnalyzer.java | 97 ++++++++++++++----- .../dataFlow/StandardInstructionVisitor.java | 8 +- .../instructions/BinopInstruction.java | 26 ++--- .../instructions/InstanceofInstruction.java | 5 +- .../dataFlow/fixture/AdvancedArrayAccess.java | 27 ++++++ 6 files changed, 117 insertions(+), 50 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java index 5183a8186513..af4b5745d508 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java @@ -210,7 +210,7 @@ public class CFGBuilder { * @return this builder */ private CFGBuilder compare(IElementType relation) { - myAnalyzer.addInstruction(new BinopInstruction(relation, null, myAnalyzer.getContext().getProject())); + myAnalyzer.addInstruction(new BinopInstruction(relation, null, PsiType.BOOLEAN)); return this; } @@ -556,7 +556,7 @@ public class CFGBuilder { if (qualifier == null) return false; PsiType type = qualifier.getOperand().getType(); push(getFactory().createTypeValue(type, Nullness.NOT_NULL)); - myAnalyzer.addInstruction(new InstanceofInstruction(methodRef, methodRef.getProject(), null, type)); + myAnalyzer.addInstruction(new InstanceofInstruction(methodRef, null, type)); return true; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 380e5c25d289..e45307c8f504 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -47,6 +47,7 @@ import static com.intellij.psi.CommonClassNames.*; public class ControlFlowAnalyzer extends JavaElementVisitor { private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer"); public static final String ORG_JETBRAINS_ANNOTATIONS_CONTRACT = Contract.class.getName(); + private static final int MAX_UNROLL_SIZE = 3; private final PsiElement myCodeFragment; private final boolean myIgnoreAssertions; private final boolean myInlining; @@ -202,7 +203,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { lExpr.accept(this); addInstruction(new DupInstruction()); rExpr.accept(this); - addInstruction(new BinopInstruction(JavaTokenType.PLUS, null, myProject)); + addInstruction(new BinopInstruction(JavaTokenType.PLUS, null, type)); } else if (isAssignmentDivision(op) && type != null && PsiType.LONG.isAssignableFrom(type)) { lExpr.accept(this); @@ -228,7 +229,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { generateBoxingUnboxingInstructionFor(lExpr,exprType); rExpr.accept(this); generateBoxingUnboxingInstructionFor(rExpr, exprType); - addInstruction(new BinopInstruction(null, null, myProject)); + addInstruction(new BinopInstruction(null, null, exprType)); } @Override public void visitAssertStatement(PsiAssertStatement statement) { @@ -483,7 +484,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (length != null) { addInstruction(new PushInstruction(length.createValue(myFactory, qualifier), null)); addInstruction(new PushInstruction(myFactory.getInt(0), null)); - addInstruction(new BinopInstruction(JavaTokenType.EQEQ, iteratedValue, myProject)); + addInstruction(new BinopInstruction(JavaTokenType.EQEQ, iteratedValue, PsiType.BOOLEAN)); addInstruction(new ConditionalGotoInstruction(loopEndOffset, false, null)); hasSizeCheck = true; } @@ -555,13 +556,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { body.accept(this); } - PsiStatement update = statement.getUpdate(); - if (update != null) { - update.accept(this); + if (!addCountingLoopBound(statement)) { + PsiStatement update = statement.getUpdate(); + if (update != null) { + update.accept(this); + } } - addCountingLoopBound(statement); - ControlFlow.ControlFlowOffset offset = initialization != null ? getEndOffset(initialization) : getStartOffset(statement); @@ -575,6 +576,15 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } } + @Nullable + private static Long asLong(PsiExpression expression) { + Object value = ExpressionUtils.computeConstantExpression(expression); + if(value instanceof Integer || value instanceof Long) { + return ((Number)value).longValue(); + } + return null; + } + /** * Add known-to-be-true condition inside counting loop, effectively converting * {@code for(int i=origin; i= 0 && diff <= MAX_UNROLL_SIZE) { + // Unroll small loops + addInstruction(new PushInstruction(loopVar, null)); + addInstruction(new PushInstruction(myFactory.getConstFactory().createFromValue(1, PsiType.INT, null), null)); + addInstruction(new BinopInstruction(JavaTokenType.PLUS, null, loopVar.getVariableType())); + } else { + pushUnknown(); + } + addInstruction(new AssignInstruction(null, null)); addInstruction(new PushInstruction(origin, null)); - addInstruction(new BinopInstruction(JavaTokenType.LE, null, myProject)); + addInstruction(new BinopInstruction(JavaTokenType.LE, null, PsiType.BOOLEAN)); addInstruction(new ConditionalGotoInstruction(getEndOffset(statement), false, null)); + return true; } @Override public void visitIfStatement(PsiIfStatement statement) { @@ -793,7 +815,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(new PushInstruction(myFactory.createValue(caseExpression), caseExpression)); caseValue.accept(this); - addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, myProject)); + addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, PsiType.BOOLEAN)); } else { pushUnknown(); @@ -1211,13 +1233,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { right.accept(this); generateBoxingUnboxingInstructionFor(right, type); checkZeroDivisor(); - addInstruction(new BinopInstruction(expression.getOperationTokenType(), expression.isPhysical() ? expression : null, myProject)); + addInstruction(new BinopInstruction(expression.getOperationTokenType(), expression.isPhysical() ? expression : null, type)); } private void checkZeroDivisor() { addInstruction(new DupInstruction()); addInstruction(new PushInstruction(myFactory.getConstFactory().createFromValue(0, PsiType.LONG, null), null)); - addInstruction(new BinopInstruction(JavaTokenType.NE, null, myProject)); + addInstruction(new BinopInstruction(JavaTokenType.NE, null, PsiType.BOOLEAN)); ConditionalGotoInstruction ifNonZero = new ConditionalGotoInstruction(null, false, null); addInstruction(ifNonZero); throwException(JavaPsiFacade.getElementFactory(myProject).createTypeByFQClassName(ArithmeticException.class.getName()), null); @@ -1236,7 +1258,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { PsiType rType = rExpr.getType(); acceptBinaryRightOperand(op, type, lExpr, lType, rExpr, rType); - addInstruction(new BinopInstruction(op, expression.isPhysical() ? expression : null, myProject)); + addInstruction(new BinopInstruction(op, expression.isPhysical() ? expression : null, type)); lExpr = rExpr; lType = rType; @@ -1332,7 +1354,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { operand.accept(this); generateBoxingUnboxingInstructionFor(operand, exprType); PsiElement psiAnchor = i == operands.length - 1 && expression.isPhysical() ? expression : null; - addInstruction(new BinopInstruction(JavaTokenType.NE, psiAnchor, myProject)); + addInstruction(new BinopInstruction(JavaTokenType.NE, psiAnchor, exprType)); } } @@ -1475,7 +1497,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { type = ((PsiClassType)type).rawType(); } addInstruction(new PushInstruction(myFactory.createTypeValue(type, Nullness.NOT_NULL), null)); - addInstruction(new InstanceofInstruction(expression, myProject, operand, type)); + addInstruction(new InstanceofInstruction(expression, operand, type)); } else { pushUnknown(); @@ -1572,7 +1594,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { // if a contract resulted in 'fail', handle it addInstruction(new DupInstruction()); addInstruction(new PushInstruction(myFactory.getConstFactory().getContractFail(), null)); - addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, myProject)); + addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, PsiType.BOOLEAN)); ConditionalGotoInstruction ifNotFail = new ConditionalGotoInstruction(null, true, null); addInstruction(ifNotFail); addInstruction(new EmptyStackInstruction()); @@ -1683,6 +1705,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(new AssignInstruction(null, length)); addInstruction(new PopInstruction()); // stack: ... var + + initializeSmallArray((PsiArrayType)type, var, dimensions); } else { pushUnknown(); // qualifier @@ -1703,6 +1727,29 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { finishElement(expression); } + private void initializeSmallArray(PsiArrayType type, DfaVariableValue var, PsiExpression[] dimensions) { + if (dimensions.length != 1) return; + PsiType componentType = type.getComponentType(); + // Ignore objects as they may produce false NPE warnings due to non-perfect loop handling + if (!(componentType instanceof PsiPrimitiveType)) return; + Object val = ExpressionUtils.computeConstantExpression(dimensions[0]); + if (val instanceof Integer) { + int lengthValue = (Integer)val; + if (lengthValue > 0 && lengthValue <= MAX_UNROLL_SIZE) { + for (int i = 0; i < lengthValue; i++) { + DfaValue value = getFactory().getExpressionFactory().getArrayElementValue(var, i); + addInstruction(new PushInstruction(value, null, true)); + } + addInstruction(new PushInstruction(getFactory().getConstFactory().createDefault(componentType), null)); + for (int i = lengthValue - 1; i >= 0; i--) { + DfaValue value = getFactory().getExpressionFactory().getArrayElementValue(var, i); + addInstruction(new AssignInstruction(null, value)); + } + addInstruction(new PopInstruction()); + } + } + } + @Nullable private PsiMethod pushConstructorArguments(PsiConstructorCall call) { PsiExpressionList args = call.getArgumentList(); @@ -1778,7 +1825,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { else if (expression.getOperationTokenType() == JavaTokenType.MINUS && (PsiType.INT.equals(type) || PsiType.LONG.equals(type))) { addInstruction(new PushInstruction(myFactory.getConstFactory().createDefault(type), null)); addInstruction(new SwapInstruction()); - addInstruction(new BinopInstruction(expression.getOperationTokenType(), expression, myProject)); + addInstruction(new BinopInstruction(expression.getOperationTokenType(), expression, type)); } else { addInstruction(new PopInstruction()); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index e4a81d167a48..0a08df6b6fe5 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -28,6 +28,7 @@ import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.ObjectUtils; import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.psiutils.MethodUtils; +import com.siyeh.ig.psiutils.TypeUtils; import gnu.trove.THashSet; import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; @@ -603,8 +604,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { } } DfaValue result = null; - PsiElement expr = instruction.getPsiAnchor(); - PsiType type = expr instanceof PsiExpression ? ((PsiExpression)expr).getType() : null; + PsiType type = instruction.getResultType(); if (PsiType.INT.equals(type) || PsiType.LONG.equals(type)) { LongRangeSet left = memState.getValueFact(dfaLeft, DfaFactType.RANGE); LongRangeSet right = memState.getValueFact(dfaRight, DfaFactType.RANGE); @@ -616,8 +616,8 @@ public class StandardInstructionVisitor extends InstructionVisitor { } } if (result == null) { - if (JavaTokenType.PLUS == opSign && !(type instanceof PsiPrimitiveType)) { - result = instruction.getNonNullStringValue(runner.getFactory()); + if (JavaTokenType.PLUS == opSign && TypeUtils.isJavaLangString(type)) { + result = runner.getFactory().createTypeValue(type, Nullness.NOT_NULL); } else if (instruction instanceof InstanceofInstruction) { handleInstanceof((InstanceofInstruction)instruction, dfaRight, dfaLeft); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java index 1cc394eb3fe5..180e261e25bf 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java @@ -16,18 +16,14 @@ package com.intellij.codeInspection.dataFlow.instructions; -import com.intellij.codeInspection.dataFlow.*; -import com.intellij.codeInspection.dataFlow.value.DfaValue; -import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; -import com.intellij.openapi.project.Project; -import com.intellij.psi.PsiClassType; +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.psi.PsiElement; -import com.intellij.psi.PsiManager; import com.intellij.psi.PsiType; -import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.tree.IElementType; import com.intellij.psi.tree.TokenSet; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import static com.intellij.psi.JavaTokenType.*; @@ -36,11 +32,11 @@ public class BinopInstruction extends BranchingInstruction { private static final TokenSet ourSignificantOperations = TokenSet.create(EQEQ, NE, LT, GT, LE, GE, INSTANCEOF_KEYWORD, PLUS, MINUS, AND, PERC, DIV, GTGT, GTGTGT); private final IElementType myOperationSign; - private final Project myProject; + private final @Nullable PsiType myResultType; - public BinopInstruction(IElementType opSign, @Nullable PsiElement psiAnchor, @NotNull Project project) { + public BinopInstruction(IElementType opSign, @Nullable PsiElement psiAnchor, @Nullable PsiType resultType) { super(psiAnchor); - myProject = project; + myResultType = resultType; myOperationSign = ourSignificantOperations.contains(opSign) ? opSign : null; } @@ -49,11 +45,9 @@ public class BinopInstruction extends BranchingInstruction { return visitor.visitBinop(this, runner, stateBefore); } - public DfaValue getNonNullStringValue(final DfaValueFactory factory) { - PsiElement anchor = getPsiAnchor(); - Project project = myProject; - PsiClassType string = PsiType.getJavaLangString(PsiManager.getInstance(project), anchor == null ? GlobalSearchScope.allScope(project) : anchor.getResolveScope()); - return factory.createTypeValue(string, Nullness.NOT_NULL); + @Nullable + public PsiType getResultType() { + return myResultType; } public String toString() { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/InstanceofInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/InstanceofInstruction.java index f5af7346d213..1b4a9a5446f0 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/InstanceofInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/InstanceofInstruction.java @@ -19,7 +19,6 @@ 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.openapi.project.Project; import com.intellij.psi.JavaTokenType; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiExpression; @@ -34,8 +33,8 @@ public class InstanceofInstruction extends BinopInstruction { @Nullable private final PsiExpression myLeft; @NotNull private final PsiType myCastType; - public InstanceofInstruction(PsiElement psiAnchor, @NotNull Project project, @Nullable PsiExpression left, @NotNull PsiType castType) { - super(JavaTokenType.INSTANCEOF_KEYWORD, psiAnchor, project); + public InstanceofInstruction(PsiElement psiAnchor, @Nullable PsiExpression left, @NotNull PsiType castType) { + super(JavaTokenType.INSTANCEOF_KEYWORD, psiAnchor, PsiType.BOOLEAN); myLeft = left; myCastType = castType; } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java b/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java index 3338cecf6a3a..e8856ceaa36f 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java @@ -167,4 +167,31 @@ class AdvancedArrayAccess { System.out.println("Impossible"); } } + + void testNonInitializedShort() { + int[] x = new int[2]; + if(x[0] == 0) { + System.out.println("Always"); + } + } + + void testFor2() { + String[] x = new String[2]; + for (int i = 0; i < 2; i++) { + x[i] = String.valueOf(i); + } + for (int i = 0; i < 2; i++) { + System.out.println(x[i].trim()); + } + } + + void testFor3() { + int[] x = new int[3]; + for (int i = 0; i < 3; i++) { + x[i] = i; + } + if(x[2] == 2) { + System.out.println("Always"); + } + } }