From 77697dc741e33712dd71552f5e1839691addb4e9 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 14 Oct 2022 18:52:44 +0200 Subject: [PATCH] [java-dfa] Fix pattern in instanceof control flow when operand is not variable Also, add diagnostics for diverged stack problem GitOrigin-RevId: 36661c8cb0a40c82c483a373fd45cd9aa90f61e1 --- .../dataFlow/java/ControlFlowAnalyzer.java | 24 +++++++++---------- .../fixture/PatternInStreamNotComplex.java | 21 ++++++++++++++++ .../DataFlowInspection17Test.java | 4 ++++ .../StandardDataFlowInterpreter.java | 10 ++++++++ .../dataFlow/memory/DfaMemoryState.java | 5 ++++ .../dataFlow/memory/DfaMemoryStateImpl.java | 5 ++++ 6 files changed, 56 insertions(+), 13 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/PatternInStreamNotComplex.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java index 60f8aa58dc4d..1ad686e2a80f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java @@ -1074,18 +1074,19 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { private void processPatternInSwitch(@NotNull PsiPattern pattern, @NotNull DfaVariableValue expressionValue, @NotNull PsiType checkType) { DeferredOffset endPatternOffset = new DeferredOffset(); - processPattern(pattern, pattern, expressionValue, checkType, null, endPatternOffset); + addInstruction(new JvmPushInstruction(expressionValue, null)); + processPattern(pattern, pattern, checkType, null, endPatternOffset); endPatternOffset.setOffset(getInstructionCount()); } private void processPatternInInstanceof(@NotNull PsiPattern pattern, @NotNull PsiInstanceOfExpression expression, - @NotNull DfaVariableValue expressionValue, @NotNull PsiType checkType) { + @NotNull PsiType checkType) { boolean potentiallyRedundantInstanceOf = pattern instanceof PsiTypeTestPattern || JavaPsiPatternUtil.skipParenthesizedPatternDown(pattern) instanceof PsiTypeTestPattern || pattern instanceof PsiDeconstructionPattern dec && JavaPsiPatternUtil.hasTotalComponents(dec); DfaAnchor instanceofAnchor = potentiallyRedundantInstanceOf ? new JavaExpressionAnchor(expression) : null; DeferredOffset endPatternOffset = new DeferredOffset(); - processPattern(pattern, pattern, expressionValue, checkType, instanceofAnchor, endPatternOffset); + processPattern(pattern, pattern, checkType, instanceofAnchor, endPatternOffset); endPatternOffset.setOffset(getInstructionCount()); if (!potentiallyRedundantInstanceOf) { addInstruction(new ResultOfInstruction(new JavaExpressionAnchor(expression))); @@ -1093,12 +1094,11 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } private void processPattern(@NotNull PsiPattern sourcePattern, @Nullable PsiPattern innerPattern, - @NotNull DfaVariableValue expressionValue, @NotNull PsiType checkType, - @Nullable DfaAnchor instanceofAnchor, @NotNull DeferredOffset endPatternOffset) { + @NotNull PsiType checkType, @Nullable DfaAnchor instanceofAnchor, @NotNull DeferredOffset endPatternOffset) { if (innerPattern == null) return; if (innerPattern instanceof PsiGuardedPattern guardedPattern) { PsiPrimaryPattern primaryPattern = guardedPattern.getPrimaryPattern(); - processPattern(sourcePattern, primaryPattern, expressionValue, checkType, instanceofAnchor, endPatternOffset); + processPattern(sourcePattern, primaryPattern, checkType, instanceofAnchor, endPatternOffset); PsiExpression expression = guardedPattern.getGuardingExpression(); if (expression != null) { expression.accept(this); @@ -1116,7 +1116,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } else if (innerPattern instanceof PsiParenthesizedPattern) { PsiPattern unwrappedPattern = JavaPsiPatternUtil.skipParenthesizedPatternDown(innerPattern); - processPattern(sourcePattern, unwrappedPattern, expressionValue, checkType, instanceofAnchor, endPatternOffset); + processPattern(sourcePattern, unwrappedPattern, checkType, instanceofAnchor, endPatternOffset); } else if (innerPattern instanceof PsiDeconstructionPattern deconstructionPattern) { PsiPatternVariable variable = deconstructionPattern.getPatternVariable(); @@ -1124,8 +1124,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { DfaVariableValue patternDfaVar = variable == null ? createTempVariable(patternType) : PlainDescriptor.createVariableValue(getFactory(), variable); - addInstruction(new JvmPushInstruction(expressionValue, null)); - addTypeCheckPattern(sourcePattern, innerPattern, checkType, endPatternOffset, patternDfaVar, instanceofAnchor); addInstruction(new PopInstruction()); @@ -1142,7 +1140,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (accessor == null) continue; DfaVariableValue accessorDfaVar = getFactory().getVarFactory().createVariableValue(new GetterDescriptor(accessor), patternDfaVar); - processPattern(sourcePattern, patternComponent, accessorDfaVar, recordComponent.getType(), null, endPatternOffset); + addInstruction(new JvmPushInstruction(accessorDfaVar, null)); + processPattern(sourcePattern, patternComponent, recordComponent.getType(), null, endPatternOffset); } } } @@ -1152,8 +1151,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (variable == null) return; DfaVariableValue patternDfaVar = PlainDescriptor.createVariableValue(getFactory(), variable); - addInstruction(new JvmPushInstruction(expressionValue, null)); - addTypeCheckPattern(sourcePattern, innerPattern, checkType, endPatternOffset, patternDfaVar, instanceofAnchor); addInstruction(new PopInstruction()); @@ -1795,6 +1792,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { DfaValue expr = JavaDfaValueFactory.getExpressionDfaValue(getFactory(), operand); if (expr instanceof DfaVariableValue) { expressionValue = (DfaVariableValue)expr; + builder.push(expressionValue); } else { expressionValue = createTempVariable(operand.getType()); @@ -1803,7 +1801,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { .pushExpression(operand) .assign(); } - processPatternInInstanceof(pattern, expression, expressionValue, operandType); + processPatternInInstanceof(pattern, expression, operandType); } else { pushUnknown(); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/PatternInStreamNotComplex.java b/java/java-tests/testData/inspection/dataFlow/fixture/PatternInStreamNotComplex.java new file mode 100644 index 000000000000..9950b0d21917 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/PatternInStreamNotComplex.java @@ -0,0 +1,21 @@ +import java.util.stream.Stream; + +class Aaa { + private static void test(String str) { + Stream.of(str) + .flatMap(Aaa::stream) + .forEach(expr -> { + if (value(expr) instanceof Integer i) { + System.out.println(expr); + } + }); + } + + private static Object value(String expr) { + return expr.trim(); + } + + private static Stream stream(String str) { + return Stream.of(str); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection17Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection17Test.java index c4b77b778dad..330410fe28af 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection17Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection17Test.java @@ -46,6 +46,10 @@ public class DataFlowInspection17Test extends DataFlowInspectionTestCase { public void testPatterns() { doTest(); } + + public void testPatternInStreamNotComplex() { + doTest(); + } public void testInstanceof() { doTest(); diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/interpreter/StandardDataFlowInterpreter.java b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/interpreter/StandardDataFlowInterpreter.java index e77b969c7fe6..340d36189a5d 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/interpreter/StandardDataFlowInterpreter.java +++ b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/interpreter/StandardDataFlowInterpreter.java @@ -298,6 +298,16 @@ public class StandardDataFlowInterpreter implements DataFlowInterpreter { private @NotNull DfaInstructionState mergeBackBranches(DfaInstructionState instructionState, Collection processed) { DfaMemoryStateImpl curState = (DfaMemoryStateImpl)instructionState.getMemoryState(); + int curStateStackSize = curState.getStackSize(); + if (processed.size() > 10 && curStateStackSize > 10) { + for (DfaMemoryState state : processed) { + int diff = curStateStackSize - state.getStackSize(); + if (diff > 10) { + throw new IllegalStateException("Stack for instruction %d increased by %d; it's likely that IR was built incorrectly" + .formatted(instructionState.getInstruction().getIndex(), diff)); + } + } + } Object key = curState.getMergeabilityKey(); DfaMemoryStateImpl mergedState = StreamEx.of(processed).filterBy(DfaMemoryState::getMergeabilityKey, key) diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryState.java b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryState.java index 3e46b394fbd4..9c49aad68a27 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryState.java +++ b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryState.java @@ -52,6 +52,11 @@ public interface DfaMemoryState { */ @Nullable DfaValue getStackValue(int offset); + /** + * @return number of values on the stack + */ + int getStackSize(); + /** * @return true if there are no values in the stack */ diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java index a8c5ebc7bdb4..e4b4cf57a36a 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java +++ b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java @@ -176,6 +176,11 @@ public class DfaMemoryStateImpl implements DfaMemoryState { return index < 0 ? null : myStack.get(index); } + @Override + public int getStackSize() { + return myStack.size(); + } + @Override public void push(@NotNull DfaValue value) { assert value.getFactory() == myFactory : value;