Debugger: Prefer the closest captured values

idea/jvm-debugger/jvm-debugger-core/src/org/jetbrains/kotlin/idea/debugger/stackFrame/CapturedValueData.kt

@@ -16,7 +16,6 @@ import com.intellij.openapi.project.Project
 import com.intellij.psi.PsiExpression
 import com.intellij.xdebugger.frame.XValueModifier
 import com.sun.jdi.*
-import org.jetbrains.org.objectweb.asm.Type as AsmType
 
 data class CapturedValueData(
     val valueName: String,

idea/jvm-debugger/jvm-debugger-core/src/org/jetbrains/kotlin/idea/debugger/stackFrame/CapturedValuesSearcher.kt

@@ -0,0 +1,134 @@
+/*
+ * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.idea.debugger.stackFrame
+
+import com.intellij.debugger.impl.descriptors.data.DescriptorData
+import com.intellij.debugger.ui.impl.watch.ValueDescriptorImpl
+import com.sun.jdi.Field
+import com.sun.jdi.ObjectReference
+import com.sun.jdi.Value
+import org.jetbrains.kotlin.codegen.AsmUtil
+import org.jetbrains.kotlin.idea.debugger.safeFields
+import java.util.*
+
+private sealed class PendingValue {
+    class Ordinary(val name: String, val field: Field, val container: Container) : PendingValue() {
+        override fun addTo(existingVariables: ExistingVariables): CapturedValueData? {
+            if (!existingVariables.add(ExistingVariable.Ordinary(name))) {
+                return null
+            }
+
+            return CapturedValueData(name, container.value, field)
+        }
+    }
+
+    class This(val label: String, val value: Value?) : PendingValue() {
+        override fun addTo(existingVariables: ExistingVariables): DescriptorData<out ValueDescriptorImpl>? {
+            val thisName = if (existingVariables.hasThisVariables) {
+                if (!existingVariables.add(ExistingVariable.LabeledThis(label))) {
+                    // Avoid item duplication
+                    return null
+                }
+
+                getThisName(label)
+            } else {
+                if (!existingVariables.add(ExistingVariable.LabeledThis(label))) {
+                    return null
+                }
+                AsmUtil.THIS
+            }
+
+            return LabeledThisData(label, thisName, value)
+        }
+    }
+
+    class Container(val value: ObjectReference) : PendingValue() {
+        fun getChildren(): List<PendingValue> {
+            return value.referenceType().safeFields()
+                .filter { it.isApplicable() }
+                .mapNotNull { createPendingValue(this, it) }
+        }
+
+        private fun Field.isApplicable(): Boolean {
+            val name = name()
+            return name.startsWith(AsmUtil.CAPTURED_PREFIX) || name == AsmUtil.CAPTURED_THIS_FIELD
+        }
+
+        override fun addTo(existingVariables: ExistingVariables): DescriptorData<out ValueDescriptorImpl>? {
+            throw IllegalStateException("Should not be called on a container")
+        }
+    }
+
+    abstract fun addTo(existingVariables: ExistingVariables): DescriptorData<out ValueDescriptorImpl>?
+}
+
+internal fun attachCapturedValues(
+    containerValue: ObjectReference,
+    existingVariables: ExistingVariables,
+    collector: (DescriptorData<out ValueDescriptorImpl>) -> Unit
+) {
+    val values = collectPendingValues(PendingValue.Container(containerValue))
+
+    for (value in values) {
+        val descriptorData = value.addTo(existingVariables) ?: continue
+        collector(descriptorData)
+    }
+}
+
+private fun collectPendingValues(container: PendingValue.Container): List<PendingValue> {
+    val queue = ArrayDeque<PendingValue>()
+    queue.offer(container)
+    val values = mutableListOf<PendingValue>()
+    collectValuesBfs(queue, values)
+    return values
+}
+
+private tailrec fun collectValuesBfs(queue: Deque<PendingValue>, consumer: MutableList<PendingValue>) {
+    val deeperValues = ArrayDeque<PendingValue>()
+
+    while (queue.isNotEmpty()) {
+        val value = queue.removeFirst() ?: break
+
+        if (value is PendingValue.Container) {
+            val children = value.getChildren()
+            deeperValues.addAll(children)
+            continue
+        }
+
+        consumer += value
+    }
+
+    if (deeperValues.isNotEmpty()) {
+        collectValuesBfs(deeperValues, consumer)
+    }
+}
+
+private fun createPendingValue(container: PendingValue.Container, field: Field): PendingValue? {
+    val name = field.name()
+
+    if (name == AsmUtil.CAPTURED_THIS_FIELD) {
+        /*
+         * Captured entities.
+         * In case of captured lambda, we just add values captured to the lambda to the list.
+         * In case of captured this (outer this, for example), we add the 'this' value.
+         */
+        val value = container.value.getValue(field) as? ObjectReference ?: return null
+        return when (val label = getThisValueLabel(value)) {
+            null -> PendingValue.Container(value)
+            else -> PendingValue.This(label, value)
+        }
+    } else if (name.startsWith(AsmUtil.CAPTURED_LABELED_THIS_FIELD)) {
+        // Extension receivers for a new scheme ($this_<label>)
+        val value = container.value.getValue(field)
+        val label = name.drop(AsmUtil.CAPTURED_LABELED_THIS_FIELD.length).takeIf { it.isNotEmpty() } ?: return null
+        return PendingValue.This(label, value)
+    }
+
+    // Ordinary values (everything but 'this')
+    assert(name.startsWith(AsmUtil.CAPTURED_PREFIX))
+    val capturedValueName = name.drop(1).takeIf { it.isNotEmpty() } ?: return null
+    return PendingValue.Ordinary(capturedValueName, field, container)
+}

idea/jvm-debugger/jvm-debugger-core/src/org/jetbrains/kotlin/idea/debugger/stackFrame/ExistingVariables.kt

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.idea.debugger.stackFrame
+
+import com.intellij.debugger.jdi.LocalVariableProxyImpl
+
+internal sealed class ExistingVariable {
+    interface This
+    object UnlabeledThis : ExistingVariable(), This
+    data class LabeledThis(val label: String) : ExistingVariable(), This
+
+    data class Ordinary(val name: String) : ExistingVariable()
+}
+
+internal class ExistingVariables(thisVariables: List<LocalVariableProxyImpl>, ordinaryVariables: List<LocalVariableProxyImpl>) {
+    private val set = HashSet<ExistingVariable>()
+
+    var hasThisVariables: Boolean
+        private set
+
+    init {
+        thisVariables.forEach {
+            val thisVariable = it as? ThisLocalVariable ?: return@forEach
+            val label = thisVariable.label
+            val newExistingVariable: ExistingVariable = when {
+                label != null -> ExistingVariable.LabeledThis(label)
+                else -> ExistingVariable.UnlabeledThis
+            }
+            set.add(newExistingVariable)
+        }
+
+        hasThisVariables = thisVariables.isNotEmpty()
+
+        ordinaryVariables.forEach { set += ExistingVariable.Ordinary(it.name()) }
+    }
+
+    fun add(variable: ExistingVariable): Boolean {
+        val result = set.add(variable)
+        if (result && !hasThisVariables && variable is ExistingVariable.This) {
+            hasThisVariables = true
+        }
+        return result
+    }
+}

idea/jvm-debugger/jvm-debugger-core/src/org/jetbrains/kotlin/idea/debugger/stackFrame/KotlinStackFrame.kt

@@ -8,7 +8,6 @@ package org.jetbrains.kotlin.idea.debugger.stackFrame
 import com.intellij.debugger.engine.DebugProcessImpl
 import com.intellij.debugger.engine.JavaStackFrame
 import com.intellij.debugger.engine.JavaValue
-import com.intellij.debugger.engine.evaluation.EvaluateException
 import com.intellij.debugger.engine.evaluation.EvaluationContextImpl
 import com.intellij.debugger.jdi.LocalVariableProxyImpl
 import com.intellij.debugger.jdi.StackFrameProxyImpl
@@ -24,17 +23,6 @@ import org.jetbrains.kotlin.codegen.inline.INLINE_FUN_VAR_SUFFIX
 import org.jetbrains.kotlin.codegen.inline.isFakeLocalVariableForInline
 import org.jetbrains.kotlin.idea.debugger.*
 import org.jetbrains.kotlin.idea.debugger.evaluate.KotlinDebuggerEvaluator
-import java.util.*
-
-private sealed class ExistingVariable {
-    interface This
-    object UnlabeledThis : ExistingVariable(), This
-    data class LabeledThis(val label: String) : ExistingVariable(), This
-
-    data class Ordinary(val name: String) : ExistingVariable()
-}
-
-private typealias ExistingVariables = MutableSet<ExistingVariable>
 
 class KotlinStackFrame(frame: StackFrameProxyImpl) : JavaStackFrame(StackFrameDescriptorImpl(frame, MethodsTracker()), true) {
     private val kotlinVariableViewService = ToggleKotlinVariablesState.getService()
@@ -65,7 +53,7 @@ class KotlinStackFrame(frame: StackFrameProxyImpl) : JavaStackFrame(StackFrameDe
         val (thisVariables, otherVariables) = visibleVariables
             .partition { it.name() == THIS || it is ThisLocalVariable }
 
-        val existingVariables = getExistingVariables(thisVariables, otherVariables)
+        val existingVariables = ExistingVariables(thisVariables, otherVariables)
 
         if (!removeSyntheticThisObject(evaluationContext, children, existingVariables) && thisVariables.isNotEmpty()) {
             remapThisObjectForOuterThis(evaluationContext, children, existingVariables)
@@ -75,24 +63,6 @@ class KotlinStackFrame(frame: StackFrameProxyImpl) : JavaStackFrame(StackFrameDe
         otherVariables.forEach(::addItem)
     }
 
-    private fun getExistingVariables(
-        thisVariables: List<LocalVariableProxyImpl>,
-        ordinaryVariables: List<LocalVariableProxyImpl>
-    ): ExistingVariables {
-        val set = HashSet<ExistingVariable>()
-        thisVariables.forEach {
-            val thisVariable = it as? ThisLocalVariable ?: return@forEach
-            val label = thisVariable.label
-            val newExistingVariable: ExistingVariable = when {
-                label != null -> ExistingVariable.LabeledThis(label)
-                else -> ExistingVariable.UnlabeledThis
-            }
-            set.add(newExistingVariable)
-        }
-        ordinaryVariables.forEach { set += ExistingVariable.Ordinary(it.name()) }
-        return set
-    }
-
     private fun removeSyntheticThisObject(
         evaluationContext: EvaluationContextImpl,
         children: XValueChildrenList,
@@ -147,81 +117,13 @@ class KotlinStackFrame(frame: StackFrameProxyImpl) : JavaStackFrame(StackFrameDe
         existingVariables: ExistingVariables
     ) {
         val nodeManager = evaluationContext.debugProcess.xdebugProcess?.nodeManager ?: return
-        val capturedFields = containerValue.referenceType().safeFields()
-            .filter { field ->
-                val name = field.name()
-                name.startsWith(AsmUtil.CAPTURED_PREFIX) || name == AsmUtil.CAPTURED_THIS_FIELD
-            }
 
-        for (field in capturedFields) {
+        attachCapturedValues(containerValue, existingVariables) { valueData ->
-            val name = field.name()
-
-            if (name == AsmUtil.CAPTURED_THIS_FIELD) {
-                val value = containerValue.getValue(field) as? ObjectReference ?: continue
-                val label = getThisValueLabel(value)
-                if (label != null) {
-                    attachCapturedThis(evaluationContext, nodeManager, children, value, label, existingVariables)
-                } else {
-                    attachCapturedValues(evaluationContext, children, value, existingVariables)
-                }
-                continue
-            } else if (name.startsWith(AsmUtil.CAPTURED_LABELED_THIS_FIELD)) {
-                val value = containerValue.getValue(field)
-                val label = name.drop(AsmUtil.CAPTURED_LABELED_THIS_FIELD.length)
-                if (label.isNotEmpty()) {
-                    attachCapturedThis(evaluationContext, nodeManager, children, value, label, existingVariables)
-                }
-                continue
-            }
-
-            val capturedValueName = name.drop(1).takeIf { it.isNotEmpty() } ?: continue
-
-            if (!existingVariables.add(ExistingVariable.Ordinary(capturedValueName))) {
-                continue
-            }
-
-            val valueData = CapturedValueData(capturedValueName, containerValue, field)
             val valueDescriptor = nodeManager.getDescriptor(this.descriptor, valueData)
             children.add(JavaValue.create(null, valueDescriptor, evaluationContext, nodeManager, false))
         }
     }
 
-    private fun getThisValueLabel(thisValue: ObjectReference): String? {
-        val thisType = thisValue.referenceType()
-        val unsafeLabel = generateThisLabelUnsafe(thisType) ?: return null
-        return checkLabel(unsafeLabel)
-    }
-
-    private fun attachCapturedThis(
-        evaluationContext: EvaluationContextImpl,
-        nodeManager: NodeManagerImpl,
-        children: XValueChildrenList,
-        thisValue: Value?,
-        label: String,
-        existingVariables: ExistingVariables
-    ) {
-        try {
-            val hasThisVariables = existingVariables.any { it is ExistingVariable.This }
-
-            val thisName = if (hasThisVariables) {
-                if (!existingVariables.add(ExistingVariable.LabeledThis(label))) {
-                    // Avoid item duplication
-                    return
-                }
-
-                getThisName(label)
-            } else {
-                existingVariables.add(ExistingVariable.LabeledThis(label))
-                THIS
-            }
-
-            val thisDescriptor = nodeManager.getDescriptor(this.descriptor, LabeledThisData(label, thisName, thisValue))
-            children.add(JavaValue.create(null, thisDescriptor, evaluationContext, nodeManager, false))
-        } catch (e: EvaluateException) {
-            // do nothing
-        }
-    }
-
     private fun remapThisObjectForOuterThis(
         evaluationContext: EvaluationContextImpl,
         children: XValueChildrenList,
@@ -309,23 +211,6 @@ class KotlinStackFrame(frame: StackFrameProxyImpl) : JavaStackFrame(StackFrameDe
         }
     }
 
-    private fun generateThisLabel(type: Type?): String? {
-        return checkLabel(generateThisLabelUnsafe(type) ?: return null)
-    }
-
-    private fun generateThisLabelUnsafe(type: Type?): String? {
-        val referenceType = type as? ReferenceType ?: return null
-        return referenceType.name().substringAfterLast('.').substringAfterLast('$')
-    }
-
-    private fun checkLabel(label: String): String? {
-        if (label.isEmpty() || label.all { it.isDigit() }) {
-            return null
-        }
-
-        return label
-    }
-
     private fun LocalVariableProxyImpl.clone(name: String, label: String?): LocalVariableProxyImpl {
         return object : LocalVariableProxyImpl(frame, variable), ThisLocalVariable {
             override fun name() = name
@@ -339,7 +224,7 @@ class KotlinStackFrame(frame: StackFrameProxyImpl) : JavaStackFrame(StackFrameDe
     }
 }
 
-private interface ThisLocalVariable {
+interface ThisLocalVariable {
     val label: String?
 }
 
@@ -350,8 +235,4 @@ private fun LocalVariableProxyImpl.wrapSyntheticInlineVariable(): LocalVariableP
         }
     }
     return LocalVariableProxyImpl(proxyWrapper, variable)
-}
-
-private fun getThisName(label: String): String {
-    return "$THIS (@$label)"
 }

idea/jvm-debugger/jvm-debugger-core/src/org/jetbrains/kotlin/idea/debugger/stackFrame/thisLabelUtils.kt

@@ -0,0 +1,38 @@
+/*
+ * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.idea.debugger.stackFrame
+
+import com.sun.jdi.ObjectReference
+import com.sun.jdi.ReferenceType
+import com.sun.jdi.Type
+import org.jetbrains.kotlin.codegen.AsmUtil
+
+fun getThisName(label: String): String {
+    return AsmUtil.THIS + " (@" + label + ")"
+}
+
+fun getThisValueLabel(thisValue: ObjectReference): String? {
+    val thisType = thisValue.referenceType()
+    val unsafeLabel = generateThisLabelUnsafe(thisType) ?: return null
+    return checkLabel(unsafeLabel)
+}
+
+fun generateThisLabelUnsafe(type: Type?): String? {
+    val referenceType = type as? ReferenceType ?: return null
+    return referenceType.name().substringAfterLast('.').substringAfterLast('$')
+}
+
+fun generateThisLabel(type: Type?): String? {
+    return checkLabel(generateThisLabelUnsafe(type) ?: return null)
+}
+
+private fun checkLabel(label: String): String? {
+    if (label.isEmpty() || label.all { it.isDigit() }) {
+        return null
+    }
+
+    return label
+}

idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/capturedValues1.out

@@ -3,14 +3,14 @@ Run Java
 Connected to the target VM
 capturedValues1.kt:17
  frame    = invoke:17, CapturedValues1Kt$foo$1$1$1 {capturedValues1}
-   unknown  = this (@foo) = 1000
-   unknown  = args = {java.lang.String[0]@uniqueID}
-   unknown  = b = 2
    unknown  = this (@place1) = x
      field    = value: char[] = {char[1]@uniqueID} (sp = String.!EXT!)
        element  = 0 = 'x' 120
      field    = hash: int = 0 (sp = String.!EXT!)
    unknown  = c2 = 3
+   unknown  = b = 2
+   unknown  = this (@foo) = 1000
+   unknown  = args = {java.lang.String[0]@uniqueID}
    local    = this: java.lang.String = y (sp = null)
      field    = value: char[] = {char[1]@uniqueID} (sp = String.!EXT!)
        element  = 0 = 'y' 121

idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/capturedValues2.kt

@@ -1,27 +1,29 @@
 package capturedValues2
 
 fun main() {
-    1000.foo()
+    val a = 1
-}
 
-fun Int.foo() {
     block {
-        val b = 1
+        foo(a)
-        val b2 = 2
+        val a = 2
-        block("x") foo@ {
+
-            //Breakpoint!
+        block {
-            this@foo + b
+            foo(a)
+
+            val a = 3
+            block {
+                //Breakpoint!
+                foo(a)
+            }
         }
     }
 }
 
-fun block(block: () -> Unit) {
+inline fun block(block: () -> Unit) {
     block()
 }
 
-fun <T> block(obj: T, block: T.() -> Unit) {
+fun foo(foo: Int) {}
-    obj.block()
-}
 
 // SHOW_KOTLIN_VARIABLES
 // PRINT_FRAME

idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/capturedValues2.out

@@ -1,13 +1,9 @@
-LineBreakpoint created at capturedValues2.kt:13
+LineBreakpoint created at capturedValues2.kt:16
 Run Java
 Connected to the target VM
-capturedValues2.kt:13
+capturedValues2.kt:16
- frame    = invoke:13, CapturedValues2Kt$foo$1$1 {capturedValues2}
+ frame    = main:16, CapturedValues2Kt {capturedValues2}
-   unknown  = b = 1
+   local    = a: int = 3 (sp = capturedValues2.kt, 13)
-   local    = this: java.lang.String = x (sp = null)
-     field    = value: char[] = {char[1]@uniqueID} (sp = String.!EXT!)
-       element  = 0 = 'x' 120
-     field    = hash: int = 0 (sp = String.!EXT!)
 Disconnected from the target VM
 
 Process finished with exit code 0