Generate bytecode directly to avoid reflection to invoke defaults constructors

moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt

@@ -18,15 +18,14 @@ package com.squareup.moshi.kotlin.codegen.api
 import com.squareup.kotlinpoet.ARRAY
 import com.squareup.kotlinpoet.AnnotationSpec
 import com.squareup.kotlinpoet.AnnotationSpec.UseSiteTarget.FILE
+import com.squareup.kotlinpoet.ClassName
 import com.squareup.kotlinpoet.CodeBlock
 import com.squareup.kotlinpoet.FileSpec
 import com.squareup.kotlinpoet.FunSpec
-import com.squareup.kotlinpoet.INT
 import com.squareup.kotlinpoet.KModifier
 import com.squareup.kotlinpoet.MemberName
 import com.squareup.kotlinpoet.NameAllocator
 import com.squareup.kotlinpoet.ParameterSpec
-import com.squareup.kotlinpoet.ParameterizedTypeName
 import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy
 import com.squareup.kotlinpoet.PropertySpec
 import com.squareup.kotlinpoet.TypeName
@@ -42,8 +41,8 @@ import com.squareup.moshi.Moshi
 import com.squareup.moshi.kotlin.codegen.api.FromJsonComponent.ParameterOnly
 import com.squareup.moshi.kotlin.codegen.api.FromJsonComponent.ParameterProperty
 import com.squareup.moshi.kotlin.codegen.api.FromJsonComponent.PropertyOnly
-import java.lang.reflect.Constructor
 import java.lang.reflect.Type
+import kotlin.jvm.internal.DefaultConstructorMarker
 import org.objectweb.asm.Type as AsmType
 
 private const val MOSHI_UTIL_PACKAGE = "com.squareup.moshi.internal"
@@ -55,10 +54,10 @@ private const val TO_STRING_SIZE_BASE = TO_STRING_PREFIX.length + 1 // 1 is the
 public class AdapterGenerator(
   private val target: TargetType,
   private val propertyList: List<PropertyGenerator>,
+  private val generateForwardingClass: ((ClassName, ByteArray) -> Unit),
 ) {
 
   private companion object {
-    private val INT_TYPE_BLOCK = CodeBlock.of("%T::class.javaPrimitiveType!!", INT)
     private val DEFAULT_CONSTRUCTOR_MARKER_TYPE_BLOCK = CodeBlock.of(
       "%M!!",
       MemberName(MOSHI_UTIL_PACKAGE, "DEFAULT_CONSTRUCTOR_MARKER"),
@@ -161,16 +160,6 @@ public class AdapterGenerator(
     )
     .build()
 
-  private val constructorProperty = PropertySpec.builder(
-    nameAllocator.newName("constructorRef"),
-    Constructor::class.asClassName().parameterizedBy(originalTypeName).copy(nullable = true),
-    KModifier.PRIVATE,
-  )
-    .addAnnotation(Volatile::class)
-    .mutable(true)
-    .initializer("null")
-    .build()
-
   public fun prepare(generateProguardRules: Boolean, typeHook: (TypeSpec) -> TypeSpec = { it }): PreparedAdapter {
     val reservedSimpleNames = mutableSetOf<String>()
     for (property in nonTransientProperties) {
@@ -207,24 +196,10 @@ public class AdapterGenerator(
       else -> error("Unexpected number of arguments on primary constructor: $primaryConstructor")
     }
 
-    var hasDefaultProperties = false
-    var parameterTypes = emptyList<String>()
-    target.constructor.signature?.let { constructorSignature ->
-      if (constructorSignature.startsWith("constructor-impl")) {
-        // Inline class, we don't support this yet.
-        // This is a static method with signature like 'constructor-impl(I)I'
-        return@let
-      }
-      hasDefaultProperties = propertyList.any { it.hasDefault }
-      parameterTypes = AsmType.getArgumentTypes(constructorSignature.removePrefix("<init>"))
-        .map { it.toReflectionString() }
-    }
     return ProguardConfig(
       targetClass = className,
       adapterName = adapterName,
       adapterConstructorParams = adapterConstructorParams,
-      targetConstructorHasDefaults = hasDefaultProperties,
-      targetConstructorParams = parameterTypes,
     )
   }
 
@@ -287,7 +262,7 @@ public class AdapterGenerator(
     }
 
     result.addFunction(generateToStringFun())
-    result.addFunction(generateFromJsonFun(result))
+    result.addFunction(generateFromJsonFun())
     result.addFunction(generateToJsonFun())
 
     return result.build()
@@ -321,7 +296,7 @@ public class AdapterGenerator(
       .build()
   }
 
-  private fun generateFromJsonFun(classBuilder: TypeSpec.Builder): FunSpec {
+  private fun generateFromJsonFun(): FunSpec {
     val result = FunSpec.builder("fromJson")
       .addModifiers(KModifier.OVERRIDE)
       .addParameter(readerParam)
@@ -510,81 +485,35 @@ public class AdapterGenerator(
       CodeBlock.of("return·")
     }
 
-    // Used to indicate we're in an if-block that's assigning our result value and
-    // needs to be closed with endControlFlow
-    var closeNextControlFlowInAssignment = false
-
     if (useDefaultsConstructor) {
-      // Happy path - all parameters with defaults are set
-      val allMasksAreSetBlock = maskNames.withIndex()
-        .map { (index, maskName) ->
-          CodeBlock.of("$maskName·== 0x${Integer.toHexString(maskAllSetValues[index])}.toInt()")
-        }
-        .joinToCode("·&& ")
-      result.beginControlFlow("if (%L)", allMasksAreSetBlock)
-      result.addComment("All parameters with defaults are set, invoke the constructor directly")
-      result.addCode("«%L·%T(", returnOrResultAssignment, originalTypeName)
-      var localSeparator = "\n"
-      val paramsToSet = components.filterIsInstance<ParameterProperty>()
-        .filterNot { it.property.isTransient }
-
-      // Set all non-transient property parameters
-      for (input in paramsToSet) {
-        result.addCode(localSeparator)
-        val property = input.property
-        result.addCode("%N = %N", property.name, property.localName)
-        if (property.isRequired) {
-          result.addMissingPropertyCheck(property, readerParam)
-        } else if (!input.type.isNullable) {
-          // Unfortunately incurs an intrinsic null-check even though we know it's set, but
-          // maybe in the future we can use contracts to omit them.
-          result.addCode("·as·%T", input.type)
-        }
-        localSeparator = ",\n"
-      }
-      result.addCode("\n»)\n")
-      result.nextControlFlow("else")
-      closeNextControlFlowInAssignment = true
-
-      classBuilder.addProperty(constructorProperty)
-      result.addComment("Reflectively invoke the synthetic defaults constructor")
+      val forwardingClassName = ClassName(originalRawTypeName.packageName, "${adapterName}Forwarder")
+
+      generateForwardingClass(forwardingClassName,
+        ForwardingClassGenerator.makeConstructorForwarder(
+          forwardingClassName.canonicalName,
+          originalRawTypeName.reflectionName(),
+          // TODO omit the marker?
+          target.constructor.descriptor!!.let {
+            val parameterSignature = it.substringAfter('(').substringBeforeLast(')')
+            parameterSignature + maskNames.joinToString("") { "I" } + "L${DefaultConstructorMarker::class.asClassName().reflectionName().replace('.', '/')};"
+          },
+          target.constructor.creatorSignature,
+        ))
+
+      result.addComment("Invoke the synthetic defaults constructor")
       // Dynamic default constructor call
-      val nonNullConstructorType = constructorProperty.type.copy(nullable = false)
-      val args = constructorPropertyTypes
-        .plus(0.until(maskCount).map { INT_TYPE_BLOCK }) // Masks, one every 32 params
-        .plus(DEFAULT_CONSTRUCTOR_MARKER_TYPE_BLOCK) // Default constructor marker is always last
-        .joinToCode(", ")
-      val coreLookupBlock = CodeBlock.of(
-        "%T::class.java.getDeclaredConstructor(%L)",
-        originalRawTypeName,
-        args,
-      )
-      val lookupBlock = if (originalTypeName is ParameterizedTypeName) {
-        CodeBlock.of("(%L·as·%T)", coreLookupBlock, nonNullConstructorType)
+      // TODO is this necessary? Think the IDE can just infer this directly
+      val possibleGeneric = if (typeVariables.isNotEmpty()) {
+        // TODO support TypeParameterName as %N
+        CodeBlock.of("<%L>", typeVariables.joinToCode { CodeBlock.of(it.name) })
       } else {
-        coreLookupBlock
+        CodeBlock.of("")
       }
-      val initializerBlock = CodeBlock.of(
-        "this.%1N·?: %2L.also·{ this.%1N·= it }",
-        constructorProperty,
-        lookupBlock,
-      )
-      val localConstructorProperty = PropertySpec.builder(
-        nameAllocator.newName("localConstructor"),
-        nonNullConstructorType,
-      )
-        .addAnnotation(
-          AnnotationSpec.builder(Suppress::class)
-            .addMember("%S", "UNCHECKED_CAST")
-            .build(),
-        )
-        .initializer(initializerBlock)
-        .build()
-      result.addCode("%L", localConstructorProperty)
       result.addCode(
-        "«%L%N.newInstance(",
+        "«%L%T.of%L(",
         returnOrResultAssignment,
-        localConstructorProperty,
+        forwardingClassName,
+        possibleGeneric,
       )
     } else {
       // Standard constructor call. Don't omit generics for parameterized types even if they can be
@@ -632,11 +561,6 @@ public class AdapterGenerator(
 
     result.addCode("\n»)\n")
 
-    // Close the result assignment control flow, if any
-    if (closeNextControlFlowInAssignment) {
-      result.endControlFlow()
-    }
-
     // Assign properties not present in the constructor.
     for (property in nonTransientProperties) {
       if (property.hasConstructorParameter) {

moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/ForwardingClassGenerator.java

@@ -0,0 +1,182 @@
+/*
+ * Copyright 2022 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.squareup.moshi.kotlin.codegen.api;
+
+import org.objectweb.asm.AnnotationVisitor;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Type;
+
+import static org.objectweb.asm.ClassWriter.COMPUTE_MAXS;
+import static org.objectweb.asm.Opcodes.ACC_FINAL;
+import static org.objectweb.asm.Opcodes.ACC_STATIC;
+import static org.objectweb.asm.Opcodes.ACC_SUPER;
+import static org.objectweb.asm.Opcodes.ALOAD;
+import static org.objectweb.asm.Opcodes.ARETURN;
+import static org.objectweb.asm.Opcodes.DLOAD;
+import static org.objectweb.asm.Opcodes.DUP;
+import static org.objectweb.asm.Opcodes.FLOAD;
+import static org.objectweb.asm.Opcodes.ILOAD;
+import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
+import static org.objectweb.asm.Opcodes.LLOAD;
+import static org.objectweb.asm.Opcodes.NEW;
+import static org.objectweb.asm.Opcodes.V1_8;
+
+/**
+ * Generates a class that invokes the constructor of another class.
+ *
+ * <p>The point here is that the constructor might be synthetic, in which case it can't be called
+ * directly from Java source code. Say we want to call the constructor {@code ConstructMe(int,
+ * String, long)} with parameters {@code 1, "2", 3L}. If the constructor is synthetic, then Java
+ * source code can't just do {@code new ConstructMe(1, "2", 3L)}. So this class allows you to
+ * generate a class file, say {@code Forwarder}, that is basically what you would get if you could
+ * compile this:
+ *
+ * <pre>
+ * final class Forwarder {
+ *   private Forwarder() {}
+ *
+ *   static ConstructMe of(int a, String b, long c) {
+ *     return new ConstructMe(a, b, c);
+ *   }
+ * }
+ * </pre>
+ *
+ * <p>Because the class file is assembled directly, rather than being produced by the Java compiler,
+ * it <i>can</i> call the synthetic constructor. Then regular Java source code can do {@code
+ * Forwarder.of(1, "2", 3L)} to call the constructor.
+ */
+final class ForwardingClassGenerator {
+  /**
+   * Assembles a class with a static method {@code of} that calls the constructor of another class
+   * with the same parameters.
+   *
+   * <p>It would be simpler if we could just pass in an {@code ExecutableElement} representing the
+   * constructor, but if it is synthetic then it won't be visible to the {@code javax.lang.model}
+   * APIs. So we have to pass the constructed type and the constructor parameter types separately.
+   *
+   * @param forwardingClassName the fully-qualified name of the class to generate
+   * @param targetClass the internal name of the type whose constructor will be invoked ({@code ConstructMe} in the
+   *     example above)
+   * @param parameterDescriptor the jvm parameters descriptor of the target class constructor to invoke. Should include the bit fields and default constructor marker.
+   * @param creatorSignature the jvm signature (not descriptor) of the to-be-generated "of" creator method. Should include generics information.
+   * @return a byte array making up the new class file.
+   */
+  static byte[] makeConstructorForwarder(
+      String forwardingClassName,
+      String targetClass,
+      String parameterDescriptor,
+      String creatorSignature
+      ) {
+
+    ClassWriter classWriter = new ClassWriter(COMPUTE_MAXS);
+    classWriter.visit(
+        V1_8,
+        ACC_FINAL | ACC_SUPER,
+        internalName(forwardingClassName),
+        null,
+        "java/lang/Object",
+        null);
+    classWriter.visitSource(forwardingClassName, null);
+
+    String internalClassToConstruct = internalName(targetClass);
+    String ofMethodDescriptor =  "(" + parameterDescriptor + ")L" + internalClassToConstruct + ";";
+    MethodVisitor ofMethodVisitor =
+        classWriter.visitMethod(
+          ACC_STATIC,
+          /* name */ "of",
+          /* descriptor */ ofMethodDescriptor,
+          /* signature */ creatorSignature,
+          null
+        );
+
+    // Tell Kotlin we're always returning non-null and avoid a platform type warning.
+    AnnotationVisitor av = ofMethodVisitor.visitAnnotation("Lorg/jetbrains/annotations/NotNull;", true);
+    av.visitEnd();
+
+    ofMethodVisitor.visitCode();
+
+    // The remaining instructions are basically what ASMifier generates for a class like the
+    // `Forwarder` class in the example above.
+    ofMethodVisitor.visitTypeInsn(NEW, internalClassToConstruct);
+    ofMethodVisitor.visitInsn(DUP);
+
+    String constructorToCallSignature = "(" + parameterDescriptor + ")V";
+    Type[] paramTypes = Type.getArgumentTypes(constructorToCallSignature);
+    int local = 0;
+    for (Type type : paramTypes) {
+      ofMethodVisitor.visitVarInsn(loadInstruction(type), local);
+      local += localSize(type);
+    }
+    ofMethodVisitor.visitMethodInsn(
+        INVOKESPECIAL,
+        internalClassToConstruct,
+        "<init>",
+        constructorToCallSignature,
+        /* isInterface= */ false);
+
+    ofMethodVisitor.visitInsn(ARETURN);
+    ofMethodVisitor.visitMaxs(0, 0);
+    ofMethodVisitor.visitEnd();
+    classWriter.visitEnd();
+    return classWriter.toByteArray();
+  }
+
+  /** The bytecode instruction that copies a parameter of the given type onto the JVM stack. */
+  private static int loadInstruction(Type type) {
+    switch (type.getSort()) {
+      case Type.BOOLEAN:
+      case Type.CHAR:
+      case Type.BYTE:
+      case Type.SHORT:
+      case Type.INT:
+        return ILOAD;
+      case Type.FLOAT:
+        return FLOAD;
+      case Type.LONG:
+        return LLOAD;
+      case Type.DOUBLE:
+        return DLOAD;
+      case Type.ARRAY:
+      case Type.OBJECT:
+        return ALOAD;
+      default:
+        throw new IllegalArgumentException("Unexpected type " + type);
+    }
+  }
+
+  /**
+   * The size in the local variable array of a value of the given type. A quirk of the JVM means
+   * that long and double variables each take up two consecutive slots in the local variable array.
+   * (The first n local variables are the parameters, so we need to know their sizes when iterating
+   * over them.)
+   */
+  private static int localSize(Type type) {
+    switch (type.getSort()) {
+      case Type.LONG:
+      case Type.DOUBLE:
+        return 2;
+      default:
+        return 1;
+    }
+  }
+
+  private static String internalName(String className) {
+    return className.replace('.', '/');
+  }
+
+  private ForwardingClassGenerator() {}
+}