From 38a54549ffd0fb0d1f1aba905f7f5c83fe6059a5 Mon Sep 17 00:00:00 2001 From: Kirill Rakhman Date: Wed, 26 Jul 2023 13:51:06 +0200 Subject: [PATCH] [FIR] Fix serialization of empty varargs in annotations in KLIBs #KT-59249 Fixed --- .../kotlin/test/LoadedMetadataDumpHandler.kt | 2 +- .../constant/ConstantValueUtils.kt | 33 ++++++++------ .../constant/FirToConstantValueTransformer.kt | 13 ++++-- .../fir/references/FirReferenceUtils.kt | 4 ++ .../StringArrayArgument.fir.k1.jvm.txt | 43 ++++++++++++++++++- .../StringArrayArgument.fir.k2.jvm.txt | 41 +++++++++++++++++- .../StringArrayArgument.fir.k2.klib.txt | 42 +++++++++++++++++- .../packageMembers/StringArrayArgument.kt | 26 ++++++++++- .../packageMembers/StringArrayArgument.txt | 12 ------ .../kotlin/kotlinp/test/KotlinpTestUtils.kt | 10 +++-- 10 files changed, 189 insertions(+), 37 deletions(-) delete mode 100644 compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.txt diff --git a/compiler/fir/analysis-tests/test/org/jetbrains/kotlin/test/LoadedMetadataDumpHandler.kt b/compiler/fir/analysis-tests/test/org/jetbrains/kotlin/test/LoadedMetadataDumpHandler.kt index 7d99300ffa1..5221526e1b6 100644 --- a/compiler/fir/analysis-tests/test/org/jetbrains/kotlin/test/LoadedMetadataDumpHandler.kt +++ b/compiler/fir/analysis-tests/test/org/jetbrains/kotlin/test/LoadedMetadataDumpHandler.kt @@ -147,7 +147,7 @@ abstract class AbstractLoadedMetadataDumpHandler } val emptyModule = TestModule( - name = "empty", module.targetPlatform, module.targetBackend, FrontendKinds.FIR, + name = "dump-${module.name}", module.targetPlatform, module.targetBackend, FrontendKinds.FIR, BackendKinds.IrBackend, module.binaryKind, files = emptyList(), allDependencies = listOf(DependencyDescription(module.name, dependencyKind, DependencyRelation.RegularDependency)), RegisteredDirectives.Empty, languageVersionSettings diff --git a/compiler/fir/fir-serialization/src/org/jetbrains/kotlin/fir/serialization/constant/ConstantValueUtils.kt b/compiler/fir/fir-serialization/src/org/jetbrains/kotlin/fir/serialization/constant/ConstantValueUtils.kt index 2615db0a865..4264744bfb0 100644 --- a/compiler/fir/fir-serialization/src/org/jetbrains/kotlin/fir/serialization/constant/ConstantValueUtils.kt +++ b/compiler/fir/fir-serialization/src/org/jetbrains/kotlin/fir/serialization/constant/ConstantValueUtils.kt @@ -6,15 +6,14 @@ package org.jetbrains.kotlin.fir.serialization.constant import org.jetbrains.kotlin.config.AnalysisFlags -import org.jetbrains.kotlin.constant.AnnotationValue -import org.jetbrains.kotlin.constant.ConstantValue -import org.jetbrains.kotlin.constant.ErrorValue -import org.jetbrains.kotlin.constant.KClassValue +import org.jetbrains.kotlin.constant.* import org.jetbrains.kotlin.fir.FirSession import org.jetbrains.kotlin.fir.declarations.FirValueParameter +import org.jetbrains.kotlin.fir.declarations.itOrExpectHasDefaultParameterValue import org.jetbrains.kotlin.fir.expressions.FirExpression import org.jetbrains.kotlin.fir.languageVersionSettings import org.jetbrains.kotlin.fir.render +import org.jetbrains.kotlin.fir.symbols.impl.FirFunctionSymbol import org.jetbrains.kotlin.fir.types.* import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.utils.addToStdlib.runIf @@ -22,24 +21,34 @@ import org.jetbrains.kotlin.utils.addToStdlib.runIf internal fun Map.convertToConstantValues( session: FirSession, constValueProvider: ConstValueProvider? -): Map> { - return this.map { (name, firExpression) -> - val constantValue = constValueProvider?.findConstantValueFor(firExpression) +): MutableMap> { + return this.mapValuesTo(mutableMapOf()) { (_, firExpression) -> + constValueProvider?.findConstantValueFor(firExpression) ?: firExpression.toConstantValue(session, constValueProvider) ?: runIf(session.languageVersionSettings.getFlag(AnalysisFlags.metadataCompilation)) { ErrorValue.ErrorValueWithMessage("Constant conversion can be ignored in metadata compilation mode") } ?: error("Cannot convert expression ${firExpression.render()} to constant") - name to constantValue - }.toMap() + } +} + +internal fun MutableMap>.addEmptyVarargValuesFor( + symbol: FirFunctionSymbol<*>?, +): MutableMap> = apply { + if (symbol == null) return@apply + for ((i, parameter) in symbol.valueParameterSymbols.withIndex()) { + if (parameter.name !in this && parameter.isVararg && !symbol.fir.itOrExpectHasDefaultParameterValue(i)) { + this[parameter.name] = ArrayValue(emptyList()) + } + } } internal fun LinkedHashMap.convertToConstantValues( session: FirSession, constValueProvider: ConstValueProvider?, -): Map> { - return this.map { (firExpression, firValueParameter) -> firValueParameter.name to firExpression } - .toMap().convertToConstantValues(session, constValueProvider) +): MutableMap> { + return this.entries.associate { (firExpression, firValueParameter) -> firValueParameter.name to firExpression } + .convertToConstantValues(session, constValueProvider) } inline fun AnnotationValue.coneTypeSafe(): T? { diff --git a/compiler/fir/fir-serialization/src/org/jetbrains/kotlin/fir/serialization/constant/FirToConstantValueTransformer.kt b/compiler/fir/fir-serialization/src/org/jetbrains/kotlin/fir/serialization/constant/FirToConstantValueTransformer.kt index 4a925ae1030..6bf1445babd 100644 --- a/compiler/fir/fir-serialization/src/org/jetbrains/kotlin/fir/serialization/constant/FirToConstantValueTransformer.kt +++ b/compiler/fir/fir-serialization/src/org/jetbrains/kotlin/fir/serialization/constant/FirToConstantValueTransformer.kt @@ -15,6 +15,7 @@ import org.jetbrains.kotlin.fir.declarations.FirField import org.jetbrains.kotlin.fir.declarations.utils.isConst import org.jetbrains.kotlin.fir.declarations.utils.isFinal import org.jetbrains.kotlin.fir.expressions.* +import org.jetbrains.kotlin.fir.references.toResolvedFunctionSymbol import org.jetbrains.kotlin.fir.render import org.jetbrains.kotlin.fir.resolve.toFirRegularClassSymbol import org.jetbrains.kotlin.fir.resolve.transformers.body.resolve.FirArrayOfCallTransformer @@ -101,9 +102,12 @@ internal object FirToConstantValueTransformer : FirDefaultVisitor { - val mapping = annotation.argumentMapping.mapping.convertToConstantValues(data.session, data.constValueProvider) + val mapping = annotation.argumentMapping.mapping.convertToConstantValues( + data.session, + data.constValueProvider + ).addEmptyVarargValuesFor(annotation.toReference()?.toResolvedFunctionSymbol()) return AnnotationValue.create(annotation.annotationTypeRef.coneType, mapping) } @@ -149,7 +153,10 @@ internal object FirToConstantValueTransformer : FirDefaultVisitor(discardErrorReference) } +fun FirReference.toResolvedFunctionSymbol(discardErrorReference: Boolean = false): FirFunctionSymbol<*>? { + return this.toResolvedSymbol>(discardErrorReference) +} + fun FirReference.toResolvedNamedFunctionSymbol(discardErrorReference: Boolean = false): FirNamedFunctionSymbol? { return this.toResolvedSymbol(discardErrorReference) } diff --git a/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k1.jvm.txt b/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k1.jvm.txt index fa0e6675255..6b1482327a7 100644 --- a/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k1.jvm.txt +++ b/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k1.jvm.txt @@ -1,7 +1,9 @@ +Module: common +Module: platform field:@FIELD:R|test/Anno|(t = (String(prosper))) public final val bar: R|kotlin/Int| public get(): R|kotlin/Int| -@R|test/Anno|(t = ()) public final fun baz(): R|kotlin/Unit| +@R|test/Anno|(t = ()) @R|test/Anno2|() @R|test/Anno3|() @R|test/Anno4|(t = ()) @R|test/Anno5|(t = ()) @R|test/Anno6|(t = ()) public final fun baz(): R|kotlin/Unit| @R|test/Anno|(t = (String(live), String(long))) public final fun foo(): R|kotlin/Unit| @@ -13,3 +15,42 @@ public final annotation class Anno : R|kotlin/Annotation| { } +public final annotation class Anno2 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| = () + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno2| + +} + +public final annotation class Anno3 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| = (String(a)) + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno3| + +} + +public final annotation class Anno4 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array|): R|test/Anno4| + +} + +public final annotation class Anno5 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| = () + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno5| + +} + +public final annotation class Anno6 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| = (String(a)) + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno6| + +} diff --git a/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k2.jvm.txt b/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k2.jvm.txt index fa0e6675255..dcfeb39a7f7 100644 --- a/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k2.jvm.txt +++ b/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k2.jvm.txt @@ -1,7 +1,7 @@ field:@FIELD:R|test/Anno|(t = (String(prosper))) public final val bar: R|kotlin/Int| public get(): R|kotlin/Int| -@R|test/Anno|(t = ()) public final fun baz(): R|kotlin/Unit| +@R|test/Anno|(t = ()) @R|test/Anno2|() @R|test/Anno3|() @R|test/Anno4|(t = ()) @R|test/Anno5|() @R|test/Anno6|() public final fun baz(): R|kotlin/Unit| @R|test/Anno|(t = (String(live), String(long))) public final fun foo(): R|kotlin/Unit| @@ -13,3 +13,42 @@ public final annotation class Anno : R|kotlin/Annotation| { } +public final annotation class Anno2 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| = () + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno2| + +} + +public final annotation class Anno3 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| = (String(a)) + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno3| + +} + +public final annotation class Anno4 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array|): R|test/Anno4| + +} + +public final annotation class Anno5 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| = () + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno5| + +} + +public final annotation class Anno6 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| = (String(a)) + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno6| + +} diff --git a/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k2.klib.txt b/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k2.klib.txt index 7656f893861..5e12d22d195 100644 --- a/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k2.klib.txt +++ b/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.fir.k2.klib.txt @@ -1,7 +1,7 @@ public final val bar: R|kotlin/Int| public get(): R|kotlin/Int| -@R|test/Anno|() public final fun baz(): R|kotlin/Unit| +@R|test/Anno|(t = ()) @R|test/Anno2|() @R|test/Anno3|() @R|test/Anno4|(t = ()) @R|test/Anno5|() @R|test/Anno6|() public final fun baz(): R|kotlin/Unit| @R|test/Anno|(t = (String(live), String(long))) public final fun foo(): R|kotlin/Unit| @@ -12,3 +12,43 @@ public final annotation class Anno : R|kotlin/Annotation| { public constructor(vararg t: R|kotlin/Array|): R|test/Anno| } + +public final annotation class Anno2 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno2| + +} + +public final annotation class Anno3 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno3| + +} + +public final annotation class Anno4 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array|): R|test/Anno4| + +} + +public final annotation class Anno5 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno5| + +} + +public final annotation class Anno6 : R|kotlin/Annotation| { + public final val t: R|kotlin/Array| + public get(): R|kotlin/Array| + + public constructor(vararg t: R|kotlin/Array| = STUB): R|test/Anno6| + +} diff --git a/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.kt b/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.kt index 66c5a21233d..b4be1de8e24 100644 --- a/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.kt +++ b/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.kt @@ -1,11 +1,33 @@ // PLATFORM_DEPENDANT_METADATA -//ALLOW_AST_ACCESS +// ALLOW_AST_ACCESS +// NO_CHECK_SOURCE_VS_BINARY +// IGNORE_BACKEND_K1: JVM_IR, ANY +// LANGUAGE: +MultiPlatformProjects +// IGNORE K1 +// ^mute KotlinpCompilerTestDataTest +// K1/K2 difference: KT-60820 + +// MODULE: common +// FILE: common.kt +package test + +expect annotation class Anno4(vararg val t: String) +expect annotation class Anno5(vararg val t: String = []) +expect annotation class Anno6(vararg val t: String = ["a"]) + +// MODULE: platform()()(common) +// FILE: test.kt package test annotation class Anno(vararg val t: String) +annotation class Anno2(vararg val t: String = []) +annotation class Anno3(vararg val t: String = ["a"]) +actual annotation class Anno4(vararg val t: String) +actual annotation class Anno5(vararg val t: String) +actual annotation class Anno6(vararg val t: String) @Anno("live", "long") fun foo() {} @field:Anno("prosper") val bar = { 42 }() -@Anno() fun baz() {} +@Anno() @Anno2() @Anno3() @Anno4() @Anno5() @Anno6() fun baz() {} diff --git a/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.txt b/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.txt deleted file mode 100644 index 61eca888132..00000000000 --- a/compiler/testData/loadJava/compiledKotlin/annotations/packageMembers/StringArrayArgument.txt +++ /dev/null @@ -1,12 +0,0 @@ -package test - -@field:test.Anno(t = {"prosper"}) public val bar: kotlin.Int - public fun ``(): kotlin.Int -@test.Anno(t = {}) public fun baz(): kotlin.Unit -@test.Anno(t = {"live", "long"}) public fun foo(): kotlin.Unit - -public final annotation class Anno : kotlin.Annotation { - /*primary*/ public constructor Anno(/*0*/ vararg t: kotlin.String /*kotlin.Array*/) - public final val t: kotlin.Array - public final fun ``(): kotlin.Array -} diff --git a/libraries/tools/kotlinp/test/org/jetbrains/kotlin/kotlinp/test/KotlinpTestUtils.kt b/libraries/tools/kotlinp/test/org/jetbrains/kotlin/kotlinp/test/KotlinpTestUtils.kt index b8d52181f68..9d5564521a5 100644 --- a/libraries/tools/kotlinp/test/org/jetbrains/kotlin/kotlinp/test/KotlinpTestUtils.kt +++ b/libraries/tools/kotlinp/test/org/jetbrains/kotlin/kotlinp/test/KotlinpTestUtils.kt @@ -27,6 +27,7 @@ import org.jetbrains.kotlin.test.util.KtTestUtil import java.io.File import kotlin.test.fail +private const val IGNORE_K1_DIRECTIVE = "// IGNORE K1" private const val IGNORE_K2_DIRECTIVE = "// IGNORE K2" fun compareAllFiles( @@ -37,15 +38,16 @@ fun compareAllFiles( readWriteAndCompare: Boolean, useK2: Boolean, ) { - val isMutedForK2 = useK2 && InTextDirectivesUtils.findStringWithPrefixes(file.readText(), IGNORE_K2_DIRECTIVE) != null + val directive = if (useK2) IGNORE_K2_DIRECTIVE else IGNORE_K1_DIRECTIVE + val isMuted = InTextDirectivesUtils.findStringWithPrefixes(file.readText(), directive) != null try { compileAndPrintAllFiles(file, disposable, tmpdir, compareWithTxt, readWriteAndCompare, useK2) } catch (e: Throwable) { - if (isMutedForK2) return + if (isMuted) return throw e } - if (isMutedForK2) { - throw AssertionError("Looks like this test can be unmuted. Remove the \"$IGNORE_K2_DIRECTIVE\" directive.") + if (isMuted) { + throw AssertionError("Looks like this test can be unmuted. Remove the \"$directive\" directive.") } }