From 6cd980765c49494a7a182bc4af39bfe54714b105 Mon Sep 17 00:00:00 2001 From: Yan Zhulanow Date: Fri, 21 Dec 2018 17:28:23 +0300 Subject: [PATCH] Do not issue exceptions for non-captured local variables This partly fixes KT-28192. --- .idea/dictionaries/yan.xml | 1 + .../debugger/evaluate/KotlinDebuggerCaches.kt | 3 +- .../evaluate/KotlinEvaluationBuilder.kt | 38 ++++++--- .../evaluate/ScopeCheckerForEvaluator.kt | 72 +++++++++++++++++ .../idea/debugger/evaluate/VariableFinder.kt | 78 +++++++++--------- .../nonCapturedVariables.kt | 79 +++++++++++++++++++ .../nonCapturedVariables.out | 32 ++++++++ .../frame/frameLambdaNotUsed.kt | 2 +- .../frame/frameLambdaNotUsed.out | 2 +- ...KotlinEvaluateExpressionTestGenerated.java | 5 ++ 10 files changed, 255 insertions(+), 57 deletions(-) create mode 100644 idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/ScopeCheckerForEvaluator.kt create mode 100644 idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/nonCapturedVariables.kt create mode 100644 idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/nonCapturedVariables.out diff --git a/.idea/dictionaries/yan.xml b/.idea/dictionaries/yan.xml index 06e1c2e0ecd..0c699ec80a2 100644 --- a/.idea/dictionaries/yan.xml +++ b/.idea/dictionaries/yan.xml @@ -9,6 +9,7 @@ parceler repl uast + unbox unboxed unmute diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinDebuggerCaches.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinDebuggerCaches.kt index 9bf908b4cef..be423439301 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinDebuggerCaches.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinDebuggerCaches.kt @@ -223,7 +223,8 @@ class KotlinDebuggerCaches(project: Project) { data class CompiledDataDescriptor( val classes: List, val sourcePosition: SourcePosition, - val parameters: List + val parameters: List, + val variablesCrossingInlineBounds: Set ) data class Parameter(val callText: String, val type: KotlinType, val value: Value? = null, val error: EvaluateException? = null) diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluationBuilder.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluationBuilder.kt index fc0466fd571..c1b9e061b7e 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluationBuilder.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluationBuilder.kt @@ -224,13 +224,15 @@ class KotlinEvaluator(val codeFragment: KtCodeFragment, val sourcePosition: Sour companion object { private fun extractAndCompile(codeFragment: KtCodeFragment, sourcePosition: SourcePosition, context: EvaluationContextImpl): CompiledDataDescriptor { - val (bindingContext) = codeFragment.checkForErrors() + var bindingContext = codeFragment.checkForErrors().bindingContext if (codeFragment.wrapToStringIfNeeded(bindingContext)) { // Repeat analysis with toString() added - codeFragment.checkForErrors() + bindingContext = codeFragment.checkForErrors().bindingContext } + val variablesCrossingInlineBounds = ScopeCheckerForEvaluator.checkScopes(bindingContext, codeFragment) + val extractionResult = getFunctionForExtractedFragment(codeFragment, sourcePosition.file, sourcePosition.line) ?: throw IllegalStateException("Code fragment cannot be extracted to function: ${codeFragment.text}") val (parametersDescriptor, extractedFunction) = try { @@ -259,9 +261,11 @@ class KotlinEvaluator(val codeFragment: KtCodeFragment, val sourcePosition: Sour val additionalFiles = outputFiles.map { ClassToLoad(getClassName(it.relativePath), it.relativePath, it.asByteArray()) } return CompiledDataDescriptor( - additionalFiles, - sourcePosition, - parametersDescriptor) + additionalFiles, + sourcePosition, + parametersDescriptor, + variablesCrossingInlineBounds + ) } private fun KtCodeFragment.wrapToStringIfNeeded(bindingContext: BindingContext): Boolean { @@ -326,7 +330,7 @@ class KotlinEvaluator(val codeFragment: KtCodeFragment, val sourcePosition: Sour // Prepare the main class val argumentTypes = Type.getArgumentTypes(methodToInvoke.desc) - val args = context.getArgumentsForEvaluation(compiledData.parameters, argumentTypes) + val args = context.getArgumentsForEvaluation(compiledData.parameters, argumentTypes, compiledData) .zip(argumentTypes) .map { (value, type) -> // Make argument type classes prepared for sure @@ -359,7 +363,7 @@ class KotlinEvaluator(val codeFragment: KtCodeFragment, val sourcePosition: Sour // Maybe just take the single method from the class, as it is done in 'evaluateWithCompilation' if (name == GENERATED_FUNCTION_NAME || name.startsWith(GENERATED_FUNCTION_NAME + "-")) { val argumentTypes = Type.getArgumentTypes(desc) - val args = context.getArgumentsForEvaluation(compiledData.parameters, argumentTypes) + val args = context.getArgumentsForEvaluation(compiledData.parameters, argumentTypes, compiledData) return object : MethodNode(Opcodes.API_VERSION, access, name, desc, signature, exceptions) { override fun visitEnd() { @@ -507,18 +511,28 @@ class KotlinEvaluator(val codeFragment: KtCodeFragment, val sourcePosition: Sour private fun EvaluationContextImpl.getArgumentsForEvaluation( parameters: List, - parameterTypes: Array + parameterTypes: Array, + compiledData: CompiledDataDescriptor ): List { val variableFinder = VariableFinder.instance(this) ?: error("No stack frame available") return parameters.zip(parameterTypes).map { (parameter, type) -> parameter.error?.let { throw it } + parameter.value?.let { return@map it } - val result = parameter.value ?: variableFinder.get(parameter.callText, type).asValue() + val name = parameter.callText + val result = variableFinder.find(name, type) - if (LOG.isDebugEnabled) { - LOG.debug("Parameter for eval4j: name = ${parameter.callText}, type = $type, value = $result") + if (result == null) { + if (name in compiledData.variablesCrossingInlineBounds) { + throw EvaluateExceptionUtil.createEvaluateException("'$name' is not captured") + } else { + throw VariableFinder.variableNotFound(this, buildString { + append("Cannot find local variable: name = '").append(name).append("', type = ").append(type.className) + }) + } + } else { + return@map result.value.asValue() } - result } } diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/ScopeCheckerForEvaluator.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/ScopeCheckerForEvaluator.kt new file mode 100644 index 00000000000..cf777fbf1ae --- /dev/null +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/ScopeCheckerForEvaluator.kt @@ -0,0 +1,72 @@ +/* + * Copyright 2010-2018 JetBrains s.r.o. 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.evaluate + +import com.intellij.psi.PsiElement +import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithVisibility +import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.idea.util.application.runReadAction +import org.jetbrains.kotlin.load.kotlin.toSourceElement +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.isAncestor +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.source.getPsi + +object ScopeCheckerForEvaluator { + fun checkScopes(bindingContext: BindingContext, codeFragment: KtCodeFragment): Set { + val result = hashSetOf() + + codeFragment.accept(object : KtTreeVisitor() { + override fun visitSimpleNameExpression(expression: KtSimpleNameExpression, data: Unit?): Void? { + val target = bindingContext[BindingContext.REFERENCE_TARGET, expression] + if (target is DeclarationDescriptorWithVisibility && target.visibility == Visibilities.LOCAL) { + val declarationPsiElement = target.toSourceElement.getPsi() + if (declarationPsiElement != null) { + runReadAction { + if (doesCrossInlineBounds(bindingContext, expression, declarationPsiElement)) { + result.add(expression.getReferencedName()) + } + } + } + } + + return null + } + }, Unit) + + return result + } + + private fun doesCrossInlineBounds(bindingContext: BindingContext, reference: KtSimpleNameExpression, declaration: PsiElement): Boolean { + val declarationParent = declaration.parent ?: return false + var currentParent: PsiElement? = reference.parent?.takeIf { it.isInside(declarationParent) } ?: return false + + while (currentParent != null && currentParent != declarationParent) { + if (currentParent is KtFunctionLiteral) { + val functionDescriptor = bindingContext[BindingContext.FUNCTION, currentParent] + if (functionDescriptor != null && !functionDescriptor.isInline) { + return true + } + } + + currentParent = when (currentParent) { + is KtCodeFragment -> currentParent.context + else -> currentParent.parent + } + } + + return false + } + + private tailrec fun PsiElement.isInside(parent: PsiElement): Boolean { + if (parent.isAncestor(this)) { + return true + } + + val context = (this.containingFile as? KtCodeFragment)?.context ?: return false + return context.isInside(parent) + } +} \ No newline at end of file diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/VariableFinder.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/VariableFinder.kt index 1e507c64f07..6b0c5a06fb7 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/VariableFinder.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/evaluate/VariableFinder.kt @@ -14,6 +14,7 @@ import com.sun.jdi.* import org.jetbrains.kotlin.codegen.AsmUtil import org.jetbrains.kotlin.codegen.AsmUtil.getCapturedFieldName import org.jetbrains.kotlin.codegen.AsmUtil.getLabeledThisName +import org.jetbrains.kotlin.codegen.coroutines.CONTINUATION_VARIABLE_NAME import org.jetbrains.kotlin.codegen.inline.INLINE_FUN_VAR_SUFFIX import org.jetbrains.kotlin.codegen.inline.INLINE_TRANSFORMATION_SUFFIX import org.jetbrains.kotlin.codegen.inline.NUMBERED_FUNCTION_PREFIX @@ -50,7 +51,40 @@ class VariableFinder private constructor(private val context: EvaluationContextI return NamedEntity("", value.type()) { value }.unwrap().value() } - private val inlinedThisRegex = getLocalVariableNameRegexInlineAware(AsmUtil.THIS + "_") + fun variableNotFound(context: EvaluationContextImpl, message: String): Exception { + val frameProxy = context.frameProxy + val location = frameProxy?.location() + val scope = context.debugProcess.searchScope + + val locationText = location?.run { "Location: ${sourceName()}:${lineNumber()}" } ?: "No location available" + + val sourceName = location?.sourceName() + val declaringTypeName = location?.declaringType()?.name()?.replace('.', '/')?.let { JvmClassName.byInternalName(it) } + + val sourceFile = if (sourceName != null && declaringTypeName != null) { + DebuggerUtils.findSourceFileForClassIncludeLibrarySources(context.project, scope, declaringTypeName, sourceName, location) + } else { + null + } + + val sourceFileText = runReadAction { sourceFile?.text } + + if (sourceName != null && sourceFileText != null) { + val attachments = mergeAttachments( + Attachment(sourceName, sourceFileText), + Attachment("location.txt", locationText) + ) + + LOG.error(message, attachments) + } + + return EvaluateExceptionUtil.createEvaluateException(message) + } + + // org.jetbrains.kotlin.codegen.inline.MethodInliner.prepareNode + private const val OUTER_THIS_FOR_INLINE = AsmUtil.THIS + '_' + + val inlinedThisRegex = getLocalVariableNameRegexInlineAware(OUTER_THIS_FOR_INLINE) private fun getCapturedVariableNameRegex(capturedName: String): Regex { val escapedName = Regex.escape(capturedName) @@ -168,17 +202,6 @@ class VariableFinder private constructor(private val context: EvaluationContextI } } - fun get(name: String, type: AsmType?): Value? { - val result = find(name, type) ?: throw variableNotFound(buildString { - append("Cannot find local variable: name = '").append(name).append("'") - if (type != null) { - append(", type = " + type.className) - } - }) - - return result.value - } - fun find(name: String, type: AsmType?): Result? { return when { name.startsWith(AsmUtil.THIS + "@") -> { @@ -317,7 +340,7 @@ class VariableFinder private constructor(private val context: EvaluationContextI name.startsWith(AsmUtil.LABELED_THIS_PARAMETER) || name == AsmUtil.RECEIVER_PARAMETER_NAME || name == AsmUtil.getCapturedFieldName(AsmUtil.THIS) - || inlinedThisRegex.matches(name) // org.jetbrains.kotlin.codegen.inline.MethodInliner.prepareNode + || inlinedThisRegex.matches(name) if (kind is VariableKind.LabeledThis) { variables.namedEntitySequence() @@ -366,35 +389,6 @@ class VariableFinder private constructor(private val context: EvaluationContextI || name == AsmUtil.CAPTURED_RECEIVER_FIELD } - private fun variableNotFound(message: String): Exception { - val location = frameProxy.location() - val scope = context.debugProcess.searchScope - - val locationText = location?.run { "Location: ${sourceName()}:${lineNumber()}" } ?: "No location available" - - val sourceName = location?.sourceName() - val declaringTypeName = location?.declaringType()?.name()?.replace('.', '/')?.let { JvmClassName.byInternalName(it) } - - val sourceFile = if (sourceName != null && declaringTypeName != null) { - DebuggerUtils.findSourceFileForClassIncludeLibrarySources(context.project, scope, declaringTypeName, sourceName, location) - } else { - null - } - - val sourceFileText = runReadAction { sourceFile?.text } - - if (sourceName != null && sourceFileText != null) { - val attachments = mergeAttachments( - Attachment(sourceName, sourceFileText), - Attachment("location.txt", locationText) - ) - - LOG.error(message, attachments) - } - - return EvaluateExceptionUtil.createEvaluateException(message) - } - private fun List.namedEntitySequence(owner: ObjectReference) = asSequence().map { NamedEntity.of(it, owner) } private fun List.namedEntitySequence() = asSequence().map { NamedEntity.of(it, frameProxy) } } \ No newline at end of file diff --git a/idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/nonCapturedVariables.kt b/idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/nonCapturedVariables.kt new file mode 100644 index 00000000000..3b9c02915ff --- /dev/null +++ b/idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/nonCapturedVariables.kt @@ -0,0 +1,79 @@ +package nonCapturedVariables + +fun main() { + val a = 5 + + inlineBlock { + //Breakpoint! + val b = 6 + + inlineBlock { + //Breakpoint! + val c = 7 + + block { + //Breakpoint! + val d = 8 + + inlineBlock { + //Breakpoint! + val e = 9 + + block { + //Breakpoint! + val f = 10 + + block { + //Breakpoint! + val g = 11 + + //Breakpoint! + val g2 = 12 + + //Breakpoint! + val g3 = 13 + + //Breakpoint! + val g4 = 14 + } + } + } + } + } + } +} + +private fun block(block: () -> Unit) { + block() +} + +private inline fun inlineBlock(block: () -> Unit) { + block() +} + +// EXPRESSION: a +// RESULT: 5: I + +// EXPRESSION: b +// RESULT: 6: I + +// EXPRESSION: c +// RESULT: 'c' is not captured + +// EXPRESSION: d +// RESULT: 8: I + +// EXPRESSION: e +// RESULT: 'e' is not captured + +// EXPRESSION: f +// RESULT: 'f' is not captured + +// EXPRESSION: e +// RESULT: 'e' is not captured + +// EXPRESSION: d +// RESULT: 'd' is not captured + +// EXPRESSION: c +// RESULT: 'c' is not captured \ No newline at end of file diff --git a/idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/nonCapturedVariables.out b/idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/nonCapturedVariables.out new file mode 100644 index 00000000000..6deaa573e11 --- /dev/null +++ b/idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/nonCapturedVariables.out @@ -0,0 +1,32 @@ +LineBreakpoint created at nonCapturedVariables.kt:8 +LineBreakpoint created at nonCapturedVariables.kt:12 +LineBreakpoint created at nonCapturedVariables.kt:16 +LineBreakpoint created at nonCapturedVariables.kt:20 +LineBreakpoint created at nonCapturedVariables.kt:24 +LineBreakpoint created at nonCapturedVariables.kt:28 +LineBreakpoint created at nonCapturedVariables.kt:31 +LineBreakpoint created at nonCapturedVariables.kt:34 +LineBreakpoint created at nonCapturedVariables.kt:37 +Run Java +Connected to the target VM +nonCapturedVariables.kt:8 +Compile bytecode for a +nonCapturedVariables.kt:12 +Compile bytecode for b +nonCapturedVariables.kt:16 +Compile bytecode for c +nonCapturedVariables.kt:20 +Compile bytecode for d +nonCapturedVariables.kt:24 +Compile bytecode for e +nonCapturedVariables.kt:28 +Compile bytecode for f +nonCapturedVariables.kt:31 +Compile bytecode for e +nonCapturedVariables.kt:34 +Compile bytecode for d +nonCapturedVariables.kt:37 +Compile bytecode for c +Disconnected from the target VM + +Process finished with exit code 0 diff --git a/idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/frameLambdaNotUsed.kt b/idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/frameLambdaNotUsed.kt index 803f555aa33..865b3eab4af 100644 --- a/idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/frameLambdaNotUsed.kt +++ b/idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/frameLambdaNotUsed.kt @@ -15,4 +15,4 @@ fun foo(f: () -> Unit) { // PRINT_FRAME // EXPRESSION: val1 -// RESULT: java.lang.AssertionError : Cannot find local variable: name = 'val1', type = int \ No newline at end of file +// RESULT: 'val1' is not captured \ No newline at end of file diff --git a/idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/frameLambdaNotUsed.out b/idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/frameLambdaNotUsed.out index 6755033c7b3..91f222c4172 100644 --- a/idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/frameLambdaNotUsed.out +++ b/idea/testData/debugger/tinyApp/src/evaluate/singleBreakpoint/frame/frameLambdaNotUsed.out @@ -20,7 +20,7 @@ fun foo(f: () -> Unit) { // PRINT_FRAME // EXPRESSION: val1 -// RESULT: java.lang.AssertionError : Cannot find local variable: name = 'val1', type = int +// RESULT: 'val1' is not captured frame = invoke:7, FrameLambdaNotUsedKt$main$1 {frameLambdaNotUsed} this = this = {frameLambdaNotUsed.FrameLambdaNotUsedKt$main$1@uniqueID}Function0 field = arity: int = 0 (sp = Lambda.!EXT!) diff --git a/idea/tests/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluateExpressionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluateExpressionTestGenerated.java index fe1aff4fb07..da430e6654c 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluateExpressionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/debugger/evaluate/KotlinEvaluateExpressionTestGenerated.java @@ -996,6 +996,11 @@ public class KotlinEvaluateExpressionTestGenerated extends AbstractKotlinEvaluat runTest("idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/multipleBreakpointsAtLine.kt"); } + @TestMetadata("nonCapturedVariables.kt") + public void testNonCapturedVariables() throws Exception { + runTest("idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/nonCapturedVariables.kt"); + } + @TestMetadata("privateMembersPriority.kt") public void testPrivateMembersPriority() throws Exception { runTest("idea/testData/debugger/tinyApp/src/evaluate/multipleBreakpoints/privateMembersPriority.kt");