Support lazy computation of hasFixedSize

src/main/java/com/code_intelligence/jazzer/mutation/api/SerializingMutator.java

@@ -17,6 +17,7 @@
 package com.code_intelligence.jazzer.mutation.api;
 
 import com.google.errorprone.annotations.DoNotMock;
+import com.google.errorprone.annotations.ForOverride;
 
 /**
  * Combines a {@link ValueMutator} with a {@link Serializer} for objects of type {@code T}.
@@ -28,8 +29,36 @@
  */
 @DoNotMock("Use TestSupport#mockMutator instead")
 public abstract class SerializingMutator<T> implements Serializer<T>, ValueMutator<T> {
+  private Boolean cachedHasFixedSize;
+
   @Override
   public final String toString() {
     return Debuggable.getDebugString(this);
   }
+
+  @Override
+  public boolean hasFixedSize() {
+    if (cachedHasFixedSize != null) {
+      return cachedHasFixedSize;
+    }
+    // If the type to mutate is recursive, computeHasFixedSize() may call back into hasFixedSize().
+    // Ensure that the innermost call returns false to terminate the cycle and rely on all
+    // intermediate calls to propagate false up to the outermost call. This is safe since only the
+    // outermost call will ever reach this code (mutators are explicitly not thread-safe).
+    cachedHasFixedSize = false;
+    cachedHasFixedSize = computeHasFixedSize();
+    return cachedHasFixedSize;
+  }
+
+  /**
+   * Computes the value of {@link ValueMutator#hasFixedSize()} by inspecting the return value of
+   * that function for child mutators.
+   *
+   * <p>If the return value is a constant, override {@link ValueMutator#hasFixedSize()} directly.
+   */
+  @ForOverride
+  protected boolean computeHasFixedSize() {
+    throw new UnsupportedOperationException(
+        "Subclasses of SerializingMutator must override hasFixedSize or computeHasFixedSize");
+  }
 }

src/main/java/com/code_intelligence/jazzer/mutation/api/ValueMutator.java

@@ -87,6 +87,10 @@ public interface ValueMutator<T> extends Debuggable {
    *
    * <p>Examples of types with fixed size include primitive types, enums, and classes with only
    * primitive types and enums as members.
+   *
+   * <p>Note: Implementing classes should only override this method if the result does not depend on
+   * the value of {@code hasFixedSize()} for any child mutators. If it would, instead override
+   * {@link SerializingMutator#computeHasFixedSize()} to prevent issues when encountering a cycle.
    */
   boolean hasFixedSize();
 }

src/main/java/com/code_intelligence/jazzer/mutation/combinator/MutatorCombinators.java

@@ -182,9 +182,10 @@ public String toString() {
       };
     }
 
-    boolean hasFixedSize = stream(partialMutators).allMatch(InPlaceMutator::hasFixedSize);
     final InPlaceMutator<T>[] mutators = Arrays.copyOf(partialMutators, partialMutators.length);
     return new InPlaceMutator<T>() {
+      private Boolean cachedHasFixedSize;
+
       @Override
       public void initInPlace(T reference, PseudoRandom prng) {
         for (InPlaceMutator<T> mutator : mutators) {
@@ -204,9 +205,15 @@ public void crossOverInPlace(T reference, T otherReference, PseudoRandom prng) {
         }
       }
 
+      /** See comment on {@link SerializingMutator#hasFixedSize()}. */
       @Override
       public boolean hasFixedSize() {
-        return hasFixedSize;
+        if (cachedHasFixedSize != null) {
+          return cachedHasFixedSize;
+        }
+        cachedHasFixedSize = false;
+        cachedHasFixedSize = stream(partialMutators).allMatch(InPlaceMutator::hasFixedSize);
+        return cachedHasFixedSize;
       }
 
       @Override
@@ -482,39 +489,33 @@ public boolean hasFixedSize() {
    * @param serializer implementation of the {@link Serializer<T>} part
    * @param lazyMutator supplies the implementation of the {@link InPlaceMutator<T>} part. This is
    *     guaranteed to be invoked exactly once and only after {@code registerSelf}.
-   * @param hasFixedSize the value to return from the resulting mutators {@link
-   *     InPlaceMutator#hasFixedSize()}
    */
   public static <T> SerializingInPlaceMutator<T> assemble(
       Consumer<SerializingInPlaceMutator<T>> registerSelf,
       Supplier<T> makeDefaultInstance,
       Serializer<T> serializer,
-      Supplier<InPlaceMutator<T>> lazyMutator,
-      boolean hasFixedSize) {
+      Supplier<InPlaceMutator<T>> lazyMutator) {
     return new DelegatingSerializingInPlaceMutator<>(
-        registerSelf, makeDefaultInstance, serializer, lazyMutator, hasFixedSize);
+        registerSelf, makeDefaultInstance, serializer, lazyMutator);
   }
 
-  private static class DelegatingSerializingInPlaceMutator<T> extends SerializingInPlaceMutator<T> {
+  private static final class DelegatingSerializingInPlaceMutator<T>
+      extends SerializingInPlaceMutator<T> {
     private final Supplier<T> makeDefaultInstance;
     private final Serializer<T> serializer;
     private final InPlaceMutator<T> mutator;
-    private final boolean hasFixedSize;
 
     private DelegatingSerializingInPlaceMutator(
         Consumer<SerializingInPlaceMutator<T>> registerSelf,
         Supplier<T> makeDefaultInstance,
         Serializer<T> serializer,
-        Supplier<InPlaceMutator<T>> lazyMutator,
-        boolean hasFixedSize) {
+        Supplier<InPlaceMutator<T>> lazyMutator) {
       requireNonNull(makeDefaultInstance);
       requireNonNull(serializer);
 
       registerSelf.accept(this);
       this.makeDefaultInstance = makeDefaultInstance;
       this.serializer = serializer;
-      // Set before invoking the supplier as that can result in calls to hasFixedSize().
-      this.hasFixedSize = hasFixedSize;
       this.mutator = lazyMutator.get();
     }
 
@@ -534,10 +535,8 @@ public void crossOverInPlace(T reference, T otherReference, PseudoRandom prng) {
     }
 
     @Override
-    public boolean hasFixedSize() {
-      // This uses a fixed value rather than calling mutator.hasFixedSize() as this method is called
-      // before the constructor has finished, which is necessary in the case of a cycle.
-      return hasFixedSize;
+    protected boolean computeHasFixedSize() {
+      return mutator.hasFixedSize();
     }
 
     @Override

src/main/java/com/code_intelligence/jazzer/mutation/combinator/PostComposedMutator.java

@@ -62,7 +62,7 @@ public R crossOver(R value, R otherValue, PseudoRandom prng) {
   }
 
   @Override
-  public boolean hasFixedSize() {
+  protected boolean computeHasFixedSize() {
     return mutator.hasFixedSize();
   }
 

src/main/java/com/code_intelligence/jazzer/mutation/combinator/ProductMutator.java

@@ -41,13 +41,11 @@ public final class ProductMutator extends SerializingInPlaceMutator<Object[]> {
   private static final int INVERSE_PICK_VALUE_SUPPLIER_FREQUENCY = 100;
 
   private final SerializingMutator[] mutators;
-  private final boolean hasFixedSize;
 
   ProductMutator(SerializingMutator[] mutators) {
     requireNonNullElements(mutators);
     require(mutators.length > 0, "mutators must not be empty");
     this.mutators = Arrays.copyOf(mutators, mutators.length);
-    this.hasFixedSize = stream(mutators).allMatch(ValueMutator::hasFixedSize);
   }
 
   @Override
@@ -128,8 +126,8 @@ public void crossOverInPlace(Object[] reference, Object[] otherReference, Pseudo
   }
 
   @Override
-  public boolean hasFixedSize() {
-    return hasFixedSize;
+  protected boolean computeHasFixedSize() {
+    return stream(mutators).allMatch(ValueMutator::hasFixedSize);
   }
 
   @Override

src/main/java/com/code_intelligence/jazzer/mutation/mutator/lang/NullableMutatorFactory.java

@@ -110,7 +110,7 @@ public T crossOver(T value, T otherValue, PseudoRandom prng) {
     }
 
     @Override
-    public boolean hasFixedSize() {
+    protected boolean computeHasFixedSize() {
       return mutator.hasFixedSize();
     }
 

src/main/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorFactory.java

@@ -30,7 +30,6 @@
 import static com.code_intelligence.jazzer.mutation.mutator.proto.BuilderAdapters.setFieldWithPresence;
 import static com.code_intelligence.jazzer.mutation.mutator.proto.BuilderAdapters.setMapField;
 import static com.code_intelligence.jazzer.mutation.mutator.proto.TypeLibrary.getDefaultInstance;
-import static com.code_intelligence.jazzer.mutation.mutator.proto.TypeLibrary.isRecursiveField;
 import static com.code_intelligence.jazzer.mutation.mutator.proto.TypeLibrary.withoutInitIfRecursive;
 import static com.code_intelligence.jazzer.mutation.support.InputStreamSupport.cap;
 import static com.code_intelligence.jazzer.mutation.support.TypeSupport.asAnnotatedType;
@@ -65,7 +64,6 @@
 import com.google.protobuf.InvalidProtocolBufferException;
 import com.google.protobuf.Message;
 import com.google.protobuf.Message.Builder;
-import com.google.protobuf.MessageOrBuilder;
 import com.google.protobuf.UnknownFieldSet;
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
@@ -336,11 +334,6 @@ private SerializingMutator<Any.Builder> mutatorForAny(
             .boxed()
             .collect(toMap(i -> getTypeUrl(getDefaultInstance(anySource.value()[i])), identity()));
 
-    boolean hasFixedSize =
-        stream(anySource.value())
-            .map(TypeLibrary::getDefaultInstance)
-            .map(MessageOrBuilder::getDescriptorForType)
-            .allMatch(BuilderMutatorFactory::hasFixedSize);
     return assemble(
         mutator -> internedMutators.put(new CacheKey(Any.getDescriptor(), anySource), mutator),
         Any.getDefaultInstance()::toBuilder,
@@ -374,8 +367,7 @@ private SerializingMutator<Any.Builder> mutatorForAny(
                                 any.setValue(message.toByteString());
                               });
                         })
-                    .toArray(InPlaceMutator[]::new)),
-        hasFixedSize);
+                    .toArray(InPlaceMutator[]::new)));
   }
 
   private static String getTypeUrl(Message message) {
@@ -474,21 +466,7 @@ private SerializingMutator<?> makeBuilderMutator(
                                     ? new Annotation[0]
                                     : new Annotation[] {anySource},
                                 factory))
-                    .toArray(InPlaceMutator[]::new)),
-        hasFixedSize(descriptor));
-  }
-
-  private static boolean hasFixedSize(Descriptor descriptor) {
-    return descriptor.getFields().stream()
-        .noneMatch(
-            field ->
-                field.isMapField()
-                    || field.isRepeated()
-                    || isRecursiveField(field)
-                    || field.getJavaType() == JavaType.STRING
-                    || field.getJavaType() == JavaType.BYTE_STRING
-                    || (field.getJavaType() == JavaType.MESSAGE
-                        && !hasFixedSize(field.getMessageType())));
+                    .toArray(InPlaceMutator[]::new)));
   }
 
   private static final class CacheKey {

src/test/java/com/code_intelligence/jazzer/mutation/combinator/MutatorCombinatorsTest.java

@@ -323,8 +323,7 @@ public Foo detach(Foo value) {
                 return null;
               }
             },
-            () -> combine(valueMutator, listMutator),
-            false);
+            () -> combine(valueMutator, listMutator));
 
     assertThat(mutator.toString()).isEqualTo("{Foo.Integer, Foo via List<Integer>}");
 
@@ -381,8 +380,7 @@ public Foo detach(Foo value) {
                 return null;
               }
             },
-            () -> combine(valueMutator, listMutator),
-            false);
+            () -> combine(valueMutator, listMutator));
 
     Foo foo = new Foo(0, singletonList(0));
     Foo fooOther = new Foo(1, singletonList(1));

src/test/java/com/code_intelligence/jazzer/mutation/mutator/StressTest.java

@@ -520,7 +520,7 @@ public static Stream<Arguments> protoStressTestCases() {
             "{Builder.Nullable<Builder.{Builder.Boolean} -> Message |"
                 + " Builder.{Builder.Nullable<(cycle) -> Message>} -> Message -> Message>} ->"
                 + " Message",
-            false,
+            true,
             exactly(
                 AnyField3.getDefaultInstance(),
                 AnyField3.newBuilder()

src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorProto2Test.java

@@ -50,6 +50,7 @@ void testPrimitiveField() {
             FACTORY.createInPlaceOrThrow(
                 new TypeHolder<PrimitiveField2.@NotNull Builder>() {}.annotatedType());
     assertThat(mutator.toString()).isEqualTo("{Builder.Nullable<Boolean>}");
+    assertThat(mutator.hasFixedSize()).isTrue();
 
     PrimitiveField2.Builder builder = PrimitiveField2.newBuilder();
 
@@ -103,6 +104,7 @@ void testRequiredPrimitiveField() {
             FACTORY.createInPlaceOrThrow(
                 new TypeHolder<RequiredPrimitiveField2.@NotNull Builder>() {}.annotatedType());
     assertThat(mutator.toString()).isEqualTo("{Builder.Boolean}");
+    assertThat(mutator.hasFixedSize()).isTrue();
 
     RequiredPrimitiveField2.Builder builder = RequiredPrimitiveField2.newBuilder();
 
@@ -124,6 +126,7 @@ void testRepeatedPrimitiveField() {
             FACTORY.createInPlaceOrThrow(
                 new TypeHolder<RepeatedPrimitiveField2.@NotNull Builder>() {}.annotatedType());
     assertThat(mutator.toString()).isEqualTo("{Builder via List<Boolean>}");
+    assertThat(mutator.hasFixedSize()).isFalse();
 
     RepeatedPrimitiveField2.Builder builder = RepeatedPrimitiveField2.newBuilder();
 
@@ -175,6 +178,7 @@ void testMessageField() {
             FACTORY.createInPlaceOrThrow(
                 new TypeHolder<MessageField2.@NotNull Builder>() {}.annotatedType());
     assertThat(mutator.toString()).isEqualTo("{Builder.Nullable<{Builder.Boolean} -> Message>}");
+    assertThat(mutator.hasFixedSize()).isTrue();
 
     MessageField2.Builder builder = MessageField2.newBuilder();
 
@@ -225,6 +229,7 @@ void testRepeatedOptionalMessageField() {
                     RepeatedOptionalMessageField2.@NotNull Builder>() {}.annotatedType());
     assertThat(mutator.toString())
         .isEqualTo("{Builder via List<{Builder.Nullable<Boolean>} -> Message>}");
+    assertThat(mutator.hasFixedSize()).isFalse();
 
     RepeatedOptionalMessageField2.Builder builder = RepeatedOptionalMessageField2.newBuilder();
 
@@ -264,6 +269,7 @@ void testRepeatedRequiredMessageField() {
             FACTORY.createInPlaceOrThrow(
                 new TypeHolder<RepeatedMessageField2.@NotNull Builder>() {}.annotatedType());
     assertThat(mutator.toString()).isEqualTo("{Builder via List<{Builder.Boolean} -> Message>}");
+    assertThat(mutator.hasFixedSize()).isFalse();
 
     RepeatedMessageField2.Builder builder = RepeatedMessageField2.newBuilder();
 
@@ -328,6 +334,7 @@ void testRecursiveMessageField() {
                 new TypeHolder<RecursiveMessageField2.@NotNull Builder>() {}.annotatedType());
     assertThat(mutator.toString())
         .isEqualTo("{Builder.Boolean, WithoutInit(Builder.Nullable<(cycle) -> Message>)}");
+    assertThat(mutator.hasFixedSize()).isFalse();
     RecursiveMessageField2.Builder builder = RecursiveMessageField2.newBuilder();
 
     try (MockPseudoRandom prng =
@@ -396,6 +403,7 @@ void testOneOfField2() {
         .isEqualTo(
             "{Builder.Boolean, Builder.Nullable<Boolean>, Builder.Nullable<Boolean> |"
                 + " Builder.Nullable<{Builder.Boolean} -> Message>}");
+    assertThat(mutator.hasFixedSize()).isTrue();
     OneOfField2.Builder builder = OneOfField2.newBuilder();
 
     try (MockPseudoRandom prng =