Hook up @AutoCodec.Interner and add tests. PiperOrigin-RevId: 587806775 Change-Id: Ic2406762ca5b36ae4dec4ee0b6a85cf6ffe0e879
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodec.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodec.java index f7d8ffc..bb715a5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodec.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodec.java
@@ -63,6 +63,15 @@ @interface Instantiator {} /** + * Marks a static method to use for interning. + * + * <p>The method must accept an instance of the enclosing {@code AutoCodec} tagged class and + * return an instance of the tagged class. + */ + @Target({ElementType.METHOD}) + @interface Interner {} + + /** * Checks whether or not this class is allowed to be serialized. See {@link * com.google.devtools.build.lib.skyframe.serialization.SerializationContext#checkClassExplicitlyAllowed}. */
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 eebca4c..2654bcf 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
@@ -11,13 +11,14 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. - package com.google.devtools.build.lib.skyframe.serialization.autocodec; import static com.google.common.base.Ascii.toLowerCase; import static com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodecProcessor.InstantiatorKind.CONSTRUCTOR; import static com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodecProcessor.InstantiatorKind.FACTORY_METHOD; +import static com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodecProcessor.InstantiatorKind.INTERNER; import static com.google.devtools.build.lib.skyframe.serialization.autocodec.TypeOperations.getErasure; +import static com.google.devtools.build.lib.skyframe.serialization.autocodec.TypeOperations.getErasureAsMirror; import static com.google.devtools.build.lib.skyframe.serialization.autocodec.TypeOperations.sanitizeTypeParameter; import static com.google.devtools.build.lib.skyframe.serialization.autocodec.TypeOperations.writeGeneratedClassToFile; @@ -27,6 +28,7 @@ import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec; import com.google.devtools.build.lib.skyframe.serialization.SerializationException; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Instantiator; +import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Interner; import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationCodeGenerator.Marshaller; import com.google.devtools.build.lib.unsafe.UnsafeProvider; import com.squareup.javapoet.ClassName; @@ -117,6 +119,9 @@ case FACTORY_METHOD: codecClass = defineClassWithInstantiator(encodedType, instantiator.method(), annotation); break; + case INTERNER: + codecClass = defineClassWithInterner(encodedType, instantiator.method(), annotation); + break; default: throw new IllegalStateException( String.format("Unknown instantiator kind: %s\n", instantiator)); @@ -158,9 +163,16 @@ return codecClassBuilder.build(); } + private TypeSpec defineClassWithInterner( + TypeElement encodedType, ExecutableElement interner, AutoCodec annotation) + throws SerializationProcessingException { + return new InterningObjectCodecGenerator(env).defineCodec(encodedType, annotation, interner); + } + enum InstantiatorKind { CONSTRUCTOR, FACTORY_METHOD, + INTERNER } @AutoValue @@ -178,8 +190,8 @@ * Determines the {@link ResolvedInstantiator} by scanning the constructors and methods for * annotations. * - * <p>If there are no {@link Instantiator} annotation, falls back to checking for a unique - * constructor or throws otherwise. + * <p>Identifies the {@link Instantiator} or {@link Interner} annotations, throwing an exception + * if there are multiple. Falls back to checking for a unique constructor or throws otherwise. */ private ResolvedInstantiator determineInstantiator(TypeElement encodedType) throws SerializationProcessingException { @@ -208,7 +220,7 @@ if (markedMethod != null) { throw new SerializationProcessingException( encodedType, - "%s has multiple Instantiator annotations: %s %s.", + "%s has multiple Instantiator or Interner annotations: %s %s.", encodedType.getQualifiedName(), markedMethod.getSimpleName(), method.getSimpleName()); @@ -216,6 +228,19 @@ markedMethod = method; instantiatorKind = FACTORY_METHOD; } + if (hasInternerAnnotation(method)) { + verifyInterner(encodedType, method); + if (markedMethod != null) { + throw new SerializationProcessingException( + encodedType, + "%s has multiple Instantiator or Interner annotations: %s %s.", + encodedType.getQualifiedName(), + markedMethod.getSimpleName(), + method.getSimpleName()); + } + markedMethod = method; + instantiatorKind = INTERNER; + } } if (markedMethod != null) { @@ -226,7 +251,7 @@ if (constructors.size() > 1) { throw new SerializationProcessingException( encodedType, - "%s has multiple constructors but no Instantiator annotation.", + "%s has multiple constructors but no Instantiator or Interner annotation.", encodedType.getQualifiedName()); } // In Java, every class has at least one constructor, so this never fails. @@ -237,6 +262,10 @@ return elt.getAnnotation(Instantiator.class) != null; } + private static boolean hasInternerAnnotation(ExecutableElement elt) { + return elt.getAnnotation(Interner.class) != null; + } + private enum Relation { INSTANCE_OF, EQUAL_TO, @@ -300,6 +329,46 @@ } } + private void verifyInterner(TypeElement encodedType, ExecutableElement method) + throws SerializationProcessingException { + if (!method.getModifiers().contains(Modifier.STATIC)) { + throw new SerializationProcessingException( + encodedType, "%s is tagged @Interner, but it's not static.", method.getSimpleName()); + } + List<? extends VariableElement> parameters = method.getParameters(); + if (parameters.size() != 1) { + throw new SerializationProcessingException( + encodedType, + "%s is tagged @Interner, but it has %d parameters instead of 1.", + method.getSimpleName(), + parameters.size()); + } + TypeMirror subjectType = getErasureAsMirror(encodedType, env); + + // The method should be able to accept a value of encodedType; + TypeMirror parameterType = getErasureAsMirror(parameters.get(0).asType(), env); + if (!env.getTypeUtils().isAssignable(subjectType, parameterType)) { + throw new SerializationProcessingException( + encodedType, + "%s is tagged @Interner, but cannot accept a value of type %s because it is not" + + " assignable to %s.", + method.getSimpleName(), + encodedType, + parameterType); + } + + // The method should return a value that can be assigned to encodedType. + TypeMirror returnType = getErasureAsMirror(method.getReturnType(), env); + if (!env.getTypeUtils().isAssignable(returnType, subjectType)) { + throw new SerializationProcessingException( + encodedType, + "%s is tagged @Interner, but its return type %s cannot be assigned to type %s.", + method.getSimpleName(), + method.getReturnType(), + encodedType); + } + } + private MethodSpec buildSerializeMethodWithInstantiator( TypeElement encodedType, List<? extends VariableElement> fields, AutoCodec annotation) throws SerializationProcessingException {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/TypeOperations.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/TypeOperations.java index 3f5e3bc..8453544 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/TypeOperations.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/TypeOperations.java
@@ -98,14 +98,28 @@ return env.getTypeUtils().isSameType(type, getTypeMirror(clazz, env)); } + /** + * Returns the erased type. + * + * <p>This is {@link TypeName} rather than {@link TypeMirror} because it is more compatible with + * Javapoet's API methods. + */ static TypeName getErasure(TypeMirror type, ProcessingEnvironment env) { - return TypeName.get(env.getTypeUtils().erasure(type)); + return TypeName.get(getErasureAsMirror(type, env)); } static TypeName getErasure(TypeElement type, ProcessingEnvironment env) { return getErasure(type.asType(), env); } + static TypeMirror getErasureAsMirror(TypeElement type, ProcessingEnvironment env) { + return getErasureAsMirror(type.asType(), env); + } + + static TypeMirror getErasureAsMirror(TypeMirror type, ProcessingEnvironment env) { + return env.getTypeUtils().erasure(type); + } + static boolean isSerializableField(VariableElement variable) { Set<Modifier> modifiers = variable.getModifiers(); return !modifiers.contains(Modifier.STATIC) && !modifiers.contains(Modifier.TRANSIENT);