Get rid of almost all Skylark codecs. We need to introduce a wrapper to turn ObjectCodec into a MEMOIZE_AFTER MemoizingCodec. I think that this is safe, because all the codecs that are being wrapped this way weren't memoizing anything internally that I could see.

In order to @AutoCodec the WithValue type, which is generic and can have null elements in lists, add functionality to @AutoCodec to deal with generic type static instantiators, matching generic type arguments (although I'm not sure why that wasn't already working), and null elements in lists.

PiperOrigin-RevId: 187740461

39 files changed, 388 insertions, 154 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
index 8f83d75985..22a03336ec 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
@@ -24,6 +24,7 @@ import com.google.common.collect.Sets;
 import com.google.common.collect.Sets.SetView;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.Concatable;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.EvalUtils;
@@ -161,6 +162,7 @@ public abstract class SkylarkInfo extends Info implements Concatable {
    * layout need be present on the instance.
    */
   @Immutable
+  @AutoCodec
   public static final class Layout {
 
     /**
@@ -176,12 +178,22 @@ public abstract class SkylarkInfo extends Info implements Concatable {
      * @throws IllegalArgumentException if any field names are given more than once
      */
     public Layout(Iterable<String> fields) {
+      this(makeMap(fields));
+    }
+
+    @AutoCodec.VisibleForSerialization
+    @AutoCodec.Instantiator
+    Layout(ImmutableMap<String, Integer> map) {
+      this.map = map;
+    }
+
+    private static ImmutableMap<String, Integer> makeMap(Iterable<String> fields) {
       ImmutableMap.Builder<String, Integer> layoutBuilder = ImmutableMap.builder();
       int i = 0;
       for (String field : fields) {
         layoutBuilder.put(field, i++);
       }
-      this.map = layoutBuilder.build();
+      return layoutBuilder.build();
     }
 
     @Override
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java
index 2bcbbbdcfb..212d536e56 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java
@@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.SkylarkInfo.Layout;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.FunctionSignature;
@@ -40,10 +41,11 @@ import javax.annotation.Nullable;
  * efficient since they do not use maps; see {@link SkylarkInfo}.
  *
  * <p>Exporting a {@code SkylarkProvider} creates a key that is used to uniquely identify it.
- * Usually a provider is exported by calling {@link #export}, but a test may wish to just create
- * a pre-exported provider directly. Exported providers use only their key for {@link #equals} and
+ * Usually a provider is exported by calling {@link #export}, but a test may wish to just create a
+ * pre-exported provider directly. Exported providers use only their key for {@link #equals} and
  * {@link #hashCode}.
  */
+@AutoCodec
 public class SkylarkProvider extends Provider implements SkylarkExportable {
 
   private static final FunctionSignature.WithValues<Object, SkylarkType> SCHEMALESS_SIGNATURE =
@@ -90,7 +92,8 @@ public class SkylarkProvider extends Provider implements SkylarkExportable {
    */
   public static SkylarkProvider createUnexportedSchemaful(
       Iterable<String> fields, Location location) {
-    return new SkylarkProvider(/*key=*/ null, fields, location);
+    return new SkylarkProvider(
+        /*key=*/ null, fields == null ? null : ImmutableList.copyOf(fields), location);
   }
 
   /**
@@ -115,7 +118,7 @@ public class SkylarkProvider extends Provider implements SkylarkExportable {
    */
   public static SkylarkProvider createExportedSchemaful(
       SkylarkKey key, Iterable<String> fields, Location location) {
-    return new SkylarkProvider(key, fields, location);
+    return new SkylarkProvider(key, fields == null ? null : ImmutableList.copyOf(fields), location);
   }
 
   /**
@@ -124,8 +127,10 @@ public class SkylarkProvider extends Provider implements SkylarkExportable {
    * <p>If {@code key} is null, the provider is unexported. If {@code fields} is null, the provider
    * is schemaless.
    */
-  private SkylarkProvider(
-      @Nullable SkylarkKey key, @Nullable Iterable<String> fields, Location location) {
+  @AutoCodec.Instantiator
+  @AutoCodec.VisibleForSerialization
+  SkylarkProvider(
+      @Nullable SkylarkKey key, @Nullable ImmutableList<String> fields, Location location) {
     // We override getName() in order to use the name that is assigned when export() is called.
     // Hence BaseFunction's constructor gets a null name.
     super(/*name=*/ null, buildSignature(fields), location);
@@ -136,7 +141,6 @@ public class SkylarkProvider extends Provider implements SkylarkExportable {
             : makeErrorMessageFormatForUnknownField(key.getExportedName());
   }
 
-
   private static FunctionSignature.WithValues<Object, SkylarkType> buildSignature(
       @Nullable Iterable<String> fields) {
     if (fields == null) {
@@ -253,6 +257,7 @@ public class SkylarkProvider extends Provider implements SkylarkExportable {
    * A serializable representation of Skylark-defined {@link SkylarkProvider} that uniquely
    * identifies all {@link SkylarkProvider}s that are exposed to SkyFrame.
    */
+  @AutoCodec
   public static class SkylarkKey extends Key {
     private final Label extensionLabel;
     private final String exportedName;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkFileDependency.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkFileDependency.java
index 0167aea98e..56d9a5cb17 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkFileDependency.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkFileDependency.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.util.Objects;
 
 /**
@@ -24,6 +25,7 @@ import java.util.Objects;
  *
  * <p>The dependency structure must be acyclic.
  */
+@AutoCodec
 public class SkylarkFileDependency {
 
   private final Label label;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/CodecScanner.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/CodecScanner.java
index 1a44467ba6..974a489062 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/CodecScanner.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/CodecScanner.java
@@ -204,6 +204,9 @@ public class CodecScanner {
     Type typeArg =
         ((ParameterizedType) codecType.getGenericInterfaces()[objectCodecIndex])
             .getActualTypeArguments()[0];
+    if (typeArg instanceof ParameterizedType) {
+      typeArg = ((ParameterizedType) typeArg).getRawType();
+    }
     if (!(typeArg instanceof Class)) {
       return null;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java
index e867969cdf..ad63fb1c72 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java
@@ -16,7 +16,9 @@ package com.google.devtools.build.lib.skyframe.serialization;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.skyframe.serialization.SerializationException.NoCodecException;
 import com.google.protobuf.CodedInputStream;
+import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
 
 /** Stateful class for providing additional context to a single deserialization "session". */
@@ -49,6 +51,50 @@ public class DeserializationContext {
         : constant;
   }
 
+  /**
+   * Returns an {@link ObjectCodec} appropriate for deserializing the next object on the wire. Only
+   * to be used by {@code MemoizingCodec}.
+   */
+  @SuppressWarnings("unchecked")
+  public <T> ObjectCodec<T> getCodecRecordedInInput(CodedInputStream codedIn)
+      throws IOException, NoCodecException {
+    int tag = codedIn.readSInt32();
+    if (tag == 0) {
+      return null;
+    }
+    T constant = (T) registry.maybeGetConstantByTag(tag);
+    return constant == null
+        ? (ObjectCodec<T>) registry.getCodecDescriptorByTag(tag).getCodec()
+        : new InitializedObjectCodec<>(constant);
+  }
+
+  private static class InitializedObjectCodec<T> implements ObjectCodec<T> {
+    private final T obj;
+    private boolean deserialized = false;
+
+    private InitializedObjectCodec(T obj) {
+      this.obj = obj;
+    }
+
+    @Override
+    public void serialize(SerializationContext context, T obj, CodedOutputStream codedOut) {
+      throw new UnsupportedOperationException("Unexpected serialize: " + obj);
+    }
+
+    @Override
+    public T deserialize(DeserializationContext context, CodedInputStream codedIn) {
+      Preconditions.checkState(!deserialized, "Deserialize called twice: %s", obj);
+      deserialized = true;
+      return obj;
+    }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    public Class<T> getEncodedClass() {
+      return (Class<T>) obj.getClass();
+    }
+  }
+
   @SuppressWarnings("unchecked")
   public <T> T getDependency(Class<T> type) {
     Preconditions.checkNotNull(type);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ImmutableListCodec.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ImmutableListCodec.java
index 0c67c38763..28e283f41a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ImmutableListCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ImmutableListCodec.java
@@ -24,7 +24,7 @@ public class ImmutableListCodec<T> implements ObjectCodec<ImmutableList<T>> {
 
   private final ObjectCodec<T> codec;
 
-  public ImmutableListCodec(ObjectCodec<T> codec) {
+  ImmutableListCodec(ObjectCodec<T> codec) {
     this.codec = codec;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistry.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistry.java
index f0968477d0..165831db86 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistry.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistry.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.skyframe.serialization;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.protobuf.ByteString;
@@ -237,6 +238,11 @@ public class ObjectCodecRegistry {
     public CodecDescriptor createDescriptor(int tag) {
       return new TypedCodecDescriptor<T>(tag, codec);
     }
+
+    @Override
+    public String toString() {
+      return MoreObjects.toStringHelper(this).add("codec", codec).toString();
+    }
   }
 
   /** Builder for {@link ObjectCodecRegistry}. */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java
index 60df5c8140..1759b0a947 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java
@@ -16,8 +16,10 @@ package com.google.devtools.build.lib.skyframe.serialization;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.skyframe.serialization.SerializationException.NoCodecException;
 import com.google.protobuf.CodedOutputStream;
 import java.io.IOException;
+import javax.annotation.Nullable;
 
 /** Stateful class for providing additional context to a single serialization "session". */
 // TODO(bazel-team): This class is just a shell, fill in.
@@ -36,21 +38,50 @@ public class SerializationContext {
     this(AutoRegistry.get(), dependencies);
   }
 
-  // TODO(shahan): consider making codedOut a member of this class.
-  public void serialize(Object object, CodedOutputStream codedOut)
-      throws IOException, SerializationException {
+  @Nullable
+  private ObjectCodecRegistry.CodecDescriptor recordAndGetDescriptorIfNotConstantOrNull(
+      @Nullable Object object, CodedOutputStream codedOut) throws IOException, NoCodecException {
     if (object == null) {
       codedOut.writeSInt32NoTag(0);
-      return;
+      return null;
     }
     Integer tag = registry.maybeGetTagForConstant(object);
     if (tag != null) {
       codedOut.writeSInt32NoTag(tag);
-      return;
+      return null;
     }
     ObjectCodecRegistry.CodecDescriptor descriptor = registry.getCodecDescriptor(object.getClass());
     codedOut.writeSInt32NoTag(descriptor.getTag());
-    descriptor.serialize(this, object, codedOut);
+    return descriptor;
+  }
+
+  // TODO(shahan): consider making codedOut a member of this class.
+  public void serialize(Object object, CodedOutputStream codedOut)
+      throws IOException, SerializationException {
+    ObjectCodecRegistry.CodecDescriptor descriptor =
+        recordAndGetDescriptorIfNotConstantOrNull(object, codedOut);
+    if (descriptor != null) {
+      descriptor.serialize(this, object, codedOut);
+    }
+  }
+
+  /**
+   * Returns the {@link ObjectCodec} to use when serializing {@code object}. The codec's tag is
+   * written to {@code codedOut} so that {@link DeserializationContext#getCodecRecordedInInput} can
+   * return the correct codec as well. Only to be used by {@code MemoizingCodec}.
+   *
+   * <p>If the return value is null, this indicates that the value was serialized directly as null
+   * or a constant. The caller needs do nothing further to serialize {@code object} in this case.
+   */
+  @SuppressWarnings("unchecked") // cast to ObjectCodec<? super T>.
+  public <T> ObjectCodec<? super T> recordAndMaybeReturnCodec(T object, CodedOutputStream codedOut)
+      throws SerializationException, IOException {
+    ObjectCodecRegistry.CodecDescriptor descriptor =
+        recordAndGetDescriptorIfNotConstantOrNull(object, codedOut);
+    if (descriptor == null) {
+      return null;
+    }
+    return (ObjectCodec<? super T>) descriptor.getCodec();
   }
 
   @SuppressWarnings("unchecked")
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 250e391adf..82ffce6529 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
@@ -222,14 +222,62 @@ public class AutoCodecProcessor extends AbstractProcessor {
     return elt.getAnnotation(AutoCodec.Instantiator.class) != null;
   }
 
+  private enum Relation {
+    INSTANCE_OF,
+    EQUAL_TO,
+    SUPERTYPE_OF,
+    UNRELATED_TO
+  }
+
+  private Relation findRelationWithGenerics(TypeMirror type1, TypeMirror type2) {
+    if (type1.getKind() == TypeKind.TYPEVAR || type2.getKind() == TypeKind.TYPEVAR) {
+      return Relation.EQUAL_TO;
+    }
+    if (env.getTypeUtils().isAssignable(type1, type2)) {
+      if (env.getTypeUtils().isAssignable(type2, type1)) {
+        return Relation.EQUAL_TO;
+      }
+      return Relation.INSTANCE_OF;
+    }
+    if (env.getTypeUtils().isAssignable(type2, type1)) {
+      return Relation.SUPERTYPE_OF;
+    }
+    // From here on out, we can't detect subtype/supertype, we're only checking for equality.
+    TypeMirror erasedType1 = env.getTypeUtils().erasure(type1);
+    TypeMirror erasedType2 = env.getTypeUtils().erasure(type2);
+    if (!env.getTypeUtils().isSameType(erasedType1, erasedType2)) {
+      // Technically, there could be a relationship, but it's too hard to figure out for now.
+      return Relation.UNRELATED_TO;
+    }
+    List<? extends TypeMirror> genericTypes1 = ((DeclaredType) type1).getTypeArguments();
+    List<? extends TypeMirror> genericTypes2 = ((DeclaredType) type2).getTypeArguments();
+    if (genericTypes1.size() != genericTypes2.size()) {
+      return null;
+    }
+    for (int i = 0; i < genericTypes1.size(); i++) {
+      Relation result = findRelationWithGenerics(genericTypes1.get(i), genericTypes2.get(i));
+      if (result != Relation.EQUAL_TO) {
+        return Relation.UNRELATED_TO;
+      }
+    }
+    return Relation.EQUAL_TO;
+  }
+
   private void verifyFactoryMethod(TypeElement encodedType, ExecutableElement elt) {
-    if (!elt.getModifiers().contains(Modifier.STATIC)
-        || !env.getTypeUtils().isSubtype(elt.getReturnType(), encodedType.asType())) {
+    boolean success = elt.getModifiers().contains(Modifier.STATIC);
+    if (success) {
+      Relation equalityTest = findRelationWithGenerics(elt.getReturnType(), encodedType.asType());
+      success = equalityTest == Relation.EQUAL_TO || equalityTest == Relation.INSTANCE_OF;
+    }
+    if (!success) {
       throw new IllegalArgumentException(
           encodedType.getQualifiedName()
               + " tags "
               + elt.getSimpleName()
-              + " as an Instantiator, but it's not a valid factory method.");
+              + " as an Instantiator, but it's not a valid factory method "
+              + elt.getReturnType()
+              + ", "
+              + encodedType.asType());
     }
   }
 
@@ -242,7 +290,8 @@ public class AutoCodecProcessor extends AbstractProcessor {
           getFieldByNameRecursive(encodedType, parameter.getSimpleName().toString());
       if (hasField.isPresent()) {
         Preconditions.checkArgument(
-            areTypesRelated(hasField.get().value.asType(), parameter.asType()),
+            findRelationWithGenerics(hasField.get().value.asType(), parameter.asType())
+                != Relation.UNRELATED_TO,
             "%s: parameter %s's type %s is unrelated to corresponding field type %s",
             encodedType.getQualifiedName(),
             parameter.getSimpleName(),
@@ -267,15 +316,6 @@ public class AutoCodecProcessor extends AbstractProcessor {
     return serializeBuilder.build();
   }
 
-  private boolean areTypesRelated(TypeMirror t1, TypeMirror t2) {
-    // If either type is generic, they are considered related.
-    // TODO(bazel-team): it may be possible to tighten this.
-    if (t1.getKind().equals(TypeKind.TYPEVAR) || t2.getKind().equals(TypeKind.TYPEVAR)) {
-      return true;
-    }
-    return env.getTypeUtils().isAssignable(t1, t2) || env.getTypeUtils().isAssignable(t2, t1);
-  }
-
   private TypeMirror sanitizeTypeParameterOfGenerics(TypeMirror type) {
     if (type instanceof TypeVariable) {
       return env.getTypeUtils().erasure(type);
@@ -309,14 +349,21 @@ public class AutoCodecProcessor extends AbstractProcessor {
     ImmutableList<String> possibleGetterNames = possibleGetterNamesBuilder.build();
 
     for (ExecutableElement element : methods) {
-      if (possibleGetterNames.contains(element.getSimpleName().toString())
-          && areTypesRelated(parameter.asType(), element.getReturnType())) {
+      if (!element.getModifiers().contains(Modifier.STATIC)
+          && !element.getModifiers().contains(Modifier.PRIVATE)
+          && possibleGetterNames.contains(element.getSimpleName().toString())
+          && findRelationWithGenerics(parameter.asType(), element.getReturnType())
+              != Relation.UNRELATED_TO) {
         return element.getSimpleName().toString();
       }
     }
 
     throw new IllegalArgumentException(
-        type + ": No getter found corresponding to parameter " + parameter.getSimpleName());
+        type
+            + ": No getter found corresponding to parameter "
+            + parameter.getSimpleName()
+            + ", "
+            + parameter.asType());
   }
 
   private static String addCamelCasePrefix(String name, String prefix) {
@@ -411,6 +458,7 @@ public class AutoCodecProcessor extends AbstractProcessor {
     if (!allThrown.isEmpty()) {
       builder.beginControlFlow("try");
     }
+    TypeName typeName = TypeName.get(env.getTypeUtils().erasure(type.asType()));
     String parameters =
         instantiator
             .getParameters()
@@ -418,14 +466,9 @@ public class AutoCodecProcessor extends AbstractProcessor {
             .map(AutoCodecProcessor::handleFromParameter)
             .collect(Collectors.joining(", "));
     if (instantiator.getKind().equals(ElementKind.CONSTRUCTOR)) {
-      builder.addStatement(
-          "return new $T($L)", TypeName.get(env.getTypeUtils().erasure(type.asType())), parameters);
+      builder.addStatement("return new $T($L)", typeName, parameters);
     } else { // Otherwise, it's a factory method.
-      builder.addStatement(
-          "return $T.$L($L)",
-          TypeName.get(type.asType()),
-          instantiator.getSimpleName(),
-          parameters);
+      builder.addStatement("return $T.$L($L)", typeName, instantiator.getSimpleName(), parameters);
     }
     if (!allThrown.isEmpty()) {
       for (TypeMirror thrown : allThrown) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/Marshallers.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/Marshallers.java
index d64fe9da4e..d160cc3519 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/Marshallers.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/Marshallers.java
@@ -36,7 +36,9 @@ import com.google.protobuf.ExtensionRegistryLite;
 import com.google.protobuf.ProtocolMessageEnum;
 import com.squareup.javapoet.TypeName;
 import java.nio.charset.Charset;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -104,13 +106,23 @@ class Marshallers {
   }
 
   /**
-   *  Writes out the deserialization loop and build code for any entity serialized as a list.
+   * Writes out the deserialization loop and build code for any entity serialized as a list.
    *
    * @param context context object for list with possibly another type.
    * @param repeated context for generic type deserialization.
    * @param builderName String for referencing the entity builder.
    */
-  void writeListDeserializationLoopAndBuild(Context context, Context repeated, String builderName) {
+  private void writeListDeserializationLoopAndBuild(
+      Context context, Context repeated, String builderName) {
+    if (matchesErased(context.getDeclaredType(), ImmutableList.class)) {
+      writeIterableDeserializationLoopWithoutNullsAndBuild(context, repeated, builderName);
+    } else {
+      writeListDeserializationLoopWithNullsAndBuild(context, repeated, builderName);
+    }
+  }
+
+  private void writeIterableDeserializationLoopWithoutNullsAndBuild(
+      Context context, Context repeated, String builderName) {
     String lengthName = context.makeName("length");
     context.builder.addStatement("int $L = codedIn.readInt32()", lengthName);
     String indexName = context.makeName("i");
@@ -122,6 +134,36 @@ class Marshallers {
     context.builder.addStatement("$L = $L.build()", context.name, builderName);
   }
 
+  private void writeListDeserializationLoopWithNullsAndBuild(
+      Context context, Context repeated, String builderName) {
+    String lengthName = context.makeName("length");
+    context.builder.addStatement("int $L = codedIn.readInt32()", lengthName);
+    String arrayListInCaseNull = context.makeName("arrayListInCaseNull");
+    context.builder.addStatement("$T $L = null", ArrayList.class, arrayListInCaseNull);
+    String indexName = context.makeName("i");
+    context.builder.beginControlFlow(
+        "for (int $L = 0; $L < $L; ++$L)", indexName, indexName, lengthName, indexName);
+    writeDeserializationCode(repeated);
+    context
+        .builder
+        .beginControlFlow("if ($L == null && $L == null)", repeated.name, arrayListInCaseNull)
+        .addStatement("$L = new ArrayList($L.build())", arrayListInCaseNull, builderName)
+        .endControlFlow()
+        .beginControlFlow("if ($L == null)", arrayListInCaseNull)
+        .addStatement("$L.add($L)", builderName, repeated.name)
+        .nextControlFlow("else")
+        .addStatement("$L.add($L)", arrayListInCaseNull, repeated.name)
+        .endControlFlow()
+        .endControlFlow()
+        .addStatement(
+            "$L = $L == null ? $L.build() : $T.unmodifiableList($L)",
+            context.name,
+            arrayListInCaseNull,
+            builderName,
+            Collections.class,
+            arrayListInCaseNull);
+  }
+
   private SerializationCodeGenerator getMatchingCodeGenerator(TypeMirror type) {
     if (type.getKind() == TypeKind.ARRAY) {
       return arrayCodeGenerator;
@@ -606,7 +648,7 @@ class Marshallers {
               repeated.getTypeName(),
               builderName,
               ImmutableSet.Builder.class);
-          writeListDeserializationLoopAndBuild(context, repeated, builderName);
+          writeIterableDeserializationLoopWithoutNullsAndBuild(context, repeated, builderName);
         }
       };
 
@@ -636,7 +678,7 @@ class Marshallers {
               builderName,
               ImmutableSortedSet.Builder.class,
               Comparator.class);
-          writeListDeserializationLoopAndBuild(context, repeated, builderName);
+          writeIterableDeserializationLoopWithoutNullsAndBuild(context, repeated, builderName);
         }
       };
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java
index 9f53513e87..ae8af0ae34 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 import java.io.Serializable;
 import java.util.ArrayList;
@@ -93,9 +94,8 @@ public abstract class AbstractComprehension extends Expression {
     void prettyPrint(Appendable buffer) throws IOException;
   }
 
-  /**
-   * A for clause in a comprehension, e.g. "for a in b" in the example above.
-   */
+  /** A for clause in a comprehension, e.g. "for a in b" in the example above. */
+  @AutoCodec
   public static final class ForClause implements Clause {
     private final LValue lvalue;
     private final Expression iterable;
@@ -158,9 +158,8 @@ public abstract class AbstractComprehension extends Expression {
     }
   }
 
-  /**
-   * A if clause in a comprehension, e.g. "if c" in the example above.
-   */
+  /** A if clause in a comprehension, e.g. "if c" in the example above. */
+  @AutoCodec
   public static final class IfClause implements Clause {
     private final Expression condition;
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Argument.java b/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
index a4d782a44f..141f763e50 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 import java.util.List;
 import javax.annotation.Nullable;
@@ -74,6 +75,7 @@ public abstract class Argument extends ASTNode {
   }
 
   /** positional argument: Expression */
+  @AutoCodec
   public static final class Positional extends Passed {
 
     public Positional(Expression value) {
@@ -92,6 +94,7 @@ public abstract class Argument extends ASTNode {
   }
 
   /** keyword argument: K = Expression */
+  @AutoCodec
   public static final class Keyword extends Passed {
 
     final String name;
@@ -120,6 +123,7 @@ public abstract class Argument extends ASTNode {
   }
 
   /** positional rest (starred) argument: *Expression */
+  @AutoCodec
   public static final class Star extends Passed {
 
     public Star(Expression value) {
@@ -139,6 +143,7 @@ public abstract class Argument extends ASTNode {
   }
 
   /** keyword rest (star_starred) parameter: **Expression */
+  @AutoCodec
   public static final class StarStar extends Passed {
 
     public StarStar(Expression value) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java
index b21ea1721e..2286f44d2a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java
@@ -14,11 +14,11 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
-/**
- * Syntax node for an assignment statement.
- */
+/** Syntax node for an assignment statement. */
+@AutoCodec
 public final class AssignmentStatement extends Statement {
 
   private final LValue lvalue;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java
index ec23b20f86..d23d78467c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java
@@ -14,9 +14,11 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
 /** Syntax node for an augmented assignment statement. */
+@AutoCodec
 public final class AugmentedAssignmentStatement extends Statement {
 
   private final Operator operator;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
index b0c82c58d0..ab5badcb92 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
@@ -15,15 +15,15 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Strings;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.Concatable.Concatter;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import java.io.IOException;
 import java.util.IllegalFormatException;
 
-/**
- * Syntax node for a binary operator expression.
- */
+/** Syntax node for a binary operator expression. */
+@AutoCodec
 public final class BinaryOperatorExpression extends Expression {
 
   private final Expression lhs;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java
index e5a0b4c116..0f016b9c58 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java
@@ -13,11 +13,11 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
-/**
- * Syntax node for an if/else expression.
- */
+/** Syntax node for an if/else expression. */
+@AutoCodec
 public final class ConditionalExpression extends Expression {
 
   // Python conditional expressions: $thenCase if $condition else $elseCase
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java
index 040485f814..f1125568e3 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java
@@ -15,13 +15,13 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
 
-/**
- * Syntax node for dictionary comprehension expressions.
- */
+/** Syntax node for dictionary comprehension expressions. */
+@AutoCodec
 public final class DictComprehension extends AbstractComprehension {
   private final Expression keyExpression;
   private final Expression valueExpression;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java
index 79a1bf14fb..410408c060 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java
@@ -15,15 +15,16 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 import java.util.List;
 
-/**
- * Syntax node for dictionary literals.
- */
+/** Syntax node for dictionary literals. */
+@AutoCodec
 public final class DictionaryLiteral extends Expression {
 
   /** Node for an individual key-value pair in a dictionary literal. */
+  @AutoCodec
   public static final class DictionaryEntryLiteral extends ASTNode {
 
     private final Expression key;
@@ -57,8 +58,8 @@ public final class DictionaryLiteral extends Expression {
 
   private final ImmutableList<DictionaryEntryLiteral> entries;
 
-  public DictionaryLiteral(List<DictionaryEntryLiteral> exprs) {
-    this.entries = ImmutableList.copyOf(exprs);
+  public DictionaryLiteral(List<DictionaryEntryLiteral> entries) {
+    this.entries = ImmutableList.copyOf(entries);
   }
 
   /** A new literal for an empty dictionary, onto which a new location can be specified */
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
index 8aa0ad92ec..06659aee2e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
@@ -15,12 +15,14 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.Streams;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.FuncallExpression.MethodDescriptor;
 import com.google.devtools.build.lib.util.SpellChecker;
 import java.io.IOException;
 import java.util.Optional;
 
 /** Syntax node for a dot expression. e.g. obj.field, but not obj.method() */
+@AutoCodec
 public final class DotExpression extends Expression {
 
   private final Expression object;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java
index 54f4fb72a5..971e72dd40 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java
@@ -14,11 +14,11 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
-/**
- * Syntax node for a function call statement. Used for build rules.
- */
+/** Syntax node for a function call statement. Used for build rules. */
+@AutoCodec
 public final class ExpressionStatement extends Statement {
 
   private final Expression expression;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java
index 1022caf1d6..71ffe661ca 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java
@@ -13,11 +13,11 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
-/**
- * A class for flow statements (e.g. break and continue)
- */
+/** A class for flow statements (e.g. break and continue) */
+@AutoCodec
 public final class FlowStatement extends Statement {
   // TODO(laurentlb): This conflicts with Statement.Kind, maybe remove it?
   public enum Kind {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java
index f1eb10bfcf..d2e06a033d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java
@@ -15,12 +15,12 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 import java.util.List;
 
-/**
- * Syntax node for a for loop statement.
- */
+/** Syntax node for a for loop statement. */
+@AutoCodec
 public final class ForStatement extends Statement {
 
   private final LValue variable;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 68e5006ead..2639c2735d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -26,6 +26,7 @@ import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.ParamType;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
@@ -52,9 +53,8 @@ import java.util.concurrent.ExecutionException;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
-/**
- * Syntax node for a function call expression.
- */
+/** Syntax node for a function call expression. */
+@AutoCodec
 public final class FuncallExpression extends Expression {
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
index 0f27ada367..5d856ede75 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
@@ -14,11 +14,11 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
-/**
- * Syntax node for a function definition.
- */
+/** Syntax node for a function definition. */
+@AutoCodec
 public final class FunctionDefStatement extends Statement {
 
   private final Identifier identifier;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
index 64c9405b75..42155ecc4c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
@@ -20,6 +20,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Interner;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.Printer.BasePrinter;
 import com.google.devtools.build.lib.util.StringCanonicalizer;
 import java.io.Serializable;
@@ -33,60 +34,65 @@ import javax.annotation.Nullable;
 /**
  * Function Signatures for BUILD language (same as Python)
  *
- * <p>Skylark's function signatures are just like Python3's.
- * A function may have 6 kinds of arguments:
- * positional mandatory, positional optional, positional rest (aka *star argument),
- * key-only mandatory, key-only optional, key rest (aka **star_star argument).
- * A caller may specify all arguments but the *star and **star_star arguments by name,
- * and thus all mandatory and optional arguments are named arguments.
+ * <p>Skylark's function signatures are just like Python3's. A function may have 6 kinds of
+ * arguments: positional mandatory, positional optional, positional rest (aka *star argument),
+ * key-only mandatory, key-only optional, key rest (aka **star_star argument). A caller may specify
+ * all arguments but the *star and **star_star arguments by name, and thus all mandatory and
+ * optional arguments are named arguments.
+ *
+ * <p>To enable various optimizations in the argument processing routine, we sort arguments
+ * according the following constraints, enabling corresponding optimizations:
  *
- * <p>To enable various optimizations in the argument processing routine,
- * we sort arguments according the following constraints, enabling corresponding optimizations:
  * <ol>
- * <li>The positional mandatories come just before the positional optionals,
- *   so they can be filled in one go.
- * <li>Positionals come first, so it's easy to prepend extra positional arguments such as "self"
- *   to an argument list, and we optimize for the common case of no key-only mandatory parameters.
- *   key-only parameters are thus grouped together.
- *   positional mandatory and key-only mandatory parameters are separate,
- *   but there is no loop over a contiguous chunk of them, anyway.
- * <li>The named are all grouped together, with star and star_star rest arguments coming last.
- * <li>Mandatory arguments in each category (positional and named-only) come before the optional
- *   arguments, for the sake of slightly better clarity to human implementers. This eschews an
- *   optimization whereby grouping optionals together allows to iterate over them in one go instead
- *   of two; however, this relatively minor optimization only matters when keyword arguments are
- *   passed, at which point it is dwarfed by the slowness of keyword processing.
+ *   <li>The positional mandatories come just before the positional optionals, so they can be filled
+ *       in one go.
+ *   <li>Positionals come first, so it's easy to prepend extra positional arguments such as "self"
+ *       to an argument list, and we optimize for the common case of no key-only mandatory
+ *       parameters. key-only parameters are thus grouped together. positional mandatory and
+ *       key-only mandatory parameters are separate, but there is no loop over a contiguous chunk of
+ *       them, anyway.
+ *   <li>The named are all grouped together, with star and star_star rest arguments coming last.
+ *   <li>Mandatory arguments in each category (positional and named-only) come before the optional
+ *       arguments, for the sake of slightly better clarity to human implementers. This eschews an
+ *       optimization whereby grouping optionals together allows to iterate over them in one go
+ *       instead of two; however, this relatively minor optimization only matters when keyword
+ *       arguments are passed, at which point it is dwarfed by the slowness of keyword processing.
  * </ol>
  *
- * <p>Parameters are thus sorted in the following order:
- * positional mandatory arguments (if any), positional optional arguments (if any),
- * key-only mandatory arguments (if any), key-only optional arguments (if any),
- * then star argument (if any), then star_star argument (if any).
+ * <p>Parameters are thus sorted in the following order: positional mandatory arguments (if any),
+ * positional optional arguments (if any), key-only mandatory arguments (if any), key-only optional
+ * arguments (if any), then star argument (if any), then star_star argument (if any).
  */
+@AutoCodec
 @AutoValue
 public abstract class FunctionSignature implements Serializable {
 
-  /**
-   * The shape of a FunctionSignature, without names
-   */
+  /** The shape of a FunctionSignature, without names */
   @AutoValue
+  @AutoCodec
   public abstract static class Shape implements Serializable {
     private static final Interner<Shape> interner = BlazeInterners.newWeakInterner();
 
     /** Create a function signature */
+    @AutoCodec.Instantiator
     public static Shape create(
         int mandatoryPositionals,
         int optionalPositionals,
         int mandatoryNamedOnly,
         int optionalNamedOnly,
-        boolean starArg,
-        boolean kwArg) {
+        boolean hasStarArg,
+        boolean hasKwArg) {
       Preconditions.checkArgument(
           0 <= mandatoryPositionals && 0 <= optionalPositionals
           && 0 <= mandatoryNamedOnly && 0 <= optionalNamedOnly);
-      return interner.intern(new AutoValue_FunctionSignature_Shape(
-          mandatoryPositionals, optionalPositionals,
-          mandatoryNamedOnly, optionalNamedOnly, starArg, kwArg));
+      return interner.intern(
+          new AutoValue_FunctionSignature_Shape(
+              mandatoryPositionals,
+              optionalPositionals,
+              mandatoryNamedOnly,
+              optionalNamedOnly,
+              hasStarArg,
+              hasKwArg));
     }
 
     // These abstract getters specify the actual argument count fields to be defined by AutoValue.
@@ -153,9 +159,10 @@ public abstract class FunctionSignature implements Serializable {
   /**
    * Signatures proper.
    *
-   * <p>A signature is a Shape and an ImmutableList of argument variable names
-   * NB: we assume these lists are short, so we may do linear scans.
+   * <p>A signature is a Shape and an ImmutableList of argument variable names NB: we assume these
+   * lists are short, so we may do linear scans.
    */
+  @AutoCodec.Instantiator
   public static FunctionSignature create(Shape shape, ImmutableList<String> names) {
     Preconditions.checkArgument(names.size() == shape.getArguments());
     return signatureInterner.intern(new AutoValue_FunctionSignature(shape, names(names)));
@@ -187,13 +194,14 @@ public abstract class FunctionSignature implements Serializable {
    *
    * <p>The lists can be null, which is an optimized path for specifying all null values.
    *
-   * <p>Note that if some values can be null (for BuiltinFunction, not for UserDefinedFunction),
-   * you should use an ArrayList; otherwise, we recommend an ImmutableList.
+   * <p>Note that if some values can be null (for BuiltinFunction, not for UserDefinedFunction), you
+   * should use an ArrayList; otherwise, we recommend an ImmutableList.
    *
-   * <p>V is the class of defaultValues and T is the class of types.
-   * When parsing a function definition at compile-time, they are &lt;Expression, Expression&gt;;
-   * when processing a @SkylarkSignature annotation at build-time, &lt;Object, SkylarkType&gt;.
+   * <p>V is the class of defaultValues and T is the class of types. When parsing a function
+   * definition at compile-time, they are &lt;Expression, Expression&gt;; when processing
+   * a @SkylarkSignature annotation at build-time, &lt;Object, SkylarkType&gt;.
    */
+  @AutoCodec
   @AutoValue
   public abstract static class WithValues<V, T> implements Serializable {
 
@@ -232,8 +240,14 @@ public abstract class FunctionSignature implements Serializable {
         copiedTypes.addAll(types);
         convertedTypes = Collections.unmodifiableList(copiedTypes);
       }
-      return new AutoValue_FunctionSignature_WithValues<>(
-          signature, convertedDefaultValues, convertedTypes);
+      return createInternal(signature, convertedDefaultValues, convertedTypes);
+    }
+
+    @AutoCodec.VisibleForSerialization
+    @AutoCodec.Instantiator
+    static <V, T> WithValues<V, T> createInternal(
+        FunctionSignature signature, @Nullable List<V> defaultValues, @Nullable List<T> types) {
+      return new AutoValue_FunctionSignature_WithValues<>(signature, defaultValues, types);
     }
 
     public static <V, T> WithValues<V, T> create(FunctionSignature signature) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
index 1bbf78c225..9f167e83b5 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.util.SpellChecker;
 import java.io.IOException;
 import java.util.Set;
@@ -29,10 +30,11 @@ import javax.annotation.Nullable;
 /**
  * Syntax node for an identifier.
  *
- * Unlike most {@link ASTNode} subclasses, this one supports {@link Object#equals} and {@link
+ * <p>Unlike most {@link ASTNode} subclasses, this one supports {@link Object#equals} and {@link
  * Object#hashCode} (but note that these methods ignore location information). They are needed
  * because {@code Identifier}s are stored in maps when constructing {@link LoadStatement}.
  */
+@AutoCodec
 public final class Identifier extends Expression {
 
   private final String name;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java
index 5055f6a39d..b4377d0531 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java
@@ -15,12 +15,12 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 import java.util.List;
 
-/**
- * Syntax node for an if/else statement.
- */
+/** Syntax node for an if/else statement. */
+@AutoCodec
 public final class IfStatement extends Statement {
 
   /**
@@ -29,6 +29,7 @@ public final class IfStatement extends Statement {
    * <p>This extends Statement, but it is not actually an independent statement in the grammar. We
    * should probably eliminate it in favor of a recursive representation of if/else chains.
    */
+  @AutoCodec
   public static final class ConditionalStatements extends Statement {
 
     private final Expression condition;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java
index efb1a64edf..0c4a7f628c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
 /**
@@ -21,6 +22,7 @@ import java.io.IOException;
  * obj[from:to]}). The object may be either a sequence or an associative mapping (most commonly
  * lists and dictionaries).
  */
+@AutoCodec
 public final class IndexExpression extends Expression {
 
   private final Expression object;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java
index ea88509f77..b642dd8649 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java
@@ -13,15 +13,15 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
-/**
- * Syntax node for an integer literal.
- */
+/** Syntax node for an integer literal. */
+@AutoCodec
 public final class IntegerLiteral extends Expression {
   private final int value;
 
-  public IntegerLiteral(Integer value) {
+  public IntegerLiteral(int value) {
     this.value = value;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
index 4cc38f4968..34ccaa8b36 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.syntax;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import java.io.IOException;
 import java.util.Collection;
@@ -24,6 +25,7 @@ import java.util.Collection;
 /**
  * A term that can appear on the left-hand side of an assignment statement, for loop, comprehension
  * clause, etc. E.g.,
+ *
  * <ul>
  *   <li>{@code lvalue = 2}
  *   <li>{@code [for lvalue in exp]}
@@ -31,16 +33,19 @@ import java.util.Collection;
  * </ul>
  *
  * <p>An {@code LValue}'s expression must have one of the following forms:
+ *
  * <ul>
  *   <li>(Variable assignment) an {@link Identifier};
  *   <li>(List or dictionary item assignment) an {@link IndexExpression}; or
  *   <li>(Sequence assignment) a non-empty {@link ListLiteral} (either list or tuple) of expressions
  *       that can themselves appear in an {@code LValue}.
  * </ul>
+ *
  * In particular and unlike Python, slice expressions, dot expressions, and starred expressions
  * cannot appear in {@code LValue}s.
  */
 // TODO(bazel-team): Add support for assigning to slices (e.g. a[2:6] = [3]).
+@AutoCodec
 public final class LValue extends ASTNode {
 
   private final Expression expr;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
index 618913cd68..8141bd9cd9 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
@@ -15,14 +15,14 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
-/**
- * Syntax node for lists comprehension expressions.
- */
+/** Syntax node for lists comprehension expressions. */
+@AutoCodec
 public final class ListComprehension extends AbstractComprehension {
   private final Expression outputExpression;
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
index a8d338acc2..ac367201e8 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import java.io.IOException;
@@ -23,10 +24,10 @@ import java.util.List;
 /**
  * Syntax node for list and tuple literals.
  *
- * <p>(Note that during evaluation, both list and tuple values are represented by
- * java.util.List objects, the only difference between them being whether or not
- * they are mutable.)
+ * <p>(Note that during evaluation, both list and tuple values are represented by java.util.List
+ * objects, the only difference between them being whether or not they are mutable.)
  */
+@AutoCodec
 public final class ListLiteral extends Expression {
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
index 180802141d..c9669fcd56 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
@@ -15,12 +15,12 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 import java.util.Map;
 
-/**
- * Syntax node for an import statement.
- */
+/** Syntax node for an import statement. */
+@AutoCodec
 public final class LoadStatement extends Statement {
 
   private final ImmutableMap<Identifier, String> symbolMap;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java b/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
index e16fe585b0..e5ffc1ca57 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 import javax.annotation.Nullable;
 
@@ -37,10 +38,6 @@ public abstract class Parameter<V, T> extends Argument {
     this.name = name;
     this.type = type;
   }
-  private Parameter(@Nullable String name) {
-    this.name = name;
-    this.type = null;
-  }
 
   public boolean isMandatory() {
     return false;
@@ -80,12 +77,14 @@ public abstract class Parameter<V, T> extends Argument {
   }
 
   /** mandatory parameter (positional or key-only depending on position): Ident */
+  @AutoCodec
   public static final class Mandatory<V, T> extends Parameter<V, T> {
 
     public Mandatory(String name) {
-      super(name);
+      this(name, null);
     }
 
+    @AutoCodec.Instantiator
     public Mandatory(String name, @Nullable T type) {
       super(name, type);
     }
@@ -102,15 +101,16 @@ public abstract class Parameter<V, T> extends Argument {
   }
 
   /** optional parameter (positional or key-only depending on position): Ident = Value */
+  @AutoCodec
   public static final class Optional<V, T> extends Parameter<V, T> {
 
     public final V defaultValue;
 
     public Optional(String name, @Nullable V defaultValue) {
-      super(name);
-      this.defaultValue = defaultValue;
+      this(name, null, defaultValue);
     }
 
+    @AutoCodec.Instantiator
     public Optional(String name, @Nullable T type, @Nullable V defaultValue) {
       super(name, type);
       this.defaultValue = defaultValue;
@@ -145,14 +145,16 @@ public abstract class Parameter<V, T> extends Argument {
   }
 
   /** extra positionals parameter (star): *identifier */
+  @AutoCodec
   public static final class Star<V, T> extends Parameter<V, T> {
 
+    @AutoCodec.Instantiator
     public Star(@Nullable String name, @Nullable T type) {
       super(name, type);
     }
 
     public Star(@Nullable String name) {
-      super(name);
+      this(name, null);
     }
 
     @Override
@@ -175,14 +177,16 @@ public abstract class Parameter<V, T> extends Argument {
   }
 
   /** extra keywords parameter (star_star): **identifier */
+  @AutoCodec
   public static final class StarStar<V, T> extends Parameter<V, T> {
 
+    @AutoCodec.Instantiator
     public StarStar(String name, @Nullable T type) {
       super(name, type);
     }
 
     public StarStar(String name) {
-      super(name);
+      this(name, null);
     }
 
     @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/PassStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/PassStatement.java
index e0351002ef..f7854e8dda 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/PassStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/PassStatement.java
@@ -14,9 +14,11 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
 /** Syntax node for a `pass` statement. */
+@AutoCodec
 public class PassStatement extends Statement {
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java
index 10b1fd26fa..37d462964b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java
@@ -14,12 +14,12 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 import javax.annotation.Nullable;
 
-/**
- * A wrapper Statement class for return expressions.
- */
+/** A wrapper Statement class for return expressions. */
+@AutoCodec
 public final class ReturnStatement extends Statement {
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java
index 54c194a021..f4074e9499 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java
@@ -14,11 +14,13 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 import java.util.List;
 import javax.annotation.Nullable;
 
 /** Syntax node for a slice expression, e.g. obj[:len(obj):2]. */
+@AutoCodec
 public final class SliceExpression extends Expression {
 
   private final Expression object;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java
index 26144ec02d..db1c23a289 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java
@@ -13,11 +13,11 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
-/**
- * Syntax node for a string literal.
- */
+/** Syntax node for a string literal. */
+@AutoCodec
 public final class StringLiteral extends Expression {
   String value;
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java
index 824333a9d8..08fcf457b9 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java
@@ -14,9 +14,11 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import java.io.IOException;
 
 /** Syntax node for a unary operator expression. */
+@AutoCodec
 public final class UnaryOperatorExpression extends Expression {
 
   private final UnaryOperator operator;