https://kotlinlang.org logo
Title
z

Zac Sweers

11/03/2021, 5:34 PM
Found what seems like a pretty significant bug for processors handling serialization of any sort - jvm modifiers from externally-compiled classes are not visible to subclasses in the current compilation https://github.com/google/ksp/issues/710
FYI @yigit since I imagine this affects Room too
y

yigit

11/03/2021, 5:36 PM
likely. we've had a couple of similar bugs. at some cases, room goes and parses the class file (for order of elements) https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:room/roo[…]ing/ksp/KspClassFileUtility.kt;l=39?q=KspClassFileUtility&sq=
z

Zac Sweers

11/03/2021, 5:48 PM
oof
I don't think we want to do that in moshi vs just waiting for a fix
j

Jiaxiang

11/03/2021, 6:17 PM
I tried to compile
BaseClass
with kotlinc and found no transient information with javap, compared with a java class with transient field. Need to check compiler if it is supported. compiled from Java.
Classfile /Users/jiaxiang/code/tmp/A.class
  Last modified Nov 3, 2021; size 211 bytes
  MD5 checksum b79bd6ca07077f984016d38c860a7b23
  Compiled from "A.java"
public class A
  minor version: 0
  major version: 55
  flags: (0x0021) ACC_PUBLIC, ACC_SUPER
  this_class: #3                          // A
  super_class: #4                         // java/lang/Object
  interfaces: 0, fields: 1, methods: 1, attributes: 1
Constant pool:
   #1 = Methodref          #4.#13         // java/lang/Object."<init>":()V
   #2 = Fieldref           #3.#14         // A.k:I
   #3 = Class              #15            // A
   #4 = Class              #16            // java/lang/Object
   #5 = Utf8               k
   #6 = Utf8               I
   #7 = Utf8               <init>
   #8 = Utf8               ()V
   #9 = Utf8               Code
  #10 = Utf8               LineNumberTable
  #11 = Utf8               SourceFile
  #12 = Utf8               A.java
  #13 = NameAndType        #7:#8          // "<init>":()V
  #14 = NameAndType        #5:#6          // k:I
  #15 = Utf8               A
  #16 = Utf8               java/lang/Object
{
  transient int k;
    descriptor: I
    flags: (0x0080) ACC_TRANSIENT

  public A();
    descriptor: ()V
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=2, locals=1, args_size=1
         0: aload_0
         1: invokespecial #1                  // Method java/lang/Object."<init>":()V
         4: aload_0
         5: iconst_1
         6: putfield      #2                  // Field k:I
         9: return
      LineNumberTable:
        line 1: 0
        line 2: 4
}
SourceFile: "A.java"
compiled from Kotlin
Classfile /Users/jiaxiang/code/tmp/BaseClass.class
  Last modified Nov 3, 2021; size 717 bytes
  MD5 checksum 056d382f7986c813aed1d76805a9c8be
  Compiled from "tmp.kt"
public abstract class BaseClass
  minor version: 0
  major version: 52
  flags: (0x0421) ACC_PUBLIC, ACC_SUPER, ACC_ABSTRACT
  this_class: #2                          // BaseClass
  super_class: #4                         // java/lang/Object
  interfaces: 0, fields: 1, methods: 3, attributes: 2
Constant pool:
   #1 = Utf8               BaseClass
   #2 = Class              #1             // BaseClass
   #3 = Utf8               java/lang/Object
   #4 = Class              #3             // java/lang/Object
   #5 = Utf8               <init>
   #6 = Utf8               ()V
   #7 = NameAndType        #5:#6          // "<init>":()V
   #8 = Methodref          #4.#7          // java/lang/Object."<init>":()V
   #9 = Utf8               lastSet
  #10 = Utf8               I
  #11 = NameAndType        #9:#10         // lastSet:I
  #12 = Fieldref           #2.#11         // BaseClass.lastSet:I
  #13 = Utf8               this
  #14 = Utf8               LBaseClass;
  #15 = Utf8               getLastSet
  #16 = Utf8               ()I
  #17 = Utf8               setLastSet
  #18 = Utf8               (I)V
  #19 = Utf8               <set-?>
  #20 = Utf8               Lkotlin/Metadata;
  #21 = Utf8               mv
  #22 = Integer            1
  #23 = Integer            6
  #24 = Integer            0
  #25 = Utf8               k
  #26 = Utf8               xi
  #27 = Integer            48
  #28 = Utf8               d1
  #29 = Utf8               \u0000\u0014\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0002\b\u0002\n\u0002\u0010\b\n\u0002\b\u0004\b&\u0018\u00002\u00020\u0001B\u0005¢\u0006\u0002\u0010\u0002R\u001a\u0010\u0003\u001a\u00020\u0004X\u0096\u000e¢\u0006\u000e\n\u0000\u001a\u0004\b\u0005\u0010\u0006\"\u0004\b\u0007\u0010\b
  #30 = Utf8               d2
  #31 = Utf8
  #32 = Utf8               tmp.kt
  #33 = Utf8               Code
  #34 = Utf8               LineNumberTable
  #35 = Utf8               LocalVariableTable
  #36 = Utf8               SourceFile
  #37 = Utf8               RuntimeVisibleAnnotations
{
  public BaseClass();
    descriptor: ()V
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=2, locals=1, args_size=1
         0: aload_0
         1: invokespecial #8                  // Method java/lang/Object."<init>":()V
         4: aload_0
         5: iconst_1
         6: putfield      #12                 // Field lastSet:I
         9: return
      LineNumberTable:
        line 1: 0
        line 2: 4
        line 1: 9
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0      10     0  this   LBaseClass;

  public int getLastSet();
    descriptor: ()I
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: getfield      #12                 // Field lastSet:I
         4: ireturn
      LineNumberTable:
        line 2: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       5     0  this   LBaseClass;

  public void setLastSet(int);
    descriptor: (I)V
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=2, locals=2, args_size=2
         0: aload_0
         1: iload_1
         2: putfield      #12                 // Field lastSet:I
         5: return
      LineNumberTable:
        line 2: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       6     0  this   LBaseClass;
            0       6     1 <set-?>   I
}
SourceFile: "tmp.kt"
RuntimeVisibleAnnotations:
  0: #20(#21=[I#22,I#23,I#24],#25=I#22,#26=I#27,#28=[s#29],#30=[s#14,s#31,s#6,s#9,s#31,s#15,s#16,s#17,s#18])
    kotlin.Metadata(
      mv=[1,6,0]
      k=1
      xi=48
      d1=["\u0000\u0014\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0002\b\u0002\n\u0002\u0010\b\n\u0002\b\u0004\b&\u0018\u00002\u00020\u0001B\u0005¢\u0006\u0002\u0010\u0002R\u001a\u0010\u0003\u001a\u00020\u0004X\u0096\u000e¢\u0006\u000e\n\u0000\u001a\u0004\b\u0005\u0010\u0006\"\u0004\b\u0007\u0010\b"]
      d2=["LBaseClass;","","()V","lastSet","","getLastSet","()I","setLastSet","(I)V"]
    )
z

Zac Sweers

11/03/2021, 6:43 PM
it must be, we're able to see this in kapt/java apt
t

Ting-Yuan Huang

11/04/2021, 6:20 AM
re: bytecode dump: the backing field is private. did you pass
-p
to
javap
? @Jiaxiang
Unfortunately Kotlin compiler didn't leave any clue about this in metadata, and therefore the only solution is parsing the class file. Because
transient
and
volatile
is JVM specific and don't really appear in Kotlin code, and also because of performance consideration, adding them to modifiers seems infeasible*1. Instead, the functionality can be provided by
Resolver.isJvmTransient(prop: KSPropertyDeclaration)
and
Resolver.isJvmVolatile(prop: KSPropertyDeclaration)
. In that way you also won't need to check annotations for Kotlin sources. Please let me know what you think. *1 for Kotlin libraries. Java classes are already parsed and the information should be handily available.
z

Zac Sweers

11/04/2021, 4:58 PM
would it maybe be simpler to do something like
Resolver.jvmModifiers(prop: KSPropertyDeclaration): Set<Modifier>
?
I also wonder if functions need something like this for synchronized/strictfp
t

Ting-Yuan Huang

11/04/2021, 11:52 PM
How about
Resolver.jvmAccessFlags
? I'm worried that
Resolver.jvmModifiers
would look too similar to the existing KSModifierListOwner.modifiers and be confusing. Making it explicit that it is jvm implementation specific probably has another benefit. IMHO, modifiers are more of a source / language level thing than library/implementation. Sometimes it's unclear what to do for compiled classes. For example (besides transient and volatile), Kotlin classes are public by default and we debated long long ago whether to synthesize it.
z

Zac Sweers

11/05/2021, 1:31 AM
hmm, they're not really access-specific though. We call them jvm modifiers in kotlinpoet fwiw
by the way - is it possible to access the underlying descriptor like you'd do in that function? Wondering if we can work around this for now in Moshi
t

Ting-Yuan Huang

11/05/2021, 3:25 AM
It is possible by a lot of casting to implementations in
symbol-processing
and
kotlin-compiler-embeddable
to read the class file as a stream, and parsing the stream with another library. I won't recommend it though if you can wait for a couple of weeks until the new API is implemented and released.
z

Zac Sweers

11/05/2021, 3:26 AM
gotcha. Unfortunately not sure if we can wait 😕. We could probably do a quick patch release after it's out though if it's not an experimental API
t

Ting-Yuan Huang

11/05/2021, 3:29 AM
btw, one of the drawback of depending on the implementation is that processor could be broken with future releases of ksp and compiler. If you really need to do it, please access the implementation as few as possible.
z

Zac Sweers

11/05/2021, 3:31 AM
yup we're not going to cast to impl versions in moshi at least, too much of a risk
t

Ting-Yuan Huang

11/05/2021, 3:48 AM
re: API: I would like to avoid "modifier" in the name of the new API. It is confusing in many cases. I'm going to implement
isJvmVolatile
,
isJvmTransient
, etc for now but please feel free to advise.
z

Zac Sweers

11/05/2021, 3:49 AM
your call! Just feels like one function is better than two
I'd also expect this to come via the existing JAVA_TRANSIENT and JAVA_VOLATILE modifiers
which is why I've been sticking to that nomenclature
t

Ting-Yuan Huang

11/05/2021, 3:52 AM
Yeah, I agree with the simplicity. We can change the API anytime before next release (or add better ones after the release); The implementations are not too different. For compiled Java code, they will have those modifiers in place. Only compiled Kotlin code need the new API.
👍 1