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)

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<Instruction, DiagnosticFactory<*>>()
+        val unusedValueExpressions = hashMapOf<KtExpression, Pair<VariableDescriptor, VariableUseContext>>()
+        val usedValueExpressions = hashSetOf<KtExpression>()
         pseudocode.traverse(TraversalOrder.BACKWARD, variableStatusData) {
             instruction: Instruction,
             enterData: Map<VariableDescriptor, VariableUseState>,
@@ -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 expressionInQuestion = instruction.element as? KtExpression ?: return@traverse
-                        val element = instruction.element
+                    if (variableUseState != READ) {
-                        when (element) {
+                        unusedValueExpressions.put(expressionInQuestion, variableDescriptor to ctxt)
-                            is KtBinaryExpression -> if (element.operationToken === KtTokens.EQ) {
+                    }
-                                element.right?.let {
+                    else {
-                                    report(Errors.UNUSED_VALUE.on(element, it, variableDescriptor), ctxt)
+                        usedValueExpressions.add(expressionInQuestion)
-                                }
-                            }
-                            is KtPostfixExpression -> {
-                                val operationToken = element.operationReference.getReferencedNameElementType()
-                                if (operationToken === KtTokens.PLUSPLUS || operationToken === KtTokens.MINUSMINUS) {
-                                    report(Errors.UNUSED_CHANGED_VALUE.on(element, element), ctxt)
-                                }
-                            }
-                        }
                     }
                 }
                 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)
+                    }
+                }
+            }
+        }
     }
 
     ////////////////////////////////////////////////////////////////////////////////

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 <START>                                                              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) -> <v0>
+    w(a|<v0>)                                                            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) -> <v2>                                                                                         USE: in: {a=ONLY_WRITTEN_NEVER_READ} out: {a=ONLY_WRITTEN_NEVER_READ}
+    w(a|<v2>)                                                            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) -> <v2>                                                                                         USE: in: {a=WRITTEN_AFTER_READ} out: {a=WRITTEN_AFTER_READ}
+    w(a|<v2>)                                                                                            USE: in: {a=READ} out: {a=WRITTEN_AFTER_READ}
+  2 merge(try { a = 3 } finally { a = 5 }|!<v1>) -> <v4>
+    mark(a.hashCode())                                                                                   USE: in: {a=READ} out: {a=READ}
+    r(a) -> <v5>                                                                                         USE: in: {} out: {a=READ}
+    mark(hashCode())
+    call(hashCode(), hashCode|<v5>) -> <v6>
+L1:
+  1 <END>                                                                INIT: in: {} out: {}
+error:
+    <ERROR>
+sink:
+    <SINK>                                                                                               USE: in: {} out: {}
+=====================

compiler/testData/cfg-variables/bugs/kt9825.kt

@@ -0,0 +1,10 @@
+fun test5() {
+    var a: Int
+    try {
+        a = 3
+    }
+    finally {
+        a = 5
+    }
+    a.hashCode()
+}

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                                                             <v0>: Int                      NEW: r(3) -> <v0>
+a = 3                                                         !<v1>: *
+{ a = 3 }                                                     !<v1>: *                       COPY
+5                                                             <v2>: Int                      NEW: r(5) -> <v2>
+a = 5                                                         !<v3>: *
+{ a = 5 }                                                     !<v3>: *                       COPY
+try { a = 3 } finally { a = 5 }                               <v4>: *                        NEW: merge(try { a = 3 } finally { a = 5 }|!<v1>) -> <v4>
+a                                                             <v5>: OR{{<: Any}, {<: Any}}   NEW: r(a) -> <v5>
+hashCode()                                                    <v6>: *                        NEW: call(hashCode(), hashCode|<v5>) -> <v6>
+a.hashCode()                                                  <v6>: *                        COPY
+{ var a: Int try { a = 3 } finally { a = 5 } a.hashCode() }   <v6>: *                        COPY
+=====================

compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.kt

@@ -0,0 +1,10 @@
+fun test5() {
+    var a: Int
+    try {
+        <!UNUSED_VALUE!>a =<!> 3
+    }
+    finally {
+        a = 5
+    }
+    a.hashCode()
+}

compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInFinally.txt

@@ -0,0 +1,3 @@
+package
+
+public fun test5(): kotlin.Unit

compiler/testData/diagnostics/tests/smartCasts/varnotnull/initInTryReturnInCatch.kt

@@ -59,8 +59,7 @@ fun test5() {
         return
     }
     finally {
-        // Error: KT-9825
+        a = 5
-        <!UNUSED_VALUE!>a =<!> 5
     }
     <!DEBUG_INFO_SMARTCAST!>a<!>.hashCode() // a is never null here
 }
@@ -73,8 +72,7 @@ fun test6() {
         return
     }
     finally {
-        // Error: KT-9825
+        a = null
-        <!UNUSED_VALUE!>a =<!> null
     }
     <!DEBUG_INFO_CONSTANT!>a<!><!UNSAFE_CALL!>.<!>hashCode() // a is null here
 }

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");

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");

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");