-
Notifications
You must be signed in to change notification settings - Fork 362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multi-field value classes #340
Comments
The link is broken. Here's a working one: https://github.com/Kotlin/KEEP/blob/dff9e07e87fbb96e38a226943e7ee302f6149042/proposals/multi-field-value-classes.md |
It will become fixed automatically when the KEEP gets accepted. But I'll take the current one for now. |
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 |
@qwwdfsad It is already listed there: KEEP. 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. |
Backed by a single field.
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.
I'm not sure this is proper reasoning to introduce a feature to the language. |
Ok, I'll publish the benchmark results as soon, as I get them all. |
How MFVC can help |
@mcpiroman I fixed the paragraph |
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 |
@francescotescari The development is actually in progress, I just don't upgrade the proposal text for now. |
"Prototype: experimental since 1.8.20" How can we test this 👀 |
You can add ValueClasses flag. I'd recommend 1.9.0-Beta instead. But there is absolutely no guaranty for the feature for now. |
From the standpoint of having promoted
When using reflection, the In this situation, is it not wrong to add more complex functionality for a limited number of use cases? At the risk of sounding rude, I don't think the |
I have tried a few things, but personally I think we should avoid using When processing in a The |
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.
|
I can only read and write English by machine translation, so I apologize if there are any mistakes or rudeness.
Everything needs to be done to make constructors and methods look like normal classes on the You should also ask other maintainers who have Finally, please increase the amount of testing.
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? |
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 If you want 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. 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 ( |
How about providing annotation and compiling all of the marked functions into a Java-compatible form?
In fact, features such as
Such content should be clearly stated in the documentation so that users are not unintentionally disadvantaged.
I understand that the There are many use cases where unboxed getters generated for If it is difficult to simply change the delimiter, how about adding a suffix, or changing the number of characters and combinations like |
There is
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.
I agree with this.
Yes, but I do not understand the reason for this. Why can users use
You can do it as kotlinx.serialization does. |
I did not know this, but it sounds very good.
You say it is an exception, but doesn't that limit the way On the other hand, I hope
My complaint is as stated above.
This is because it seems unlikely that strings like The combination of |
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.
That is a deal of
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 Kebab-case is not used in Kotlin. However, it makes sense to change the separator to |
At any rate, I hope My only personal remaining concern is that it would take a great deal of work to make this happen.
When a property described in CamelCase is mechanically converted to KebabCase, unintentional misconversions may occur.
Thanks. |
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:
If you remove the |
What version of Kotlin is it? |
1.9.10, also tested with 1.9.20-Beta |
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 |
Thanks! I'll look at it. |
Discussion of the proposal: Multi-field value classes.
The text was updated successfully, but these errors were encountered: