diff options
author | 2018-03-03 11:12:36 -0800 | |
---|---|---|
committer | 2018-03-03 11:14:47 -0800 | |
commit | 76de1ad3af6297cbc7c34054dd4340e02750a712 (patch) | |
tree | b68a171ae3c13de3c92b7a3c7d8b293ce4647a4d /src/main/java/com/google/devtools/build | |
parent | d5f374ce5caea11a0a760f121e1e75e68f107dab (diff) |
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
Diffstat (limited to 'src/main/java/com/google/devtools/build')
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 <Expression, Expression>; - * when processing a @SkylarkSignature annotation at build-time, <Object, SkylarkType>. + * <p>V is the class of defaultValues and T is the class of types. When parsing a function + * definition at compile-time, they are <Expression, Expression>; when processing + * a @SkylarkSignature annotation at build-time, <Object, SkylarkType>. */ + @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; |