Kotlin / KEEP

Kotlin Evolution and Enhancement Process
Apache License 2.0
3.29k stars 357 forks source link

Multi-field value classes #340

Open zhelenskiy opened 1 year ago

zhelenskiy commented 1 year ago

Discussion of the proposal: Multi-field value classes.

edrd-f commented 1 year ago

The link is broken. Here's a working one: https://github.com/Kotlin/KEEP/blob/dff9e07e87fbb96e38a226943e7ee302f6149042/proposals/multi-field-value-classes.md

zhelenskiy commented 1 year ago

It will become fixed automatically when the KEEP gets accepted. But I'll take the current one for now.

qwwdfsad commented 1 year ago

It is unclear from the design rationale why this feature should be implemented before Valhalla: it carries not that many benefits (compared i.e. to benefits regular value classes have brought), while exposing quite a lot of implementation and design burdens.

The performance impact is likely to be negligible (moreover, people may start to over-rely on inline, producing much more bloated apps), and it's unclear from use cases why one may choose to use MFVC compared to regular [data] classes. The small benefits of being identity-less cannot be enforced deterministically due to boxing, and everything else is a matter of code-style or much less intrusive compiler plugin (that probably can be considered as an alternative), while the weight of the feature itself is significant

zhelenskiy commented 1 year ago

@qwwdfsad It is already listed there: KEEP. What do you mean when you say "regular value classes"?

I haven't published the results of benchmarks, because I have done only micro ones yet. But the results were quite encouraging, especially for Android. The feature is already partly implemented in compiler and I see no way and reason for it to be a plugin.

I also didn't understand what you meant by the last sentence.

qwwdfsad commented 1 year ago

What do you mean when you say "regular value classes"?

Backed by a single field.

But the results were quite encouraging, especially for Android.

I believe that these results might indeed be important for the final decision about this feature. For now, it's totally unclear what benefits it brings to the users except for the lack of identity.

The feature is already partly implemented in compiler and I see no way and reason for it to be a plugin.

I'm not sure this is proper reasoning to introduce a feature to the language.

zhelenskiy commented 1 year ago

Ok, I'll publish the benchmark results as soon, as I get them all.

mcpiroman commented 1 year ago

Shallow immutability is chosen to make it possible to implement internally mutable classes with immutable public API such as Lazy.

How MFVC can help Lazy<T>? Shallow immutability means all fields like value need to be vals and if there is some box class with mutable field it would then defeat the purpose of value classes.

zhelenskiy commented 1 year ago

@mcpiroman I fixed the paragraph

francescotescari commented 1 year ago

Any updates on the benchmarks?

It is an interesting proposal, I would expect this to be effective also in Kotlin Native where the compiler could do some interesting optimizations with the MFVC

zhelenskiy commented 1 year ago

@francescotescari The development is actually in progress, I just don't upgrade the proposal text for now.

efemoney commented 10 months ago

"Prototype: experimental since 1.8.20"

How can we test this 👀

zhelenskiy commented 10 months ago

You can add ValueClasses flag. I'd recommend 1.9.0-Beta instead. But there is absolutely no guaranty for the feature for now.

k163377 commented 7 months ago

From the standpoint of having promoted value class support in jackson-module-kotlin, I am not convinced that this feature will be good, especially in terms of Java integration and reflection.

value class was supposed to have been stabilized a long time ago, but important issues with reflection (e.g. KT-57357) have been left unresolved. Several important problems with function calls were also left for a long time until I fixed them. Also, due to the lack of Java integration capabilities, not only jackson-module-kotlin, but even many leading libraries/frameworks such as moshi and Spring still cannot support value class.

When using reflection, the value class is also inferior to the data class in terms of performance. Even looking at kotlin-reflect alone, there is the overhead of boxing and using ValueClassAwareCaller. Also, Java reflection-based libraries require more reflection for every parsing. These can easily cancel out the internal performance gains. In fact, there are benchmarks that suggest performance degradation when using value class in jackson-module-kotlin.

In this situation, is it not wrong to add more complex functionality for a limited number of use cases? Wouldn't it be better to have the functionality needed for the existing value class first?

At the risk of sounding rude, I don't think the Kotlin development team is properly handling the complexity brought by the value class. Reflection and integration with Java is very important to many Kotlin users, seems to be severely neglected.

k163377 commented 7 months ago

I have tried a few things, but personally I think we should avoid using - as a separator for unboxed getter names. I don't know if this is feasible, but as much as possible we should use characters that the user cannot type.

When processing in a Java reflection-based library, there may be a need to exclude these getters from processing. This process is easier to implement if you can exclude them by looking at just the getter name.

The - is unwieldy in that it can be entered by the user and is also used in mangled getters for single field value class.

zhelenskiy commented 7 months ago

From the standpoint of having promoted value class support in jackson-module-kotlin, I am not convinced that this feature will be good, especially in terms of Java integration and reflection.

What type of integration is needed? The existing bugs are kept because of issues with higher priority to be fixed first (not connected with MFVC, of course: it has much lower priority), these bugs are not undoable. The mentioned bug has even a PR for a while that is not yet merged. (I am going to fix it this week.)

Value classes (both single and multi-field) are not a silver bullet in any language with dynamic polymorphism. They are good at cases when they are used monomorphically and bad when used polymorphically. And the reflection is the latter case.

Most of the human resources are spent on the K2 development, that is the reason of the time. However, thank you for the feedback — it will raise the priority of the bugs.

- is only used in the getters that should not be used from the code. That is why it was chosen. If you want to exclude getMfvc and all its subgetters, you can exclude getMfvc-.* getters as well.

k163377 commented 7 months ago

I can only read and write English by machine translation, so I apologize if there are any mistakes or rudeness.

What type of integration is needed?

Everything needs to be done to make constructors and methods look like normal classes on the JVM. In addition, we need to make sure that performance is not degraded when handling from reflection. Without these, the value class will either not be fully supported or, if supported, will suffer performance degradation and lose its value.

You should also ask other maintainers who have Kotlin support in their Java reflection based libraries. There must be a reason why support has not progressed this long after its release, even though it offers some attractive features.

Finally, please increase the amount of testing. Even the simpler SFVC lacks many tests for reflection, resulting in "stable" releases with fatal bugs. I helped to add about 3,000 lines of testing, but this was still not enough. https://github.com/JetBrains/kotlin/pull/4743

- is only used in the getters that should not be used from the code. That is why it was chosen. If you want to exclude getMfvc and all its subgetters, you can exclude getMfvc-.* getters as well.

Just off the top of my head, it seems to me that we can't simply exclude the case where a function is defined with a similar prefix.

Wouldn't it be more reasonable to use characters that users would not normally use? I am not aware of all use cases, but at the very least, - is unwieldy because it is commonly used as a delimiter. Even jackson-module-kotlin actually had special handling for getters containing -, but issues were reported by users who wanted to use properties containing -.

zhelenskiy commented 7 months ago

They already look like normal classes in Kotlin code and Kotlin reflection (besides some bugs, unfortunately). No Java reflection support is given until Valhalla, that is why the current implementations (single field and multi field) are marked with @JvmInline.

If you want @JvmInline value classes to look like regular classes in the bytecode, it is impossible. Their goal was to get performance boost in monomorphic code and make lightweight wrappers.

If reflection over Inline classes takes a lot in your application and your performance suffers, you may either use regular classes or use them only in places where reflection is needed.

I absolutely agree with you about testing. Thank you for your help with fixing bugs! You really help us. When I developed MFVC, I copied and adopted IC reflection tests.

The problem with characters is that they all can be used beside ones that are forbidden by JVM specification. The same thing happens with nested classes (Outer$Inner) etc. It is also impossible to change for already existing classes. It was also chosen as a delimiter by me to show it the stacktrace. The rule of thumb I know is that if you use ` in your names, some clashes may appear.

k163377 commented 7 months ago

If you want @JvmInline value classes to look like regular classes in the bytecode, it is impossible.

How about providing annotation and compiling all of the marked functions into a Java-compatible form?

Their goal was to get performance boost in monomorphic code and make lightweight wrappers.

In fact, features such as unsigned integers provide more value than wrapping. We have often received requests to use these in jackson as well.

If reflection over Inline classes takes a lot in your application and your performance suffers, you may either use regular classes or use them only in places where reflection is needed.

Such content should be clearly stated in the documentation so that users are not unintentionally disadvantaged. https://kotlinlang.org/docs/inline-classes.html

The problem with characters is that they all can be used beside ones that are forbidden by JVM specification. The same thing happens with nested classes (Outer$Inner) etc. It is also impossible to change for already existing classes. It was also chosen as a delimiter by me to show it the stacktrace. The rule of thumb I know is that if you use ` in your names, some clashes may appear.

I understand that the SFVC is difficult to change. However, is it possible to make changes to the MFVC?

There are many use cases where unboxed getters generated for MFVC are excluded and only boxed getters are used. In this case, it is necessary to be able to distinguish the getters for SFVC so that they can remain.

If it is difficult to simply change the delimiter, how about adding a suffix, or changing the number of characters and combinations like -- or -$?

zhelenskiy commented 7 months ago

How about providing annotation and compiling all the marked functions into a Java-compatible form?

There is @JvmExposed that is doing exactly that. It is being developed by @CommanderTvis.

In fact, features such as unsigned integers provide more value than wrapping. We have often received requests to use these in jackson as well.

Unsigned integers are mostly an exception to the rule as they are provided by the standard library and have special syntax for literals. But still, if not value classes, they could have been done as usual data classes.

Such content should be clearly stated in the documentation so that users are not unintentionally disadvantaged.

I agree with this.

I understand that the SFVC is difficult to change. However, is it possible to make changes to the MFVC?

Yes, but I do not understand the reason for this. Why can users use - in names but not -$?

There are many use cases where unboxed getters generated for MFVC are excluded and only boxed getters are used. In this case, it is necessary to be able to distinguish the getters for SFVC so that they can remain.

You can do it as kotlinx.serialization does.

k163377 commented 7 months ago

There is @JvmExposed that is doing exactly that.

I did not know this, but it sounds very good.

Unsigned integers are mostly an exception to the rule as they are provided by the standard library and have special syntax for literals.

You say it is an exception, but doesn't that limit the way value class is specified in everything provided by JetBrains? Such a policy is impractical and I think it is unhealthy to restrict the feasible expressions.

On the other hand, Kotlin states Write concise and expressive code while maintaining full compatibility and interoperability with Java. Then, more effort should have been made to ensure that the use of Java based libraries is not restricted.

I hope @JvmExposed addresses these complaints adequately.

You can do it as kotlinx.serialization does.

My complaint is as stated above. To add, I don't think it is best to choose a Java based library or framework when using Kotlin on the JVM, but I think the argument that we should develop without them is wrong.

Yes, but I do not understand the reason for this. Why can users use - in names but not -$?

This is because it seems unlikely that strings like -$ will appear in property or function names. It is just my feeling, but if the target is a sequence of symbols of at least two characters (three or more is more certain), I can say that it is an edge case, even if there are reported problems with processing based on it.

The combination of - and alphanumeric characters alone is difficult to distinguish from a kebab case entered by the user.

zhelenskiy commented 7 months ago

You say it is an exception, but doesn't that limit the way value class is specified in everything provided by JetBrains? Such a policy is impractical and I think it is unhealthy to restrict the feasible expressions.

Of course, we think about the application of features we introduce, otherwise we could have done all the features from all the programming languages just because they exist. If you have another application that we haven't found, tell us about it. We don't want to make SFINAE in Kotlin.

On the other hand, Kotlin states Write concise and expressive code while maintaining full compatibility and interoperability with Java. Then, more effort should have been made to ensure that the use of Java based libraries is not restricted.

That is a deal of @JvmExposed. Prevalhalla value classes have interop with Java from the user side not from generated java declarations site. If you want one, use @JvmExposed, which introduces Java-like wrappers to be more comfortable to be used by Java libs.

To add, I don't think it is best to choose a Java based library or framework when using Kotlin on the JVM, but I think the argument that we should develop without them is wrong.

I agree, but if some feature does not exist on java site, then it strange to expect its full support from Java libs that operate with bytecode (or java reflection that is bytecode based). We try to balance and support Java interoperability as much as possible. But in the case of value classes, it is not possible to support java libs out of the box as our compiler outputs declarations in the bytecode which would be inefficient if they looked java-like. So, from Java code, you can use the classes like usual classes (but inefficiently, boxed). It is possible to call its methods from java using interface, the VC implements or using @JvmExposed.

Kebab-case is not used in Kotlin. However, it makes sense to change the separator to -$ now.

k163377 commented 7 months ago

At any rate, I hope @JvmExposed will solve a lot of problems. I will verify with jackson when I am able to try it. Thanks for your thoughtful replies so far.

My only personal remaining concern is that it would take a great deal of work to make this happen.

Kebab-case is not used in Kotlin.

When a property described in CamelCase is mechanically converted to KebabCase, unintentional misconversions may occur. Personally, I do not think this is a good practice, but I think a possible solution is to use ` to express directly rather than using annotations.

it makes sense to change the separator to -$ now.

Thanks.

demidko commented 7 months ago

Hola! @zhelenskiy This is my multi-field value class:

@JvmInline
value class Sentence(private val wholeText: String, private val basis: Set<String>) {

  operator fun contains(otherBasis: Set<Set<String>>): Boolean {
    return otherBasis.any {
      basis.containsAll(it)
    }
  }

  override fun toString(): String {
    return wholeText
  }
}

And, this is simple test:

  @Test
  fun testSentence() {
    /** skiped code there **/

    val sentence = sentencesOf(file, analyzer).first().toString()
    assertThat(sentence).isEqualTo("Антуан де Сент-Экзюпери")
  }

I received the following error:

org.jetbrains.kotlin.backend.common.BackendException: Backend Internal error: Exception during IR lowering
File being compiled: /Users/MyUser/Projects/MyProject/backend/src/test/kotlin/search/domain/utils/MyTest.kt
The root cause java.lang.NullPointerException was thrown at: org.jetbrains.kotlin.backend.jvm.MfvcNodeFactoryKt.makeRootMfvcNodeSubnodes(MfvcNodeFactory.kt:524)
    at org.jetbrains.kotlin.backend.common.CodegenUtil.reportBackendException(CodegenUtil.kt:253)
    at org.jetbrains.kotlin.backend.common.CodegenUtil.reportBackendException$default(CodegenUtil.kt:237)
    at org.jetbrains.kotlin.backend.common.phaser.PerformByIrFilePhase.invokeSequential(performByIrFile.kt:65)
    at org.jetbrains.kotlin.backend.common.phaser.PerformByIrFilePhase.invoke(performByIrFile.kt:52)
    at org.jetbrains.kotlin.backend.common.phaser.PerformByIrFilePhase.invoke(performByIrFile.kt:38)
    at org.jetbrains.kotlin.backend.common.phaser.NamedCompilerPhase.phaseBody(CompilerPhase.kt:147)
    at org.jetbrains.kotlin.backend.common.phaser.AbstractNamedCompilerPhase.invoke(CompilerPhase.kt:94)
    at org.jetbrains.kotlin.backend.common.phaser.CompositePhase.invoke(PhaseBuilders.kt:29)
    at org.jetbrains.kotlin.backend.common.phaser.CompositePhase.invoke(PhaseBuilders.kt:16)
    at org.jetbrains.kotlin.backend.common.phaser.NamedCompilerPhase.phaseBody(CompilerPhase.kt:147)
    at org.jetbrains.kotlin.backend.common.phaser.AbstractNamedCompilerPhase.invoke(CompilerPhase.kt:94)
    at org.jetbrains.kotlin.backend.common.phaser.CompilerPhaseKt.invokeToplevel(CompilerPhase.kt:43)
    at org.jetbrains.kotlin.backend.jvm.JvmIrCodegenFactory.invokeLowerings(JvmIrCodegenFactory.kt:348)
    at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.runLowerings(KotlinToJVMBytecodeCompiler.kt:330)
    at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.compileModules$cli(KotlinToJVMBytecodeCompiler.kt:114)
    at org.jetbrains.kotlin.cli.jvm.compiler.KotlinToJVMBytecodeCompiler.compileModules$cli$default(KotlinToJVMBytecodeCompiler.kt:43)
    at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecute(K2JVMCompiler.kt:165)
    at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecute(K2JVMCompiler.kt:50)
    at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.kt:104)
    at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.kt:48)
    at org.jetbrains.kotlin.cli.common.CLITool.exec(CLITool.kt:101)
    at org.jetbrains.kotlin.incremental.IncrementalJvmCompilerRunner.runCompiler(IncrementalJvmCompilerRunner.kt:463)
    at org.jetbrains.kotlin.incremental.IncrementalJvmCompilerRunner.runCompiler(IncrementalJvmCompilerRunner.kt:62)
    at org.jetbrains.kotlin.incremental.IncrementalCompilerRunner.doCompile(IncrementalCompilerRunner.kt:477)
    at org.jetbrains.kotlin.incremental.IncrementalCompilerRunner.compileImpl(IncrementalCompilerRunner.kt:400)
    at org.jetbrains.kotlin.incremental.IncrementalCompilerRunner.compileNonIncrementally(IncrementalCompilerRunner.kt:281)
    at org.jetbrains.kotlin.incremental.IncrementalCompilerRunner.compile(IncrementalCompilerRunner.kt:125)
    at org.jetbrains.kotlin.daemon.CompileServiceImplBase.execIncrementalCompiler(CompileServiceImpl.kt:657)
    at org.jetbrains.kotlin.daemon.CompileServiceImplBase.access$execIncrementalCompiler(CompileServiceImpl.kt:105)
    at org.jetbrains.kotlin.daemon.CompileServiceImpl.compile(CompileServiceImpl.kt:1624)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at java.rmi/sun.rmi.server.UnicastServerRef.dispatch(UnicastServerRef.java:360)
    at java.rmi/sun.rmi.transport.Transport$1.run(Transport.java:200)
    at java.rmi/sun.rmi.transport.Transport$1.run(Transport.java:197)
    at java.base/java.security.AccessController.doPrivileged(AccessController.java:714)
    at java.rmi/sun.rmi.transport.Transport.serviceCall(Transport.java:196)
    at java.rmi/sun.rmi.transport.tcp.TCPTransport.handleMessages(TCPTransport.java:598)
    at java.rmi/sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run0(TCPTransport.java:844)
    at java.rmi/sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.lambda$run$0(TCPTransport.java:721)
    at java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
    at java.rmi/sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run(TCPTransport.java:720)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
    at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.lang.NullPointerException
    at org.jetbrains.kotlin.backend.jvm.MfvcNodeFactoryKt.makeRootMfvcNodeSubnodes(MfvcNodeFactory.kt:524)
    at org.jetbrains.kotlin.backend.jvm.MfvcNodeFactoryKt.getRootNode(MfvcNodeFactory.kt:360)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements$getRootMfvcNode$1.invoke(MemoizedMultiFieldValueClassReplacements.kt:309)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements$getRootMfvcNode$1.invoke(MemoizedMultiFieldValueClassReplacements.kt:307)
    at org.jetbrains.kotlin.storage.LockBasedStorageManager$MapBasedMemoizedFunction.invoke(LockBasedStorageManager.java:578)
    at org.jetbrains.kotlin.storage.LockBasedStorageManager$MapBasedMemoizedFunctionToNotNull.invoke(LockBasedStorageManager.java:681)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements.grouped(MemoizedMultiFieldValueClassReplacements.kt:62)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements.makeAndAddGroupedValueParametersFrom(MemoizedMultiFieldValueClassReplacements.kt:123)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements.access$makeAndAddGroupedValueParametersFrom(MemoizedMultiFieldValueClassReplacements.kt:36)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements$createStaticReplacement$1.invoke(MemoizedMultiFieldValueClassReplacements.kt:211)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements$createStaticReplacement$1.invoke(MemoizedMultiFieldValueClassReplacements.kt:204)
    at org.jetbrains.kotlin.backend.jvm.MemoizedValueClassAbstractReplacements.commonBuildReplacementInner(MemoizedValueClassAbstractReplacements.kt:120)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements.buildReplacement(MemoizedMultiFieldValueClassReplacements.kt:99)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements.createStaticReplacement(MemoizedMultiFieldValueClassReplacements.kt:204)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements$getReplacementFunctionImpl$1.invoke(MemoizedMultiFieldValueClassReplacements.kt:277)
    at org.jetbrains.kotlin.backend.jvm.MemoizedMultiFieldValueClassReplacements$getReplacementFunctionImpl$1.invoke(MemoizedMultiFieldValueClassReplacements.kt:254)
    at org.jetbrains.kotlin.storage.LockBasedStorageManager$MapBasedMemoizedFunction.invoke(LockBasedStorageManager.java:578)
    at org.jetbrains.kotlin.backend.jvm.MemoizedValueClassAbstractReplacements.getReplacementFunction(MemoizedValueClassAbstractReplacements.kt:32)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmMultiFieldValueClassLowering.visitFunctionAccess(JvmMultiFieldValueClassLowering.kt:865)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitCall(IrElementTransformerVoid.kt:214)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmMultiFieldValueClassLowering.visitCall(JvmMultiFieldValueClassLowering.kt:995)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitCall(IrElementTransformerVoid.kt:215)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitCall(IrElementTransformerVoid.kt:24)
    at org.jetbrains.kotlin.ir.expressions.IrCall.accept(IrCall.kt:26)
    at org.jetbrains.kotlin.ir.expressions.IrExpression.transform(IrExpression.kt:31)
    at org.jetbrains.kotlin.ir.declarations.IrVariable.transformChildren(IrVariable.kt:45)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitDeclaration(IrElementTransformerVoid.kt:57)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmValueClassAbstractLowering.visitDeclaration(JvmValueClassAbstractLowering.kt:253)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitVariable(IrElementTransformerVoid.kt:101)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmMultiFieldValueClassLowering.visitVariable(JvmMultiFieldValueClassLowering.kt:1200)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitVariable(IrElementTransformerVoid.kt:102)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitVariable(IrElementTransformerVoid.kt:24)
    at org.jetbrains.kotlin.ir.declarations.IrVariable.accept(IrVariable.kt:38)
    at org.jetbrains.kotlin.ir.IrElementBase.transform(IrElementBase.kt:24)
    at org.jetbrains.kotlin.ir.IrElementsKt.transformStatement(IrElements.kt:11)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmValueClassAbstractLowering.visitStatementContainer$backend_jvm_lower(JvmValueClassAbstractLowering.kt:148)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmValueClassAbstractLowering.visitBlockBody(JvmValueClassAbstractLowering.kt:164)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitBlockBody(IrElementTransformerVoid.kt:118)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitBlockBody(IrElementTransformerVoid.kt:24)
    at org.jetbrains.kotlin.ir.expressions.IrBlockBody.accept(IrBlockBody.kt:25)
    at org.jetbrains.kotlin.ir.expressions.IrBody.transform(IrBody.kt:22)
    at org.jetbrains.kotlin.ir.declarations.IrFunction.transformChildren(IrFunction.kt:62)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoidKt.transformChildrenVoid(IrElementTransformerVoid.kt:346)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.transformChildrenVoid(IrElementTransformerVoid.kt:341)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmValueClassAbstractLowering.transformFunctionFlat(JvmValueClassAbstractLowering.kt:49)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmMultiFieldValueClassLowering$visitClassNew$1.invoke(JvmMultiFieldValueClassLowering.kt:290)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmMultiFieldValueClassLowering$visitClassNew$1.invoke(JvmMultiFieldValueClassLowering.kt:288)
    at org.jetbrains.kotlin.ir.util.TransformKt.transformDeclarationsFlat(transform.kt:105)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmMultiFieldValueClassLowering.visitClassNew(JvmMultiFieldValueClassLowering.kt:288)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmValueClassLoweringDispatcher.visitClassNew(JvmValueClassLoweringDispatcher.kt:54)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmValueClassLoweringDispatcher.visitClassNew(JvmValueClassLoweringDispatcher.kt:41)
    at org.jetbrains.kotlin.backend.common.IrElementTransformerVoidWithContext.visitClass(IrElementTransformerVoidWithContext.kt:62)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitClass(IrElementTransformerVoid.kt:67)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.visitClass(IrElementTransformerVoid.kt:24)
    at org.jetbrains.kotlin.ir.declarations.IrClass.accept(IrClass.kt:73)
    at org.jetbrains.kotlin.ir.IrElementBase.transform(IrElementBase.kt:24)
    at org.jetbrains.kotlin.ir.util.TransformKt.transformInPlace(transform.kt:35)
    at org.jetbrains.kotlin.ir.declarations.IrFile.transformChildren(IrFile.kt:41)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoidKt.transformChildrenVoid(IrElementTransformerVoid.kt:346)
    at org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid.transformChildrenVoid(IrElementTransformerVoid.kt:341)
    at org.jetbrains.kotlin.backend.jvm.lower.JvmValueClassLoweringDispatcher.lower(JvmValueClassLoweringDispatcher.kt:49)
    at org.jetbrains.kotlin.backend.common.phaser.FileLoweringPhaseAdapter.invoke(PhaseBuilders.kt:120)
    at org.jetbrains.kotlin.backend.common.phaser.FileLoweringPhaseAdapter.invoke(PhaseBuilders.kt:116)
    at org.jetbrains.kotlin.backend.common.phaser.NamedCompilerPhase.phaseBody(CompilerPhase.kt:147)
    at org.jetbrains.kotlin.backend.common.phaser.AbstractNamedCompilerPhase.invoke(CompilerPhase.kt:94)
    at org.jetbrains.kotlin.backend.common.phaser.PerformByIrFilePhase.invokeSequential(performByIrFile.kt:62)
    ... 42 more

If you remove the value modifier, then everything works fine.

zhelenskiy commented 7 months ago

What version of Kotlin is it?

demidko commented 7 months ago

What version of Kotlin is it?

1.9.10, also tested with 1.9.20-Beta

zhelenskiy commented 7 months ago

Create an issue in YouTrack, please. If it is possible from your site, assign it to me. This way you would be notified about its progress.

demidko commented 7 months ago

Create an issue in YouTrack, please. If it is possible from your site, assign it to me. This way you would be notified about its progress.

https://youtrack.jetbrains.com/issue/KT-62455/Problem-with-multi-field-value-class

zhelenskiy commented 7 months ago

Thanks! I'll look at it.