Migrating the Util.kt and KModifier from JVM to common
#2006
Conversation
| internal actual inline fun <reified E : Enum<E>> enumSetOf(vararg values: E): MutableSet<E> { | ||
| return when (values.size) { | ||
| 0 -> EnumSet.noneOf(E::class.java) | ||
| 1 -> EnumSet.of(values[0]) | ||
| 2 -> EnumSet.of(values[0], values[1]) | ||
| 3 -> EnumSet.of(values[0], values[1], values[2]) | ||
| 4 -> EnumSet.of(values[0], values[1], values[2], values[3]) | ||
| 5 -> EnumSet.of(values[0], values[1], values[2], values[3], values[4]) | ||
| else -> EnumSet.copyOf(values.toSet()) | ||
| } | ||
| } |
There was a problem hiding this comment.
There is no need to do this. Just use a regular set and mutable set in common code. We're no longer gaining any advantage of EnumSet having to go through this allocation-heavy path.
There was a problem hiding this comment.
Okay, I get it.
| internal actual fun formatIsoControlCode(code: Int): String { | ||
| return buildString(6) { | ||
| append("\\u") | ||
| appendFormat04x(code) | ||
| } | ||
| } | ||
|
|
||
| @OptIn(ExperimentalStdlibApi::class) | ||
| private val HexFormatWithoutLeadingZeros = HexFormat { | ||
| number { | ||
| removeLeadingZeros = true | ||
| } | ||
| } | ||
|
|
||
| @OptIn(ExperimentalStdlibApi::class) | ||
| internal fun Appendable.appendFormat04x(code: Int) { | ||
| val hex = code.toHexString(HexFormatWithoutLeadingZeros) | ||
| if (hex.length < 4) { | ||
| repeat(4 - hex.length) { append('0') } | ||
| } | ||
| append(hex) | ||
| } |
There was a problem hiding this comment.
This seems like it could be simplified to toString(16).padStart(4, '0') and placed directly in common code.
There was a problem hiding this comment.
Oh! It seems so. Will simplify it later
| internal actual fun Int.toHexStr(): String = | ||
| toHexString(HexFormatWithoutLeadingZeros) |
There was a problem hiding this comment.
Is this just toString(16)? If so, we can use that directly in common code (without relying on unstable APIs, as well).
There was a problem hiding this comment.
Yes, it seems so. I'll simplify it
| internal actual fun <K, V> Map<K, V>.toImmutableMap(): Map<K, V> = | ||
| toMap() | ||
|
|
||
| internal actual fun <T> Collection<T>.toImmutableList(): List<T> = | ||
| toList() | ||
|
|
||
| internal actual fun <T> Collection<T>.toImmutableSet(): Set<T> = | ||
| toSet() |
There was a problem hiding this comment.
If we're not going to put these in an unmodifiable wrapper on other targets, do we still need to do it on JVM? Are we really worried about Java callers mutating things? I don't think I am.
There was a problem hiding this comment.
The original implementation was copied when we forked from JavaPoet, for what it's worth.
There was a problem hiding this comment.
Well, you have a point. If we no longer care about calls that mutate things on the JVM, do we keep toImmutable* in common and have its result just return a to* (like toSet), or do we simply remove them and use to* instead where appropriate?
There was a problem hiding this comment.
I think it's defending against something that is not a real problem in practice, and I would eliminate the concept entirely and use Kotlin's collection APIs directly everywhere. This should probably be done in its own PR rather than cluttering this one.
Part of #304, #1959
Migrating
Util.ktandKModifierfrom JVM to common.Regexin WasmJS.On the JVM, however, the effect is the same before and after migration.
Let me know if there's anything I need to do, thanks.
If these changes are not reviewed until after 2.0, you can always tell me, convert it to draft, or close it~