Add keep rules to ensure the name is kept for defined kotlin primitive types

[mkj-gram]

When looking up java methods from kotlin metadata, the reflection library goes through src/kotlin/reflect/jvm/internal/RuntimeTypeMapper.kt to find a concrete java method. To bridge kotlin specific types it uses a hard coded map (I believe it is this one: https://github.com/JetBrains/kotlin/blob/master/core/metadata.jvm/src/org/jetbrains/kotlin/metadata/jvm/deserialization/ClassMapperLite.kt) to map void to kotlin.Unit to void etc.

kotlin-stdlib defines kotlin.Unit, UByte, ..., ULong as classes in the kotlin standard library. Since these types can be present in the output after an R8 compilation, and they have no defined keep rules, their names can move freely. R8 will therefore rename kotlin.Unit to fx. a.a and rewrite kotlin/Unit to a/a in the metadata. The result is that looking up java methods fails.

A bug reported on this issue can be found here: https://issuetracker.google.com/issues/196179629

Note that R8 from version 3.1.24 handles pruning of these specific types. Before version 3.1.24, if we were able to prune kotlin.Unit we would rewrite the type in the Metadata to kotlin/Any since kotlin.Unit is a reference type - it is defined as a class in the library. From version 3.1.24 we now have special handling for the list of types in this CL such that their name stays the same.

There is no need to update the other PG files since it is unknown how proguard rewrites metadata and earlier version than R8 1.6.0 do not support metadata.

[udalov]

Unsigned types and arrays are not handled by ClassMapperLite, could you clarify why it's necessary to have them here?

[mkj-gram]

I am not particular sure on the internals on where things get rewritten (also why I wrote believe :P). I created this test:

  fun testUInt() : UInt {
    return 42u
  }

  fun testUIntArray() : UIntArray {
    return uintArrayOf(42u)
  }
}

The generated method definitions are:

  public final int testUInt-pVg5ArA();

  public final int[] testUIntArray--hP7Qyg();

Using kotlin.reflect it will somehow map UInt -> int and UIntArray -> int[] using https://github.com/JetBrains/kotlin/blob/e2f462095dbcc78a31c373af0ab3f209bdf196f0/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KFunctionImpl.kt#L62

When compiling with R8 and having kotlin-stdlib defined on the program path we are free to rename kotlin/UInt and kotlin/UIntArray to foo.a and bar.b. We then also rename those types in the metadata. After the R8 compilation kotlin.reflect no longer works because RuntimeMapper.mapSignature no longer maps to ()int but to ()foo.a and similar for UIntArray. So internally in the belly of RuntimeMapper.mapSignature is some known map that gives special meaning to kotlin.UInt and kotlin.UIntArray. I do hope it is not the *.builtins that we need to parse and update.

[udalov]

Sorry for the delay, and thanks for the explanation. The way RuntimeTypeMapper.mapSignature works is as follows: if there's a JVM signature recorded for a declaration in the metadata, use it as is; if there's no JVM signature, try to re-create it via a hash map of known types given in ClassMapperLite + mapping any class straightforwardly to its FQ name. This type mapping logic was implemented as an optimization, to avoid storing JVM signatures for declarations whose JVM signature can be almost trivially computed from the Kotlin signature.

The problem with unsigned types and arrays (UInt, UIntArray, ...) is probably related to the fact that after R8 has moved an unsigned type, its FQ name is now different, and the mangling suffix for declarations which use that unsigned type will be different. This is for example -pVg5ArA in your example. If the JVM signature is not recorded for this declaration, the "dumb" ClassMapperLite behavior will give a totally incorrect result.

But if that's the case, the problem will likely happen for any user-defined inline class (@JvmInline value class), not just the unsigned types from kotlin-stdlib, since this mangling is happening for all inline classes (except @kotlin.Result for an unrelated reason). Can you check if it's the case? Probably R8 should either explicitly write JVM signature for all declarations which have inline class types in the signature via KmFunction.signature/KmProperty.getterSignature/..., or disable relocation for inline class types altogether.

[mkj-gram]

You are right that we will prune the types of a value class if it is not kept:

@JvmInline
value class Password(val s: String)

@KeepForApi
fun login(pw : Password) {
  println(pw.s)
}

The login-XXXX method will in it's metadata have the signature 'Any' because we prune the generated Password class. That is perfectly fine because R8 do not guarantee that a method that is kept will also have it's signature kept. One has to add -keep class Password for being certain that login can work. And if that keep rule is added we will not prune Password from the metadata signature.

However, the small example showed a small problem if anyone tries to use -includedescriptorclasses because one would probably think that Password would then be kept. But since the generated login-function do not actually refer to Password we remove it anyway. I added https://issuetracker.google.com/issues/208209210 to track that. Thanks for pointing it out.