From 6a9d058db42d74b2ccb32f77925c56dde3d85b41 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Thu, 25 Aug 2016 18:41:21 +0300 Subject: [PATCH] UNUSED_VALUE is now reported on expression only if this expression is unused in all (e.g. finally) branches #KT-9825 Fixed (cherry picked from commit aac8e94) --- .../cfg/ControlFlowInformationProvider.kt | 40 ++++++++++------ .../cfg-variables/bugs/kt9825.instructions | 46 +++++++++++++++++++ .../testData/cfg-variables/bugs/kt9825.kt | 10 ++++ .../testData/cfg-variables/bugs/kt9825.values | 24 ++++++++++ .../controlFlowAnalysis/assignedInFinally.kt | 10 ++++ .../controlFlowAnalysis/assignedInFinally.txt | 3 ++ .../varnotnull/initInTryReturnInCatch.kt | 6 +-- .../kotlin/cfg/DataFlowTestGenerated.java | 6 +++ .../kotlin/cfg/PseudoValueTestGenerated.java | 6 +++ .../checkers/DiagnosticsTestGenerated.java | 6 +++ 10 files changed, 138 insertions(+), 19 deletions(-) create mode 100644 compiler/testData/cfg-variables/bugs/kt9825.instructions create mode 100644 compiler/testData/cfg-variables/bugs/kt9825.kt create mode 100644 compiler/testData/cfg-variables/bugs/kt9825.values create mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.kt create mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt index 8005943432a..def819a534a 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.kt @@ -533,6 +533,8 @@ class ControlFlowInformationProvider private constructor( private fun markUnusedVariables() { val variableStatusData = pseudocodeVariablesData.variableUseStatusData val reportedDiagnosticMap = hashMapOf>() + val unusedValueExpressions = hashMapOf>() + val usedValueExpressions = hashSetOf() pseudocode.traverse(TraversalOrder.BACKWARD, variableStatusData) { instruction: Instruction, enterData: Map, @@ -551,21 +553,12 @@ class ControlFlowInformationProvider private constructor( when (instruction) { is WriteValueInstruction -> { if (trace.get(CAPTURED_IN_CLOSURE, variableDescriptor) != null) return@traverse - if (variableUseState !== READ) { - val element = instruction.element - when (element) { - is KtBinaryExpression -> if (element.operationToken === KtTokens.EQ) { - element.right?.let { - report(Errors.UNUSED_VALUE.on(element, it, variableDescriptor), ctxt) - } - } - is KtPostfixExpression -> { - val operationToken = element.operationReference.getReferencedNameElementType() - if (operationToken === KtTokens.PLUSPLUS || operationToken === KtTokens.MINUSMINUS) { - report(Errors.UNUSED_CHANGED_VALUE.on(element, element), ctxt) - } - } - } + val expressionInQuestion = instruction.element as? KtExpression ?: return@traverse + if (variableUseState != READ) { + unusedValueExpressions.put(expressionInQuestion, variableDescriptor to ctxt) + } + else { + usedValueExpressions.add(expressionInQuestion) } } is VariableDeclarationInstruction -> { @@ -623,6 +616,23 @@ class ControlFlowInformationProvider private constructor( } } } + unusedValueExpressions.keys.removeAll(usedValueExpressions) + for ((expressionInQuestion, variableInContext) in unusedValueExpressions) { + val (variableDescriptor, ctxt) = variableInContext + when (expressionInQuestion) { + is KtBinaryExpression -> if (expressionInQuestion.operationToken === KtTokens.EQ) { + expressionInQuestion.right?.let { + report(Errors.UNUSED_VALUE.on(expressionInQuestion, it, variableDescriptor), ctxt) + } + } + is KtPostfixExpression -> { + val operationToken = expressionInQuestion.operationReference.getReferencedNameElementType() + if (operationToken === KtTokens.PLUSPLUS || operationToken === KtTokens.MINUSMINUS) { + report(Errors.UNUSED_CHANGED_VALUE.on(expressionInQuestion, expressionInQuestion), ctxt) + } + } + } + } } //////////////////////////////////////////////////////////////////////////////// diff --git a/compiler/testData/cfg-variables/bugs/kt9825.instructions b/compiler/testData/cfg-variables/bugs/kt9825.instructions new file mode 100644 index 00000000000..0ec9d7e0691 --- /dev/null +++ b/compiler/testData/cfg-variables/bugs/kt9825.instructions @@ -0,0 +1,46 @@ +== test5 == +fun test5() { + var a: Int + try { + a = 3 + } + finally { + a = 5 + } + a.hashCode() +} +--------------------- +L0: + 1 INIT: in: {} out: {} USE: in: {} out: {} + 2 mark({ var a: Int try { a = 3 } finally { a = 5 } a.hashCode() }) + v(var a: Int) INIT: in: {} out: {a=D} + mark(try { a = 3 } finally { a = 5 }) INIT: in: {a=D} out: {a=D} + jmp?(L2) + 3 mark({ a = 3 }) + r(3) -> + w(a|) INIT: in: {a=D} out: {a=ID} + 2 jmp(L3) INIT: in: {a=ID} out: {a=ID} USE: in: {a=WRITTEN_AFTER_READ} out: {a=WRITTEN_AFTER_READ} +L2 [onExceptionToFinallyBlock]: +L4 [start finally]: + 3 mark({ a = 5 }) INIT: in: {a=D} out: {a=D} + r(5) -> USE: in: {a=ONLY_WRITTEN_NEVER_READ} out: {a=ONLY_WRITTEN_NEVER_READ} + w(a|) INIT: in: {a=D} out: {a=ID} USE: in: {} out: {a=ONLY_WRITTEN_NEVER_READ} +L5 [finish finally]: + 2 jmp(error) INIT: in: {a=ID} out: {a=ID} USE: in: {} out: {} +L3 [skipFinallyToErrorBlock]: +L6 [copy of L2, onExceptionToFinallyBlock]: + 3 mark({ a = 5 }) + r(5) -> USE: in: {a=WRITTEN_AFTER_READ} out: {a=WRITTEN_AFTER_READ} + w(a|) USE: in: {a=READ} out: {a=WRITTEN_AFTER_READ} + 2 merge(try { a = 3 } finally { a = 5 }|!) -> + mark(a.hashCode()) USE: in: {a=READ} out: {a=READ} + r(a) -> USE: in: {} out: {a=READ} + mark(hashCode()) + call(hashCode(), hashCode|) -> +L1: + 1 INIT: in: {} out: {} +error: + +sink: + USE: in: {} out: {} +===================== diff --git a/compiler/testData/cfg-variables/bugs/kt9825.kt b/compiler/testData/cfg-variables/bugs/kt9825.kt new file mode 100644 index 00000000000..245ce470e75 --- /dev/null +++ b/compiler/testData/cfg-variables/bugs/kt9825.kt @@ -0,0 +1,10 @@ +fun test5() { + var a: Int + try { + a = 3 + } + finally { + a = 5 + } + a.hashCode() +} \ No newline at end of file diff --git a/compiler/testData/cfg-variables/bugs/kt9825.values b/compiler/testData/cfg-variables/bugs/kt9825.values new file mode 100644 index 00000000000..27553b2bd3f --- /dev/null +++ b/compiler/testData/cfg-variables/bugs/kt9825.values @@ -0,0 +1,24 @@ +== test5 == +fun test5() { + var a: Int + try { + a = 3 + } + finally { + a = 5 + } + a.hashCode() +} +--------------------- +3 : Int NEW: r(3) -> +a = 3 !: * +{ a = 3 } !: * COPY +5 : Int NEW: r(5) -> +a = 5 !: * +{ a = 5 } !: * COPY +try { a = 3 } finally { a = 5 } : * NEW: merge(try { a = 3 } finally { a = 5 }|!) -> +a : OR{{<: Any}, {<: Any}} NEW: r(a) -> +hashCode() : * NEW: call(hashCode(), hashCode|) -> +a.hashCode() : * COPY +{ var a: Int try { a = 3 } finally { a = 5 } a.hashCode() } : * COPY +===================== diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.kt new file mode 100644 index 00000000000..e93f25ac05e --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.kt @@ -0,0 +1,10 @@ +fun test5() { + var a: Int + try { + a = 3 + } + finally { + a = 5 + } + a.hashCode() +} diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.txt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.txt new file mode 100644 index 00000000000..3e3fb2b351e --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.txt @@ -0,0 +1,3 @@ +package + +public fun test5(): kotlin.Unit diff --git a/compiler/testData/diagnostics/tests/smartCasts/varnotnull/initInTryReturnInCatch.kt b/compiler/testData/diagnostics/tests/smartCasts/varnotnull/initInTryReturnInCatch.kt index 8ef22d30b74..087c88b63f6 100644 --- a/compiler/testData/diagnostics/tests/smartCasts/varnotnull/initInTryReturnInCatch.kt +++ b/compiler/testData/diagnostics/tests/smartCasts/varnotnull/initInTryReturnInCatch.kt @@ -59,8 +59,7 @@ fun test5() { return } finally { - // Error: KT-9825 - a = 5 + a = 5 } a.hashCode() // a is never null here } @@ -73,8 +72,7 @@ fun test6() { return } finally { - // Error: KT-9825 - a = null + a = null } a.hashCode() // a is null here } diff --git a/compiler/tests/org/jetbrains/kotlin/cfg/DataFlowTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/cfg/DataFlowTestGenerated.java index 647d400d834..b5336fbbcb5 100644 --- a/compiler/tests/org/jetbrains/kotlin/cfg/DataFlowTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/cfg/DataFlowTestGenerated.java @@ -112,6 +112,12 @@ public class DataFlowTestGenerated extends AbstractDataFlowTest { doTest(fileName); } + @TestMetadata("kt9825.kt") + public void testKt9825() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg-variables/bugs/kt9825.kt"); + doTest(fileName); + } + @TestMetadata("referenceToPropertyInitializer.kt") public void testReferenceToPropertyInitializer() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg-variables/bugs/referenceToPropertyInitializer.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java index 6e078ad7a71..a00aa895d44 100644 --- a/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java @@ -870,6 +870,12 @@ public class PseudoValueTestGenerated extends AbstractPseudoValueTest { doTest(fileName); } + @TestMetadata("kt9825.kt") + public void testKt9825() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg-variables/bugs/kt9825.kt"); + doTest(fileName); + } + @TestMetadata("referenceToPropertyInitializer.kt") public void testReferenceToPropertyInitializer() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg-variables/bugs/referenceToPropertyInitializer.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 20ec31a4f62..5de3b71d61d 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -3258,6 +3258,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/controlFlowAnalysis"), Pattern.compile("^(.+)\\.kt$"), true); } + @TestMetadata("assignedInFinally.kt") + public void testAssignedInFinally() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.kt"); + doTest(fileName); + } + @TestMetadata("backingFieldInsideGetter.kt") public void testBackingFieldInsideGetter() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlFlowAnalysis/backingFieldInsideGetter.kt");