Found what seems like a pretty significant bug for processor

Question

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>

Zac Sweers · Answer

FYI < yigit> since I imagine this affects Room too

yigit · Answer

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 room compiler processing src main java androidx room compiler processing ksp KspClassFileUtility kt l=39 q=KspClassFileUtility amp sq=|https cs android com androidx platform frameworks support + androidx main room roo ing ksp KspClassFileUtility kt l=39 q=KspClassFileUtility amp sq=>

Zac Sweers · Answer

oof

Zac Sweers · Answer

I don t think we want to do that in moshi vs just waiting for a fix

Jiaxiang · Answer

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 lt init gt 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 lt init gt 8 = Utf8 V 9 = Utf8 Code 10 = Utf8 LineNumberTable 11 = Utf8 SourceFile 12 = Utf8 A java 13 = NameAndType 7 8 lt init gt 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 lt init gt 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 lt init gt 6 = Utf8 V 7 = NameAndType 5 6 lt init gt V 8 = Methodref 4 7 java lang Object lt init gt 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 lt set gt 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 amp 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 lt init gt 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 lt set gt 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 amp 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 ```

Zac Sweers · Answer

it must be we re able to see this in kapt java apt

Ting-Yuan Huang · Answer

re bytecode dump the backing field is private did you pass ` p` to `javap` < Jiaxiang>

Ting-Yuan Huang · Answer

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

Zac Sweers · Answer

would it maybe be simpler to do something like `Resolver jvmModifiers prop KSPropertyDeclaration Set lt Modifier gt `

Zac Sweers · Answer

I also wonder if functions need something like this for synchronized strictfp

Ting-Yuan Huang · Answer

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

Zac Sweers · Answer

hmm they re not really access specific though We call them jvm modifiers in kotlinpoet fwiw

Zac Sweers · Answer

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

Ting-Yuan Huang · Answer

It is possible by a lot of to implementations in `symbol processing` and `kotlin compiler embeddable` to read the class file as a stream and 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

Zac Sweers · Answer

gotcha Unfortunately not sure if we can wait confused We could probably do a quick patch release after it s out though if it s not an experimental API

Ting-Yuan Huang · Answer

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

Zac Sweers · Answer

yup we re not going to cast to impl versions in moshi at least too much of a risk

Ting-Yuan Huang · Answer

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

Zac Sweers · Answer

your call Just feels like one function is better than two

Zac Sweers · Answer

I d also expect this to come via the existing JAVA TRANSIENT and JAVA VOLATILE modifiers

Zac Sweers · Answer

which is why I ve been sticking to that nomenclature

Ting-Yuan Huang · Answer

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