In AutoAnnotation, precompute the invariable part of the hashCode for an annotation where some members are being defaulted. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=147062302
diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoAnnotationTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoAnnotationTest.java index b7563bd..a8ae7b4 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoAnnotationTest.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoAnnotationTest.java
@@ -53,7 +53,7 @@ @Empty @StringValues("oops") - class AnnotatedClass {} + static class AnnotatedClass {} @Test public void testSimple() { @@ -423,4 +423,71 @@ AnnotatedWithClassesAnnotation.class.getAnnotation(ClassesAnnotation.class); assertEquals(fromReflect, generated); } + + @Retention(RetentionPolicy.RUNTIME) + @interface IntegersAnnotation { + int one() default Integer.MAX_VALUE; + int two() default Integer.MAX_VALUE; + int three(); + } + + @IntegersAnnotation(three = 23) + static class AnnotatedWithIntegersAnnotation {} + + @AutoAnnotation static IntegersAnnotation newIntegersAnnotation(int three) { + return new AutoAnnotation_AutoAnnotationTest_newIntegersAnnotation(three); + } + + @Test + public void testConstantOverflowInHashCode() { + IntegersAnnotation generated = newIntegersAnnotation(23); + IntegersAnnotation fromReflect = + AnnotatedWithIntegersAnnotation.class.getAnnotation(IntegersAnnotation.class); + new EqualsTester().addEqualityGroup(generated, fromReflect).testEquals(); + } + + @Retention(RetentionPolicy.RUNTIME) + @interface EverythingWithDefaults { + byte aByte() default 5; + short aShort() default 17; + int anInt() default 23; + long aLong() default 1729; + float aFloat() default 5; + double aDouble() default 17; + char aChar() default 'x'; + boolean aBoolean() default true; + String aString() default "whatever"; + RetentionPolicy anEnum() default RetentionPolicy.CLASS; + // We don't yet support defaulting annotation values. + // StringValues anAnnotation() default @StringValues({"foo", "bar"}); + byte[] bytes() default {1, 2}; + short[] shorts() default {3, 4}; + int[] ints() default {5, 6}; + long[] longs() default {7, 8}; + float[] floats() default {9, 10}; + double[] doubles() default {11, 12}; + char[] chars() default {'D', 'E'}; + boolean[] booleans() default {true, false}; + String[] strings() default {"vrai", "faux"}; + RetentionPolicy[] enums() default {RetentionPolicy.SOURCE, RetentionPolicy.CLASS}; + // We don't yet support defaulting annotation values. + // StringValues[] annotations() default { + // @StringValues({"foo", "bar"}), @StringValues({"baz", "buh"}) + // }; + } + + @EverythingWithDefaults + static class AnnotatedWithEverythingWithDefaults {} + + @AutoAnnotation static EverythingWithDefaults newEverythingWithDefaults() { + return new AutoAnnotation_AutoAnnotationTest_newEverythingWithDefaults(); + } + + @Test + public void testDefaultedValues() { + EverythingWithDefaults generated = newEverythingWithDefaults(); + EverythingWithDefaults fromReflect = + AnnotatedWithEverythingWithDefaults.class.getAnnotation(EverythingWithDefaults.class); + new EqualsTester().addEqualityGroup(generated, fromReflect).testEquals(); + } }
diff --git a/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java index 6ac32e4..a8936fe 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoAnnotationProcessor.java
@@ -19,6 +19,7 @@ import com.google.auto.common.SuperficialValidation; import com.google.auto.service.AutoService; import com.google.auto.value.AutoAnnotation; +import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; @@ -177,11 +178,70 @@ vars.pkg = pkg; vars.wrapperTypesUsedInCollections = wrapperTypesUsedInCollections; vars.gwtCompatible = isGwtCompatible(annotationElement); + ImmutableMap<String, Integer> invariableHashes = invariableHashes(members, parameters.keySet()); + vars.invariableHashSum = 0; + for (int h : invariableHashes.values()) { + vars.invariableHashSum += h; + } + vars.invariableHashes = invariableHashes.keySet(); String text = vars.toText(); text = Reformatter.fixup(text); writeSourceFile(pkg + "." + generatedClassName, text, methodClass); } + /** + * Returns the hashCode of the given AnnotationValue, if that hashCode is guaranteed to be always + * the same. The hashCode of a String or primitive type never changes. The hashCode of a Class + * or an enum constant does potentially change in different runs of the same program. The hashCode + * of an array doesn't change if the hashCodes of its elements don't. Although we could have a + * similar rule for nested annotation values, we currently don't. + */ + private static Optional<Integer> invariableHash(AnnotationValue annotationValue) { + Object value = annotationValue.getValue(); + if (value instanceof String || Primitives.isWrapperType(value.getClass())) { + return Optional.of(value.hashCode()); + } else if (value instanceof List<?>) { + @SuppressWarnings("unchecked") // by specification + List<? extends AnnotationValue> list = (List<? extends AnnotationValue>) value; + return invariableHash(list); + } else { + return Optional.absent(); + } + } + + private static Optional<Integer> invariableHash( + List<? extends AnnotationValue> annotationValues) { + int h = 1; + for (AnnotationValue annotationValue : annotationValues) { + Optional<Integer> maybeHash = invariableHash(annotationValue); + if (!maybeHash.isPresent()) { + return Optional.absent(); + } + h = h * 31 + maybeHash.get(); + } + return Optional.of(h); + } + + /** + * Returns a map from the names of members with invariable hashCodes to the values of those + * hashCodes. + */ + private static ImmutableMap<String, Integer> invariableHashes( + ImmutableMap<String, Member> members, ImmutableSet<String> parameters) { + ImmutableMap.Builder<String, Integer> builder = ImmutableMap.builder(); + for (String element : members.keySet()) { + if (!parameters.contains(element)) { + Member member = members.get(element); + AnnotationValue annotationValue = member.method.getDefaultValue(); + Optional<Integer> invariableHash = invariableHash(annotationValue); + if (invariableHash.isPresent()) { + builder.put(element, (element.hashCode() * 127) ^ invariableHash.get()); + } + } + } + return builder.build(); + } + private boolean methodsAreOverloaded(List<ExecutableElement> methods) { boolean overloaded = false; Set<String> classNames = new HashSet<String>();
diff --git a/value/src/main/java/com/google/auto/value/processor/AutoAnnotationTemplateVars.java b/value/src/main/java/com/google/auto/value/processor/AutoAnnotationTemplateVars.java index 6eee165..9617a20 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoAnnotationTemplateVars.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoAnnotationTemplateVars.java
@@ -84,6 +84,17 @@ */ Boolean gwtCompatible; + /** + * The names of members that are defaulted (not mentioned) in this {@code @AutoAnnotation}, + * and whose hash codes are invariable. + */ + Set<String> invariableHashes; + + /** + * The sum of the hash code contributions from the members in {@link #invariableHashes}. + */ + Integer invariableHashSum; + private static final Template TEMPLATE = parsedTemplateForResource("autoannotation.vm"); @Override
diff --git a/value/src/main/java/com/google/auto/value/processor/autoannotation.vm b/value/src/main/java/com/google/auto/value/processor/autoannotation.vm index bd8796c..f4a321b 100644 --- a/value/src/main/java/com/google/auto/value/processor/autoannotation.vm +++ b/value/src/main/java/com/google/auto/value/processor/autoannotation.vm
@@ -245,27 +245,31 @@ ${hc} ## #end +## The hashCode is the sum of two parts, an invariable part and a variable part. The invariable part +## comes from defaulted members (ones that don't appear in the @AutoAnnotation method parameters) +## whose values have hash codes that never change. (That doesn't include Class constants, for +## example.) We precompute the invariable part, as an optimization but also in order to avoid +## falling afoul of constant-overflow checks in the compiler. + @Override public int hashCode() { - #if ($members.isEmpty()) - - return 0; - - #else - return - #foreach ($m in $members) + ## If the invariable part is 0, we avoid outputting `return 0 + ...` just because it generates + ## unnecessary byte code. But if there are no members then we must say `return 0;` here. + #if ($invariableHashSum != 0 || $members.empty) - (#memberNameHash($m) ^ (#memberHashCodeExpression($m))) ## - #if ($foreach.hasNext) + #end + $invariableHashSum + // $invariableHashSum is the contribution from default members $invariableHashes #end + #foreach ($m in $members) + #if (!$invariableHashes.contains($m.toString())) + + + (#memberNameHash($m) ^ (#memberHashCodeExpression($m))) + // #memberNameHash($m) is 127 * "${m}".hashCode() + #end + #end + ; - #foreach ($m in $members) - - // #memberNameHash($m) is 127 * "${m}".hashCode() - #end - - #end }
diff --git a/value/src/test/java/com/google/auto/value/processor/AutoAnnotationCompilationTest.java b/value/src/test/java/com/google/auto/value/processor/AutoAnnotationCompilationTest.java index 81b92f5..107d013 100644 --- a/value/src/test/java/com/google/auto/value/processor/AutoAnnotationCompilationTest.java +++ b/value/src/test/java/com/google/auto/value/processor/AutoAnnotationCompilationTest.java
@@ -51,8 +51,10 @@ "", "public @interface MyAnnotation {", " MyEnum value();", + " int defaultedValue() default 23;", "}" ); + int invariableHash = ("defaultedValue".hashCode() * 127) ^ 23; JavaFileObject myEnumJavaFile = JavaFileObjects.forSourceLines( "com.example.enums.MyEnum", "package com.example.enums;", @@ -81,11 +83,12 @@ "", "import com.example.annotations.MyAnnotation;", "import com.example.enums.MyEnum;", - "import javax.annotation.Generated", + "import javax.annotation.Generated;", "", "@Generated(\"" + AutoAnnotationProcessor.class.getName() + "\")", "final class AutoAnnotation_AnnotationFactory_newMyAnnotation implements MyAnnotation {", " private final MyEnum value;", + " private static final int defaultedValue = 23;", "", " AutoAnnotation_AnnotationFactory_newMyAnnotation(MyEnum value) {", " if (value == null) {", @@ -102,6 +105,10 @@ " return value;", " }", "", + " @Override public int defaultedValue() {", + " return defaultedValue;", + " }", + "", " @Override public String toString() {", " StringBuilder sb = new StringBuilder(\"@com.example.annotations.MyAnnotation(\");", " sb.append(value);", @@ -114,13 +121,17 @@ " }", " if (o instanceof MyAnnotation) {", " MyAnnotation that = (MyAnnotation) o;", - " return (value.equals(that.value()));", + " return (value.equals(that.value()))", + " && (defaultedValue == that.defaultedValue());", " }", " return false;", " }", "", " @Override public int hashCode() {", - " return (" + 127 * "value".hashCode() + " ^ (value.hashCode()));", + " return ", + " " + invariableHash, + " + (" + 127 * "value".hashCode() + " ^ (value.hashCode()))", + " ;", " }", "}" ); @@ -211,7 +222,8 @@ " }", "", " @Override public int hashCode() {", - " return (" + 127 * "value".hashCode() + " ^ (Arrays.hashCode(value)));", + " return ", + " + (" + 127 * "value".hashCode() + " ^ (Arrays.hashCode(value)));", " }", "}" ); @@ -334,8 +346,8 @@ "", " @Override public int hashCode() {", " return ", - " (" + 127 * "value".hashCode() + " ^ (Arrays.hashCode(value))) +", - " (" + 127 * "enums".hashCode() + " ^ (Arrays.hashCode(enums)));", + " + (" + 127 * "value".hashCode() + " ^ (Arrays.hashCode(value)))", + " + (" + 127 * "enums".hashCode() + " ^ (Arrays.hashCode(enums)));", " }", "", " private static int[] intArrayFromCollection(Collection<Integer> c) {",