Add ability to use getter in AutoCodecProcessor when field for instantiator parameter isn't present. Allows us to handle cases where the class type encodes the parameter value. This also gives a compile-time check that field is present before blindly using it in codec.

Lets us get rid of a non-AutoCodec class.

PiperOrigin-RevId: 185573686

2 files changed, 73 insertions, 108 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
index 5cdf67cd31..33d29d3461 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
@@ -22,14 +22,9 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
-import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext;
 import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
-import com.google.devtools.build.lib.skyframe.serialization.SerializationContext;
-import com.google.devtools.build.lib.skyframe.serialization.SerializationException;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.skyframe.SkyFunctionName;
-import com.google.protobuf.CodedInputStream;
-import com.google.protobuf.CodedOutputStream;
-import java.io.IOException;
 import java.util.Objects;
 import javax.annotation.Nullable;
 
@@ -37,8 +32,9 @@ import javax.annotation.Nullable;
  * A (Label, Configuration key) pair. Note that this pair may be used to look up the generating
  * action of an artifact.
  */
+@AutoCodec
 public class ConfiguredTargetKey extends ActionLookupKey {
-  public static final ObjectCodec<ConfiguredTargetKey> CODEC = Codec.INSTANCE;
+  public static final ObjectCodec<ConfiguredTargetKey> CODEC = new ConfiguredTargetKey_AutoCodec();
 
   private final Label label;
   @Nullable private final BuildConfigurationValue.Key configurationKey;
@@ -70,6 +66,7 @@ public class ConfiguredTargetKey extends ActionLookupKey {
     return of(label, keyAndHost.key, keyAndHost.isHost);
   }
 
+  @AutoCodec.Instantiator
   static ConfiguredTargetKey of(
       Label label,
       @Nullable BuildConfigurationValue.Key configurationKey,
@@ -201,40 +198,4 @@ public class ConfiguredTargetKey extends ActionLookupKey {
       this.isHost = isHost;
     }
   }
-
-  private static final class Codec implements ObjectCodec<ConfiguredTargetKey> {
-    private static final Codec INSTANCE = new Codec();
-
-    private Codec() {}
-
-    @Override
-    public Class<ConfiguredTargetKey> getEncodedClass() {
-      return ConfiguredTargetKey.class;
-    }
-
-    @Override
-    public void serialize(
-        SerializationContext context, ConfiguredTargetKey obj, CodedOutputStream codedOut)
-        throws SerializationException, IOException {
-      Label.CODEC.serialize(context, obj.label, codedOut);
-      if (obj.configurationKey == null) {
-        codedOut.writeBoolNoTag(false);
-      } else {
-        codedOut.writeBoolNoTag(true);
-        BuildConfigurationValue.Key.CODEC.serialize(context, obj.configurationKey, codedOut);
-      }
-      codedOut.writeBoolNoTag(obj.isHostConfiguration());
-    }
-
-    @Override
-    public ConfiguredTargetKey deserialize(DeserializationContext context, CodedInputStream codedIn)
-        throws SerializationException, IOException {
-      return of(
-          Label.CODEC.deserialize(context, codedIn),
-          codedIn.readBool()
-              ? BuildConfigurationValue.Key.CODEC.deserialize(context, codedIn)
-              : null,
-          codedIn.readBool());
-    }
-  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java
index fc9547daca..38bcf24994 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java
@@ -206,42 +206,49 @@ public class AutoCodecProcessor extends AbstractProcessor {
         AutoCodecUtil.initializeSerializeMethodBuilder(encodedType);
     for (VariableElement parameter : fields) {
       TypeKind typeKind = parameter.asType().getKind();
-      switch (typeKind) {
-        case BOOLEAN:
-          serializeBuilder.addStatement(
-              "codedOut.writeBoolNoTag($T.getInstance().getBoolean(input, $L_offset))",
-              UnsafeProvider.class,
-              parameter.getSimpleName());
-          break;
-        case INT:
-          serializeBuilder.addStatement(
-              "codedOut.writeInt32NoTag($T.getInstance().getInt(input, $L_offset))",
-              UnsafeProvider.class,
-              parameter.getSimpleName());
-          break;
-        case ARRAY:
-          // fall through
-        case DECLARED:
-          serializeBuilder.addStatement(
-              "$T unsafe_$L = ($T) $T.getInstance().getObject(input, $L_offset)",
-              parameter.asType(),
-              parameter.getSimpleName(),
-              parameter.asType(),
-              UnsafeProvider.class,
-              parameter.getSimpleName());
-          marshallers.writeSerializationCode(
-              new Marshaller.Context(
-                  serializeBuilder, parameter.asType(), "unsafe_" + parameter.getSimpleName()));
-          break;
-        default:
-          throw new UnsupportedOperationException("Unimplemented or invalid kind: " + typeKind);
+      Optional<FieldValueAndClass> hasField =
+          getFieldByNameRecursive(encodedType, parameter.getSimpleName().toString());
+      if (hasField.isPresent()) {
+        switch (typeKind) {
+          case BOOLEAN:
+            serializeBuilder.addStatement(
+                "codedOut.writeBoolNoTag($T.getInstance().getBoolean(input, $L_offset))",
+                UnsafeProvider.class,
+                parameter.getSimpleName());
+            break;
+          case INT:
+            serializeBuilder.addStatement(
+                "codedOut.writeInt32NoTag($T.getInstance().getInt(input, $L_offset))",
+                UnsafeProvider.class,
+                parameter.getSimpleName());
+            break;
+          case ARRAY:
+            // fall through
+          case DECLARED:
+            serializeBuilder.addStatement(
+                "$T unsafe_$L = ($T) $T.getInstance().getObject(input, $L_offset)",
+                parameter.asType(),
+                parameter.getSimpleName(),
+                parameter.asType(),
+                UnsafeProvider.class,
+                parameter.getSimpleName());
+            marshallers.writeSerializationCode(
+                new Marshaller.Context(
+                    serializeBuilder, parameter.asType(), "unsafe_" + parameter.getSimpleName()));
+            break;
+          default:
+            throw new UnsupportedOperationException("Unimplemented or invalid kind: " + typeKind);
+        }
+      } else {
+        addSerializeParameterWithGetter(encodedType, parameter, serializeBuilder);
       }
     }
     return serializeBuilder.build();
   }
 
-  private String findGetterForAutoValue(VariableElement parameter, TypeElement type) {
-    List<ExecutableElement> methods = ElementFilter.methodsIn(type.getEnclosedElements());
+  private String findGetterForClass(VariableElement parameter, TypeElement type) {
+    List<ExecutableElement> methods =
+        ElementFilter.methodsIn(env.getElementUtils().getAllMembers(type));
 
     ImmutableList.Builder<String> possibleGetterNamesBuilder =
         ImmutableList.<String>builder().add(parameter.getSimpleName().toString());
@@ -262,7 +269,7 @@ public class AutoCodecProcessor extends AbstractProcessor {
     }
 
     throw new IllegalArgumentException(
-        "No AutoValue getter found corresponding to parameter " + parameter.getSimpleName());
+        type + ": No getter found corresponding to parameter " + parameter.getSimpleName());
   }
 
   private String addCamelCasePrefix(String name, String prefix) {
@@ -273,29 +280,34 @@ public class AutoCodecProcessor extends AbstractProcessor {
     }
   }
 
+  private void addSerializeParameterWithGetter(
+      TypeElement encodedType, VariableElement parameter, MethodSpec.Builder serializeBuilder) {
+    TypeKind typeKind = parameter.asType().getKind();
+    String getter = "input." + findGetterForClass(parameter, encodedType) + "()";
+    switch (typeKind) {
+      case BOOLEAN:
+        serializeBuilder.addStatement("codedOut.writeBoolNoTag($L)", getter);
+        break;
+      case INT:
+        serializeBuilder.addStatement("codedOut.writeInt32NoTag($L)", getter);
+        break;
+      case ARRAY:
+        // fall through
+      case DECLARED:
+        marshallers.writeSerializationCode(
+            new Marshaller.Context(serializeBuilder, parameter.asType(), getter));
+        break;
+      default:
+        throw new UnsupportedOperationException("Unimplemented or invalid kind: " + typeKind);
+    }
+  }
+
   private MethodSpec buildSerializeMethodWithInstantiatorForAutoValue(
       TypeElement encodedType, List<? extends VariableElement> fields) {
     MethodSpec.Builder serializeBuilder =
         AutoCodecUtil.initializeSerializeMethodBuilder(encodedType);
     for (VariableElement parameter : fields) {
-      TypeKind typeKind = parameter.asType().getKind();
-      String getter = "input." + findGetterForAutoValue(parameter, encodedType) + "()";
-      switch (typeKind) {
-        case BOOLEAN:
-          serializeBuilder.addStatement("codedOut.writeBoolNoTag($L)", getter);
-          break;
-        case INT:
-          serializeBuilder.addStatement("codedOut.writeInt32NoTag($L)", getter);
-          break;
-        case ARRAY:
-          // fall through
-        case DECLARED:
-          marshallers.writeSerializationCode(
-              new Marshaller.Context(serializeBuilder, parameter.asType(), getter));
-          break;
-        default:
-          throw new UnsupportedOperationException("Unimplemented or invalid kind: " + typeKind);
-      }
+      addSerializeParameterWithGetter(encodedType, parameter, serializeBuilder);
     }
     return serializeBuilder.build();
   }
@@ -462,7 +474,12 @@ public class AutoCodecProcessor extends AbstractProcessor {
       List<? extends VariableElement> parameters) {
     MethodSpec.Builder constructor = MethodSpec.constructorBuilder();
     for (VariableElement param : parameters) {
-      FieldValueAndClass field = getFieldByName(encodedType, param.getSimpleName().toString());
+      Optional<FieldValueAndClass> field =
+          getFieldByNameRecursive(encodedType, param.getSimpleName().toString());
+      if (!field.isPresent()) {
+        // Will attempt to use a getter for this field instead.
+        continue;
+      }
       builder.addField(
           TypeName.LONG, param.getSimpleName() + "_offset", Modifier.PRIVATE, Modifier.FINAL);
       constructor.beginControlFlow("try");
@@ -470,7 +487,7 @@ public class AutoCodecProcessor extends AbstractProcessor {
           "this.$L_offset = $T.getInstance().objectFieldOffset($T.class.getDeclaredField(\"$L\"))",
           param.getSimpleName(),
           UnsafeProvider.class,
-          ClassName.get(field.declaringClassType),
+          ClassName.get(field.get().declaringClassType),
           param.getSimpleName());
       constructor.nextControlFlow("catch ($T e)", NoSuchFieldException.class);
       constructor.addStatement("throw new $T(e)", IllegalStateException.class);
@@ -490,19 +507,6 @@ public class AutoCodecProcessor extends AbstractProcessor {
     }
   }
 
-  /**
-   * Returns the VariableElement for the field named {@code name}.
-   *
-   * <p>Throws IllegalArgumentException if no such field is found.
-   */
-  private FieldValueAndClass getFieldByName(TypeElement type, String name) {
-    return getFieldByNameRecursive(type, name)
-        .orElseThrow(
-            () ->
-                new IllegalArgumentException(
-                    type.getQualifiedName() + ": no field with name matching " + name));
-  }
-
   private Optional<FieldValueAndClass> getFieldByNameRecursive(TypeElement type, String name) {
     Optional<VariableElement> field =
         ElementFilter.fieldsIn(type.getEnclosedElements())