From 5b16b2c71e5385a8cb44ac37f9542d268f812403 Mon Sep 17 00:00:00 2001 From: Ivan Kylchik Date: Fri, 21 Oct 2022 15:44:38 +0200 Subject: [PATCH] Properly verify args in string concatenation expression for interpreter If string concatenation contains object value, then this argument will not have explicit toString call. We must find and check toString method manually. #KT-54615 --- ...CompilerTestFE10TestdataTestGenerated.java | 6 +++ ...irOldFrontendDiagnosticsTestGenerated.java | 6 +++ ...DiagnosticsWithLightTreeTestGenerated.java | 6 +++ .../fir/analysis/checkers/FirConstChecks.kt | 6 ++- .../FirBlackBoxCodegenTestGenerated.java | 6 +++ .../ir/interpreter/checker/EvaluationMode.kt | 2 +- .../checker/IrCompileTimeChecker.kt | 25 +++++++++--- .../interpreter/checker/IrConstTransformer.kt | 6 ++- .../evaluate/stringConcatenationWithObject.kt | 15 +++++++ .../const/stringConcatenationWithObject.kt | 18 +++++++++ .../const/stringConcatenationWithObject.txt | 40 +++++++++++++++++++ .../test/runners/DiagnosticTestGenerated.java | 6 +++ .../IrBlackBoxCodegenTestGenerated.java | 6 +++ 13 files changed, 139 insertions(+), 9 deletions(-) create mode 100644 compiler/testData/codegen/box/evaluate/stringConcatenationWithObject.kt create mode 100644 compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.kt create mode 100644 compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.txt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java index 0fec67f2b95..7f5a8438f6b 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java @@ -20495,6 +20495,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag runTest("compiler/testData/diagnostics/tests/modifiers/const/stdlibConstFun.kt"); } + @Test + @TestMetadata("stringConcatenationWithObject.kt") + public void testStringConcatenationWithObject() throws Exception { + runTest("compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.kt"); + } + @Test @TestMetadata("types.kt") public void testTypes() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java index d87541b50b0..41d57926d7c 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java @@ -20495,6 +20495,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti runTest("compiler/testData/diagnostics/tests/modifiers/const/stdlibConstFun.kt"); } + @Test + @TestMetadata("stringConcatenationWithObject.kt") + public void testStringConcatenationWithObject() throws Exception { + runTest("compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.kt"); + } + @Test @TestMetadata("types.kt") public void testTypes() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java index afb8cab5d36..fe60f751364 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java @@ -20495,6 +20495,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac runTest("compiler/testData/diagnostics/tests/modifiers/const/stdlibConstFun.kt"); } + @Test + @TestMetadata("stringConcatenationWithObject.kt") + public void testStringConcatenationWithObject() throws Exception { + runTest("compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.kt"); + } + @Test @TestMetadata("types.kt") public void testTypes() throws Exception { diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirConstChecks.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirConstChecks.kt index d617d28f97f..58cdc761502 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirConstChecks.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirConstChecks.kt @@ -66,8 +66,12 @@ internal fun checkConstantArguments( return checkConstantArguments(expression.compareToCall, session) } expression is FirStringConcatenationCall || expression is FirEqualityOperatorCall -> { - for (exp in (expression as FirCall).arguments) + for (exp in (expression as FirCall).arguments) { + if (exp is FirResolvedQualifier) { + return ConstantArgumentKind.NOT_CONST + } checkConstantArguments(exp, session).let { return it } + } } expression is FirGetClassCall -> { var coneType = (expression as? FirCall)?.argument?.typeRef?.coneType diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index 8a29a6b077b..aa204b1a2f8 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -17033,6 +17033,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/evaluate/simpleCallBinary.kt"); } + @Test + @TestMetadata("stringConcatenationWithObject.kt") + public void testStringConcatenationWithObject() throws Exception { + runTest("compiler/testData/codegen/box/evaluate/stringConcatenationWithObject.kt"); + } + @Test @TestMetadata("thisPlusString.kt") public void testThisPlusString() throws Exception { diff --git a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/EvaluationMode.kt b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/EvaluationMode.kt index 928d3a11e6e..087bc26c4fd 100644 --- a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/EvaluationMode.kt +++ b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/EvaluationMode.kt @@ -103,7 +103,7 @@ enum class EvaluationMode(protected val mustCheckBody: Boolean) { abstract fun canEvaluateEnumValue(enumEntry: IrGetEnumValue, context: IrCall? = null): Boolean abstract fun canEvaluateReference(reference: IrCallableReference<*>, context: IrCall? = null): Boolean - fun canEvaluateBody(function: IrFunction): Boolean { + fun mustCheckBodyOf(function: IrFunction): Boolean { if (function is IrSimpleFunction && function.correspondingPropertySymbol != null) return true return (mustCheckBody || function.isLocal) && !function.isContract() && !function.isMarkedAsEvaluateIntrinsic() } diff --git a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/IrCompileTimeChecker.kt b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/IrCompileTimeChecker.kt index b04e989ba52..a262433b69c 100644 --- a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/IrCompileTimeChecker.kt +++ b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/IrCompileTimeChecker.kt @@ -54,11 +54,14 @@ class IrCompileTimeChecker( private fun visitConstructor(expression: IrFunctionAccessExpression): Boolean { return when { !visitValueParameters(expression, null) || !mode.canEvaluateFunction(expression.symbol.owner, contextExpression) -> false - mode.canEvaluateBody(expression.symbol.owner) -> expression.symbol.owner.body?.accept(this, null) != false - else -> true + else -> expression.symbol.owner.visitBodyIfNeeded() } } + private fun IrFunction.visitBodyIfNeeded(): Boolean { + return this.asVisited { !mode.mustCheckBodyOf(this) || (this.body?.accept(this@IrCompileTimeChecker, null) ?: true) } + } + override fun visitCall(expression: IrCall, data: Nothing?): Boolean { val owner = expression.symbol.owner if (!mode.canEvaluateFunction(owner, expression)) return false @@ -67,7 +70,7 @@ class IrCompileTimeChecker( val dispatchReceiverComputable = expression.dispatchReceiver?.accept(this, null) ?: true val extensionReceiverComputable = expression.extensionReceiver?.accept(this, null) ?: true if (!visitValueParameters(expression, null)) return@saveContext false - val bodyComputable = owner.asVisited { if (mode.canEvaluateBody(owner)) owner.body?.accept(this, null) ?: true else true } + val bodyComputable = owner.visitBodyIfNeeded() return@saveContext dispatchReceiverComputable && extensionReceiverComputable && bodyComputable } } @@ -126,7 +129,19 @@ class IrCompileTimeChecker( } override fun visitStringConcatenation(expression: IrStringConcatenation, data: Nothing?): Boolean { - return expression.arguments.all { it.accept(this, data) } + return expression.arguments.all { arg -> + when (arg) { + is IrGetObjectValue -> { + val toString = arg.symbol.owner.declarations + .filterIsInstance() + .single { it.name.asString() == "toString" && it.valueParameters.isEmpty() } + + mode.canEvaluateFunction(toString, null) && toString.visitBodyIfNeeded() + } + + else -> arg.accept(this, data) + } + } } override fun visitGetObjectValue(expression: IrGetObjectValue, data: Nothing?): Boolean { @@ -218,7 +233,7 @@ class IrCompileTimeChecker( if (!mode.canEvaluateFunction(owner, contextExpression)) return false - val bodyComputable = owner.asVisited { if (mode.canEvaluateBody(owner)) owner.body?.accept(this, null) ?: true else true } + val bodyComputable = owner.visitBodyIfNeeded() return dispatchReceiverComputable && extensionReceiverComputable && bodyComputable } diff --git a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/IrConstTransformer.kt b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/IrConstTransformer.kt index b7719476e7f..b529f9cc6ee 100644 --- a/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/IrConstTransformer.kt +++ b/compiler/ir/ir.interpreter/src/org/jetbrains/kotlin/ir/interpreter/checker/IrConstTransformer.kt @@ -55,8 +55,10 @@ class IrConstTransformer( val result = try { interpreter.interpret(this, irFile) } catch (e: Throwable) { - if (!suppressExceptions) throw AssertionError("Error occurred while optimizing an expression:\n${this.dump()}", e) - return IrErrorExpressionImpl(UNDEFINED_OFFSET, UNDEFINED_OFFSET, this.type, e.message.toString()).warningIfError(this) + if (suppressExceptions) { + return this + } + throw AssertionError("Error occurred while optimizing an expression:\n${this.dump()}", e) } return if (failAsError) result.reportIfError(this) else result.warningIfError(this) diff --git a/compiler/testData/codegen/box/evaluate/stringConcatenationWithObject.kt b/compiler/testData/codegen/box/evaluate/stringConcatenationWithObject.kt new file mode 100644 index 00000000000..d8fac86057c --- /dev/null +++ b/compiler/testData/codegen/box/evaluate/stringConcatenationWithObject.kt @@ -0,0 +1,15 @@ +// TARGET_BACKEND: JVM_IR + +object K : Code("K") + +open class Code(val x: String) { + override fun toString() = "$x" +} + +class O { + companion object: Code("O") +} + +fun box(): String { + return "$O" + "$K" // must not be evaluated during compile time +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.kt b/compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.kt new file mode 100644 index 00000000000..d45298b5856 --- /dev/null +++ b/compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.kt @@ -0,0 +1,18 @@ +// FIR_IDENTICAL + +object O : Code(0) + +open class Code(val x: Int) { + override fun toString() = "$x" +} + +class A { + companion object: Code(0) +} + +const val toString1 = O.toString() +const val toString2 = A.toString() +const val plusString1 = "string" + O +const val plusString2 = "string" + A +const val stringConcat1 = "$O" +const val stringConcat2 = "$A" diff --git a/compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.txt b/compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.txt new file mode 100644 index 00000000000..aa9727da207 --- /dev/null +++ b/compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.txt @@ -0,0 +1,40 @@ +package + +public const val plusString1: kotlin.String +public const val plusString2: kotlin.String +public const val stringConcat1: kotlin.String +public const val stringConcat2: kotlin.String +public const val toString1: kotlin.String +public const val toString2: kotlin.String + +public final class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + public companion object Companion : Code { + private constructor Companion() + public final override /*1*/ /*fake_override*/ val x: kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } +} + +public open class Code { + public constructor Code(/*0*/ x: kotlin.Int) + public final val x: kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ fun toString(): kotlin.String +} + +public object O : Code { + private constructor O() + public final override /*1*/ /*fake_override*/ val x: kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index 74a2c0c56c4..7a8b0657b06 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -20501,6 +20501,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/modifiers/const/stdlibConstFun.kt"); } + @Test + @TestMetadata("stringConcatenationWithObject.kt") + public void testStringConcatenationWithObject() throws Exception { + runTest("compiler/testData/diagnostics/tests/modifiers/const/stringConcatenationWithObject.kt"); + } + @Test @TestMetadata("types.kt") public void testTypes() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index c8f0da0f234..68e1c31dc4a 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -17033,6 +17033,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/evaluate/simpleCallBinary.kt"); } + @Test + @TestMetadata("stringConcatenationWithObject.kt") + public void testStringConcatenationWithObject() throws Exception { + runTest("compiler/testData/codegen/box/evaluate/stringConcatenationWithObject.kt"); + } + @Test @TestMetadata("thisPlusString.kt") public void testThisPlusString() throws Exception {