Version 1.4.75 Cherry-pick: Disallow direct manipulation of field arrays. Also, verify the absence of duplicate fields. CL: https://r8-review.googlesource.com/35700 Cherry-pick: Remove a wrong logging. CL: https://r8-review.googlesource.com/c/r8/+/35801 Bug: 127932803 Change-Id: I7ba45c73fe7b464ab271194acf737e6aa1da023e
diff --git a/src/main/java/com/android/tools/r8/ResourceShrinker.java b/src/main/java/com/android/tools/r8/ResourceShrinker.java index 6267db1..d7805b3 100644 --- a/src/main/java/com/android/tools/r8/ResourceShrinker.java +++ b/src/main/java/com/android/tools/r8/ResourceShrinker.java
@@ -233,11 +233,11 @@ private void processAnnotations(DexProgramClass classDef) { Stream<DexAnnotation> instanceFieldAnnotations = - Arrays.stream(classDef.instanceFields()) + classDef.instanceFields().stream() .filter(DexEncodedField::hasAnnotation) .flatMap(f -> Arrays.stream(f.annotations.annotations)); Stream<DexAnnotation> staticFieldAnnotations = - Arrays.stream(classDef.staticFields()) + classDef.staticFields().stream() .filter(DexEncodedField::hasAnnotation) .flatMap(f -> Arrays.stream(f.annotations.annotations)); Stream<DexAnnotation> virtualMethodAnnotations =
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java index 3acb55b..3f82b47 100644 --- a/src/main/java/com/android/tools/r8/Version.java +++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@ // This field is accessed from release scripts using simple pattern matching. // Therefore, changing this field could break our release scripts. - public static final String LABEL = "1.4.74"; + public static final String LABEL = "1.4.75"; private Version() { }
diff --git a/src/main/java/com/android/tools/r8/dex/FileWriter.java b/src/main/java/com/android/tools/r8/dex/FileWriter.java index e8ebbbb..9e7c09b 100644 --- a/src/main/java/com/android/tools/r8/dex/FileWriter.java +++ b/src/main/java/com/android/tools/r8/dex/FileWriter.java
@@ -562,8 +562,8 @@ item -> mixedSectionOffsets.getOffsetFor(item.parameterAnnotationsList)); } - private void writeEncodedFields(DexEncodedField[] fields) { - assert PresortedComparable.isSorted(Arrays.asList(fields)); + private void writeEncodedFields(List<DexEncodedField> fields) { + assert PresortedComparable.isSorted(fields); int currentOffset = 0; for (DexEncodedField field : fields) { int nextOffset = mapping.getOffsetFor(field.field); @@ -600,8 +600,8 @@ private void writeClassData(DexProgramClass clazz) { assert clazz.hasMethodsOrFields(); mixedSectionOffsets.setOffsetFor(clazz, dest.position()); - dest.putUleb128(clazz.staticFields().length); - dest.putUleb128(clazz.instanceFields().length); + dest.putUleb128(clazz.staticFields().size()); + dest.putUleb128(clazz.instanceFields().size()); dest.putUleb128(clazz.directMethods().size()); dest.putUleb128(clazz.virtualMethods().size()); writeEncodedFields(clazz.staticFields());
diff --git a/src/main/java/com/android/tools/r8/graph/DexAnnotationDirectory.java b/src/main/java/com/android/tools/r8/graph/DexAnnotationDirectory.java index 1ef2f68..f283d46 100644 --- a/src/main/java/com/android/tools/r8/graph/DexAnnotationDirectory.java +++ b/src/main/java/com/android/tools/r8/graph/DexAnnotationDirectory.java
@@ -8,7 +8,6 @@ import com.android.tools.r8.errors.Unreachable; import com.android.tools.r8.utils.OrderedMergingIterator; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.function.Function; @@ -38,11 +37,10 @@ parameterAnnotations.add(method); } } - assert isSorted(Arrays.asList(clazz.staticFields())); - assert isSorted(Arrays.asList(clazz.instanceFields())); + assert isSorted(clazz.staticFields()); + assert isSorted(clazz.instanceFields()); OrderedMergingIterator<DexEncodedField, DexField> fields = - new OrderedMergingIterator<>( - Arrays.asList(clazz.staticFields()), Arrays.asList(clazz.instanceFields())); + new OrderedMergingIterator<>(clazz.staticFields(), clazz.instanceFields()); fieldAnnotations = new ArrayList<>(); while (fields.hasNext()) { DexEncodedField field = fields.next(); @@ -68,7 +66,6 @@ return fieldAnnotations; } - /** * DexAnnotationDirectory of a class can be canonicalized only if a clazz has annotations and * does not contains annotations for his fields, methods or parameters. Indeed, if a field, method
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java index 5e65aa3..5220d64 100644 --- a/src/main/java/com/android/tools/r8/graph/DexClass.java +++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -23,8 +23,11 @@ public abstract class DexClass extends DexDefinition { - public interface MethodSetter { + public interface FieldSetter { + void setField(int index, DexEncodedField field); + } + public interface MethodSetter { void setMethod(int index, DexEncodedMethod method); } @@ -171,8 +174,8 @@ assert verifyNoDuplicateMethods(); } - public void setDirectMethods(DexEncodedMethod[] values) { - directMethods = MoreObjects.firstNonNull(values, NO_METHODS); + public void setDirectMethods(DexEncodedMethod[] methods) { + directMethods = MoreObjects.firstNonNull(methods, NO_METHODS); assert verifyCorrectnessOfMethodHolders(directMethods()); assert verifyNoDuplicateMethods(); } @@ -221,14 +224,14 @@ assert verifyNoDuplicateMethods(); } - public void setVirtualMethods(DexEncodedMethod[] values) { - virtualMethods = MoreObjects.firstNonNull(values, NO_METHODS); + public void setVirtualMethods(DexEncodedMethod[] methods) { + virtualMethods = MoreObjects.firstNonNull(methods, NO_METHODS); assert verifyCorrectnessOfMethodHolders(virtualMethods()); assert verifyNoDuplicateMethods(); } private boolean verifyCorrectnessOfMethodHolder(DexEncodedMethod method) { - assert method.method.holder == type + assert method.method.getHolder() == type : "Expected method `" + method.method.toSourceString() + "` to have holder `" @@ -340,12 +343,53 @@ } } - public DexEncodedField[] staticFields() { - return staticFields; + public List<DexEncodedField> staticFields() { + assert staticFields != null; + if (InternalOptions.assertionsEnabled()) { + return Collections.unmodifiableList(Arrays.asList(staticFields)); + } + return Arrays.asList(staticFields); } - public void setStaticFields(DexEncodedField[] values) { - staticFields = MoreObjects.firstNonNull(values, NO_FIELDS); + public void appendStaticField(DexEncodedField field) { + DexEncodedField[] newFields = new DexEncodedField[staticFields.length + 1]; + System.arraycopy(staticFields, 0, newFields, 0, staticFields.length); + newFields[staticFields.length] = field; + staticFields = newFields; + assert verifyCorrectnessOfFieldHolder(field); + assert verifyNoDuplicateFields(); + } + + public void appendStaticFields(Collection<DexEncodedField> fields) { + DexEncodedField[] newFields = new DexEncodedField[staticFields.length + fields.size()]; + System.arraycopy(staticFields, 0, newFields, 0, staticFields.length); + int i = staticFields.length; + for (DexEncodedField field : fields) { + newFields[i] = field; + i++; + } + staticFields = newFields; + assert verifyCorrectnessOfFieldHolders(fields); + assert verifyNoDuplicateFields(); + } + + public void removeStaticField(int index) { + DexEncodedField[] newFields = new DexEncodedField[staticFields.length - 1]; + System.arraycopy(staticFields, 0, newFields, 0, index); + System.arraycopy(staticFields, index + 1, newFields, index, staticFields.length - index - 1); + staticFields = newFields; + } + + public void setStaticField(int index, DexEncodedField field) { + staticFields[index] = field; + assert verifyCorrectnessOfFieldHolder(field); + assert verifyNoDuplicateFields(); + } + + public void setStaticFields(DexEncodedField[] fields) { + staticFields = MoreObjects.firstNonNull(fields, NO_FIELDS); + assert verifyCorrectnessOfFieldHolders(staticFields()); + assert verifyNoDuplicateFields(); } public boolean definesStaticField(DexField field) { @@ -357,12 +401,80 @@ return false; } - public DexEncodedField[] instanceFields() { - return instanceFields; + public List<DexEncodedField> instanceFields() { + assert instanceFields != null; + if (InternalOptions.assertionsEnabled()) { + return Collections.unmodifiableList(Arrays.asList(instanceFields)); + } + return Arrays.asList(instanceFields); } - public void setInstanceFields(DexEncodedField[] values) { - instanceFields = MoreObjects.firstNonNull(values, NO_FIELDS); + public void appendInstanceField(DexEncodedField field) { + DexEncodedField[] newFields = new DexEncodedField[instanceFields.length + 1]; + System.arraycopy(instanceFields, 0, newFields, 0, instanceFields.length); + newFields[instanceFields.length] = field; + instanceFields = newFields; + assert verifyCorrectnessOfFieldHolder(field); + assert verifyNoDuplicateFields(); + } + + public void appendInstanceFields(Collection<DexEncodedField> fields) { + DexEncodedField[] newFields = new DexEncodedField[instanceFields.length + fields.size()]; + System.arraycopy(instanceFields, 0, newFields, 0, instanceFields.length); + int i = instanceFields.length; + for (DexEncodedField field : fields) { + newFields[i] = field; + i++; + } + instanceFields = newFields; + assert verifyCorrectnessOfFieldHolders(fields); + assert verifyNoDuplicateFields(); + } + + public void removeInstanceField(int index) { + DexEncodedField[] newFields = new DexEncodedField[instanceFields.length - 1]; + System.arraycopy(instanceFields, 0, newFields, 0, index); + System.arraycopy( + instanceFields, index + 1, newFields, index, instanceFields.length - index - 1); + instanceFields = newFields; + } + + public void setInstanceField(int index, DexEncodedField field) { + instanceFields[index] = field; + assert verifyCorrectnessOfFieldHolder(field); + assert verifyNoDuplicateFields(); + } + + public void setInstanceFields(DexEncodedField[] fields) { + instanceFields = MoreObjects.firstNonNull(fields, NO_FIELDS); + assert verifyCorrectnessOfFieldHolders(instanceFields()); + assert verifyNoDuplicateFields(); + } + + private boolean verifyCorrectnessOfFieldHolder(DexEncodedField field) { + assert field.field.getHolder() == type + : "Expected field `" + + field.field.toSourceString() + + "` to have holder `" + + type.toSourceString() + + "`"; + return true; + } + + private boolean verifyCorrectnessOfFieldHolders(Iterable<DexEncodedField> fields) { + for (DexEncodedField field : fields) { + assert verifyCorrectnessOfFieldHolder(field); + } + return true; + } + + private boolean verifyNoDuplicateFields() { + Set<DexField> unique = Sets.newIdentityHashSet(); + for (DexEncodedField field : fields()) { + boolean changed = unique.add(field.field); + assert changed : "Duplicate field `" + field.field.toSourceString() + "`"; + } + return true; } public DexEncodedField[] allFieldsSorted() { @@ -380,14 +492,14 @@ * Find static field in this class matching field */ public DexEncodedField lookupStaticField(DexField field) { - return lookupTarget(staticFields(), field); + return lookupTarget(staticFields, field); } /** * Find instance field in this class matching field. */ public DexEncodedField lookupInstanceField(DexField field) { - return lookupTarget(instanceFields(), field); + return lookupTarget(instanceFields, field); } /** @@ -598,7 +710,7 @@ } public boolean defaultValuesForStaticFieldsMayTriggerAllocation() { - return Arrays.stream(staticFields()) + return staticFields().stream() .anyMatch(field -> field.getStaticValue().mayTriggerAllocation()); } @@ -669,6 +781,8 @@ public boolean isValid() { assert !isInterface() || Arrays.stream(virtualMethods).noneMatch(method -> method.accessFlags.isFinal()); + assert verifyCorrectnessOfFieldHolders(fields()); + assert verifyNoDuplicateFields(); assert verifyCorrectnessOfMethodHolders(methods()); assert verifyNoDuplicateMethods(); return true;
diff --git a/src/main/java/com/android/tools/r8/graph/DexProgramClass.java b/src/main/java/com/android/tools/r8/graph/DexProgramClass.java index 57b08fb..4312bfd 100644 --- a/src/main/java/com/android/tools/r8/graph/DexProgramClass.java +++ b/src/main/java/com/android/tools/r8/graph/DexProgramClass.java
@@ -225,7 +225,7 @@ } public boolean hasMethodsOrFields() { - int numberOfFields = staticFields().length + instanceFields().length; + int numberOfFields = staticFields().size() + instanceFields().size(); int numberOfMethods = directMethods().size() + virtualMethods().size(); return numberOfFields + numberOfMethods > 0; }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/SwitchMapCollector.java b/src/main/java/com/android/tools/r8/ir/optimize/SwitchMapCollector.java index a620ea9..44bba62 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/SwitchMapCollector.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/SwitchMapCollector.java
@@ -19,7 +19,6 @@ import com.android.tools.r8.utils.InternalOptions; import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap; import it.unimi.dsi.fastutil.ints.Int2ReferenceMap; -import java.util.Arrays; import java.util.IdentityHashMap; import java.util.List; import java.util.Map; @@ -93,7 +92,7 @@ if (!clazz.accessFlags.isSynthetic() && !clazz.hasClassInitializer()) { return; } - List<DexEncodedField> switchMapFields = Arrays.stream(clazz.staticFields()) + List<DexEncodedField> switchMapFields = clazz.staticFields().stream() .filter(this::maybeIsSwitchMap).collect(Collectors.toList()); if (!switchMapFields.isEmpty()) { IRCode initializer =
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java index 37441ac..0b98926 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -209,7 +209,7 @@ // of class inlining // - if (eligibleClassDefinition.instanceFields().length > 0) { + if (eligibleClassDefinition.instanceFields().size() > 0) { return false; } if (eligibleClassDefinition.type.hasSubtypes()) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/lambda/CaptureSignature.java b/src/main/java/com/android/tools/r8/ir/optimize/lambda/CaptureSignature.java index ede37b1..5a39e71 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/lambda/CaptureSignature.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/lambda/CaptureSignature.java
@@ -16,6 +16,7 @@ import java.util.Arrays; import java.util.BitSet; import java.util.Comparator; +import java.util.List; import java.util.function.IntFunction; // While mapping fields representing lambda captures we rearrange fields to make sure @@ -106,8 +107,8 @@ } // Compute capture signature based on lambda class capture fields. - public static String getCaptureSignature(DexEncodedField[] fields) { - return getCaptureSignature(fields.length, i -> fields[i].field.type); + public static String getCaptureSignature(List<DexEncodedField> fields) { + return getCaptureSignature(fields.size(), i -> fields.get(i).field.type); } // Compute capture signature based on type list. @@ -118,7 +119,7 @@ // Having a list of fields of lambda captured values, maps one of them into // an index of the appropriate field in normalized capture signature. public static int mapFieldIntoCaptureIndex( - String capture, DexEncodedField[] lambdaFields, DexField fieldToMap) { + String capture, List<DexEncodedField> lambdaFields, DexField fieldToMap) { char fieldKind = fieldToMap.type.toShorty(); int numberOfSameCaptureKind = 0; int result = -1;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaGroup.java b/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaGroup.java index f3fecde..375e1cc 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaGroup.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/lambda/LambdaGroup.java
@@ -109,16 +109,16 @@ return lambdas.get(lambda).id; } - protected final DexEncodedField[] lambdaCaptureFields(DexType lambda) { + protected final List<DexEncodedField> lambdaCaptureFields(DexType lambda) { assert lambdas.containsKey(lambda); return lambdas.get(lambda).clazz.instanceFields(); } protected final DexEncodedField lambdaSingletonField(DexType lambda) { assert lambdas.containsKey(lambda); - DexEncodedField[] fields = lambdas.get(lambda).clazz.staticFields(); - assert fields.length < 2; - return fields.length == 0 ? null : fields[0]; + List<DexEncodedField> fields = lambdas.get(lambda).clazz.staticFields(); + assert fields.size() < 2; + return fields.size() == 0 ? null : fields.get(0); } // Contains less than 2 elements?
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/JStyleLambdaGroup.java b/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/JStyleLambdaGroup.java index e8fcd40..21a3083 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/JStyleLambdaGroup.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/JStyleLambdaGroup.java
@@ -24,6 +24,7 @@ import com.android.tools.r8.kotlin.Kotlin; import com.android.tools.r8.utils.ThrowingConsumer; import com.google.common.collect.Lists; +import java.util.List; import java.util.function.IntFunction; // Represents a j-style lambda group created to combine several lambda classes @@ -152,8 +153,8 @@ } @Override - int getInstanceInitializerSize(DexEncodedField[] captures) { - return captures.length + 2; + int getInstanceInitializerSize(List<DexEncodedField> captures) { + return captures.size() + 2; } @Override
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KStyleLambdaGroup.java b/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KStyleLambdaGroup.java index f677b46..292e120 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KStyleLambdaGroup.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KStyleLambdaGroup.java
@@ -27,6 +27,7 @@ import com.android.tools.r8.kotlin.Kotlin; import com.android.tools.r8.utils.ThrowingConsumer; import com.google.common.collect.Lists; +import java.util.List; import java.util.function.IntFunction; // Represents a k-style lambda group created to combine several lambda classes @@ -158,8 +159,8 @@ } @Override - int getInstanceInitializerSize(DexEncodedField[] captures) { - return captures.length + 3; + int getInstanceInitializerSize(List<DexEncodedField> captures) { + return captures.size() + 3; } @Override
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaClassValidator.java b/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaClassValidator.java index 2385f35..d49a2bd 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaClassValidator.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaClassValidator.java
@@ -29,6 +29,7 @@ import com.android.tools.r8.ir.optimize.lambda.LambdaGroup.LambdaStructureError; import com.android.tools.r8.kotlin.Kotlin; import com.android.tools.r8.utils.ThrowingConsumer; +import java.util.List; // Encapsulates the logic of deep-checking of the lambda class assumptions. // @@ -118,14 +119,14 @@ } } - abstract int getInstanceInitializerSize(DexEncodedField[] captures); + abstract int getInstanceInitializerSize(List<DexEncodedField> captures); abstract int validateInstanceInitializerEpilogue( com.android.tools.r8.code.Instruction[] instructions, int index) throws LambdaStructureError; private void validateInstanceInitializer(DexClass lambda, Code code) throws LambdaStructureError { - DexEncodedField[] captures = lambda.instanceFields(); + List<DexEncodedField> captures = lambda.instanceFields(); com.android.tools.r8.code.Instruction[] instructions = code.asDexCode().instructions; int index = 0; @@ -142,7 +143,7 @@ assert index == instructions.length; } - private int validateInstanceInitializerParameterMapping(DexEncodedField[] captures, + private int validateInstanceInitializerParameterMapping(List<DexEncodedField> captures, Instruction[] instructions, int index) throws LambdaStructureError { int wideFieldsSeen = 0; for (DexEncodedField field : captures) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaGroupIdFactory.java b/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaGroupIdFactory.java index 234bcc6..63e0684 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaGroupIdFactory.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaGroupIdFactory.java
@@ -111,30 +111,30 @@ } void validateStaticFields(Kotlin kotlin, DexClass lambda) throws LambdaStructureError { - DexEncodedField[] staticFields = lambda.staticFields(); - if (staticFields.length == 1) { - DexEncodedField field = staticFields[0]; + List<DexEncodedField> staticFields = lambda.staticFields(); + if (staticFields.size() == 1) { + DexEncodedField field = staticFields.get(0); if (field.field.name != kotlin.functional.kotlinStyleLambdaInstanceName || field.field.type != lambda.type || !field.accessFlags.isPublic() || !field.accessFlags.isFinal() || !field.accessFlags.isStatic()) { throw new LambdaStructureError("unexpected static field " + field.toSourceString()); } // No state if the lambda is a singleton. - if (lambda.instanceFields().length > 0) { + if (lambda.instanceFields().size() > 0) { throw new LambdaStructureError("has instance fields along with INSTANCE"); } checkAccessFlags("static field access flags", field.accessFlags, SINGLETON_FIELD_FLAGS); checkFieldAnnotations(field); - } else if (staticFields.length > 1) { + } else if (staticFields.size() > 1) { throw new LambdaStructureError( - "only one static field max expected, found " + staticFields.length); + "only one static field max expected, found " + staticFields.size()); } } String validateInstanceFields(DexClass lambda, boolean accessRelaxed) throws LambdaStructureError { - DexEncodedField[] instanceFields = lambda.instanceFields(); + List<DexEncodedField> instanceFields = lambda.instanceFields(); for (DexEncodedField field : instanceFields) { checkAccessFlags("capture field access flags", field.accessFlags, accessRelaxed ? CAPTURE_FIELD_FLAGS_RELAXED : CAPTURE_FIELD_FLAGS); @@ -147,7 +147,7 @@ for (DexEncodedMethod method : lambda.directMethods()) { if (method.isClassInitializer()) { // We expect to see class initializer only if there is a singleton field. - if (lambda.staticFields().length != 1) { + if (lambda.staticFields().size() != 1) { throw new LambdaStructureError("has static initializer, but no singleton field"); } checkAccessFlags("unexpected static initializer access flags", @@ -162,15 +162,15 @@ // Lambda class is expected to have one constructor // with parameters matching capture signature. DexType[] parameters = method.method.proto.parameters.values; - DexEncodedField[] instanceFields = lambda.instanceFields(); - if (parameters.length != instanceFields.length) { + List<DexEncodedField> instanceFields = lambda.instanceFields(); + if (parameters.length != instanceFields.size()) { throw new LambdaStructureError("constructor parameters don't match captured values."); } for (int i = 0; i < parameters.length; i++) { // Kotlin compiler sometimes reshuffles the parameters so that their order // in the constructor don't match order of capture fields. We could add // support for it, but it happens quite rarely so don't bother for now. - if (parameters[i] != instanceFields[i].field.type) { + if (parameters[i] != instanceFields.get(i).field.type) { throw new LambdaStructureError( "constructor parameters don't match captured values.", false); }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java index 443a2f4..bf205b3 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java
@@ -141,18 +141,18 @@ } // High-level limitations on what classes we consider eligible. - if (cls.isInterface() || // Must not be an interface or an abstract class. - cls.accessFlags.isAbstract() || + if (cls.isInterface() // Must not be an interface or an abstract class. + || cls.accessFlags.isAbstract() // Don't support candidates with instance fields - cls.instanceFields().length > 0 || + || cls.instanceFields().size() > 0 // Only support classes directly extending java.lang.Object - cls.superType != factory.objectType || + || cls.superType != factory.objectType // Instead of requiring the class being final, // just ensure it does not have subtypes - cls.type.hasSubtypes() || + || cls.type.hasSubtypes() // Staticizing classes implementing interfaces is more // difficult, so don't support it until we really need it. - !cls.interfaces.isEmpty()) { + || !cls.interfaces.isEmpty()) { notEligible.add(cls.type); } });
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java index ff4a12e..4f4d4ef 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
@@ -46,7 +46,6 @@ import java.util.concurrent.Future; import java.util.function.BiConsumer; import java.util.stream.Collectors; -import java.util.stream.Stream; final class StaticizingProcessor { @@ -119,7 +118,7 @@ // CHECK: instance initializer used to create an instance is trivial. // NOTE: Along with requirement that candidate does not have instance // fields this should guarantee that the constructor is empty. - assert candidateClass.instanceFields().length == 0; + assert candidateClass.instanceFields().size() == 0; assert constructorUsed.isProcessed(); TrivialInitializer trivialInitializer = constructorUsed.getOptimizationInfo().getTrivialInitializerInfo(); @@ -225,7 +224,7 @@ // extend java.lang.Object guarantees that the constructor is actually // empty and does not need to be inlined. assert candidateInfo.candidate.superType == factory().objectType; - assert candidateInfo.candidate.instanceFields().length == 0; + assert candidateInfo.candidate.instanceFields().size() == 0; Value singletonValue = instruction.outValue(); assert singletonValue != null; @@ -313,7 +312,7 @@ // From staticizer's viewpoint, `sp` is trivial in the sense that it is composed of values that // refer to the same singleton field. If so, we can safely relax the assertion; remove uses of // field reads; remove quasi-trivial phis; and then remove original field reads. - private boolean testAndcollectPhisComposedOfSameFieldRead( + private boolean testAndCollectPhisComposedOfSameFieldRead( Set<Phi> phisToCheck, DexField field, Set<Phi> trivialPhis) { for (Phi phi : phisToCheck) { Set<Phi> chainedPhis = Sets.newIdentityHashSet(); @@ -330,7 +329,7 @@ } } if (!chainedPhis.isEmpty()) { - if (!testAndcollectPhisComposedOfSameFieldRead(chainedPhis, field, trivialPhis)) { + if (!testAndCollectPhisComposedOfSameFieldRead(chainedPhis, field, trivialPhis)) { return false; } } @@ -347,7 +346,7 @@ // However, it may be not true if re-processing introduces phis after optimizing common suffix. Set<Phi> trivialPhis = Sets.newIdentityHashSet(); boolean hasTrivialPhis = - testAndcollectPhisComposedOfSameFieldRead(dest.uniquePhiUsers(), field, trivialPhis); + testAndCollectPhisComposedOfSameFieldRead(dest.uniquePhiUsers(), field, trivialPhis); assert dest.numberOfPhiUsers() == 0 || hasTrivialPhis; Set<Instruction> users = new HashSet<>(dest.uniqueUsers()); // If that is the case, method calls we want to fix up include users of those phis. @@ -510,12 +509,13 @@ private boolean classMembersConflict(DexClass a, DexClass b) { assert Streams.stream(a.methods()).allMatch(DexEncodedMethod::isStatic); - assert a.instanceFields().length == 0; - return Stream.of(a.staticFields()).anyMatch(fld -> b.lookupField(fld.field) != null) || - Streams.stream(a.methods()).anyMatch(method -> b.lookupMethod(method.method) != null); + assert a.instanceFields().size() == 0; + return a.staticFields().stream().anyMatch(fld -> b.lookupField(fld.field) != null) + || Streams.stream(a.methods()).anyMatch(method -> b.lookupMethod(method.method) != null); } - private void moveMembersIntoHost(Set<DexEncodedMethod> staticizedMethods, + private void moveMembersIntoHost( + Set<DexEncodedMethod> staticizedMethods, DexProgramClass candidateClass, DexType hostType, DexClass hostClass, BiMap<DexMethod, DexMethod> methodMapping, @@ -523,26 +523,36 @@ candidateToHostMapping.put(candidateClass.type, hostType); // Process static fields. - // Append fields first. - if (candidateClass.staticFields().length > 0) { - DexEncodedField[] oldFields = hostClass.staticFields(); - DexEncodedField[] extraFields = candidateClass.staticFields(); - DexEncodedField[] newFields = new DexEncodedField[oldFields.length + extraFields.length]; - System.arraycopy(oldFields, 0, newFields, 0, oldFields.length); - System.arraycopy(extraFields, 0, newFields, oldFields.length, extraFields.length); - hostClass.setStaticFields(newFields); - } - - // Fixup field types. - DexEncodedField[] staticFields = hostClass.staticFields(); - for (int i = 0; i < staticFields.length; i++) { - DexEncodedField field = staticFields[i]; + int numOfHostStaticFields = hostClass.staticFields().size(); + DexEncodedField[] newFields = + candidateClass.staticFields().size() > 0 + ? new DexEncodedField[numOfHostStaticFields + candidateClass.staticFields().size()] + : new DexEncodedField[numOfHostStaticFields]; + List<DexEncodedField> oldFields = hostClass.staticFields(); + for (int i = 0; i < oldFields.size(); i++) { + DexEncodedField field = oldFields.get(i); DexField newField = mapCandidateField(field.field, candidateClass.type, hostType); if (newField != field.field) { - staticFields[i] = field.toTypeSubstitutedField(newField); + newFields[i] = field.toTypeSubstitutedField(newField); fieldMapping.put(field.field, newField); + } else { + newFields[i] = field; } } + if (candidateClass.staticFields().size() > 0) { + List<DexEncodedField> extraFields = candidateClass.staticFields(); + for (int i = 0; i < extraFields.size(); i++) { + DexEncodedField field = extraFields.get(i); + DexField newField = mapCandidateField(field.field, candidateClass.type, hostType); + if (newField != field.field) { + newFields[numOfHostStaticFields + i] = field.toTypeSubstitutedField(newField); + fieldMapping.put(field.field, newField); + } else { + newFields[numOfHostStaticFields + i] = field; + } + } + } + hostClass.setStaticFields(newFields); // Process static methods. List<DexEncodedMethod> extraMethods = candidateClass.directMethods();
diff --git a/src/main/java/com/android/tools/r8/naming/MinifiedNameMapPrinter.java b/src/main/java/com/android/tools/r8/naming/MinifiedNameMapPrinter.java index e3d09b6..0fdeebc 100644 --- a/src/main/java/com/android/tools/r8/naming/MinifiedNameMapPrinter.java +++ b/src/main/java/com/android/tools/r8/naming/MinifiedNameMapPrinter.java
@@ -72,7 +72,7 @@ } } - private void writeFields(DexEncodedField[] fields, StringBuilder out) { + private void writeFields(List<DexEncodedField> fields, StringBuilder out) { for (DexEncodedField encodedField : fields) { DexField field = encodedField.field; DexString renamed = namingLens.lookupName(field);
diff --git a/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java b/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java index a447e37..78dad02 100644 --- a/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java +++ b/src/main/java/com/android/tools/r8/naming/ProguardMapApplier.java
@@ -7,6 +7,7 @@ import com.android.tools.r8.graph.DexAnnotation; import com.android.tools.r8.graph.DexAnnotationSet; import com.android.tools.r8.graph.DexClass; +import com.android.tools.r8.graph.DexClass.FieldSetter; import com.android.tools.r8.graph.DexClass.MethodSetter; import com.android.tools.r8.graph.DexEncodedAnnotation; import com.android.tools.r8.graph.DexEncodedField; @@ -326,8 +327,8 @@ clazz.annotations = substituteTypesIn(clazz.annotations); fixupMethods(clazz.directMethods(), clazz::setDirectMethod); fixupMethods(clazz.virtualMethods(), clazz::setVirtualMethod); - clazz.setStaticFields(substituteTypesIn(clazz.staticFields())); - clazz.setInstanceFields(substituteTypesIn(clazz.instanceFields())); + fixupFields(clazz.staticFields(), clazz::setStaticField); + fixupFields(clazz.instanceFields(), clazz::setInstanceField); } private void fixupMethods(List<DexEncodedMethod> methods, MethodSetter setter) { @@ -352,12 +353,12 @@ } } - private DexEncodedField[] substituteTypesIn(DexEncodedField[] fields) { + private void fixupFields(List<DexEncodedField> fields, FieldSetter setter) { if (fields == null) { - return null; + return; } - for (int i = 0; i < fields.length; i++) { - DexEncodedField encodedField = fields[i]; + for (int i = 0; i < fields.size(); i++) { + DexEncodedField encodedField = fields.get(i); DexField appliedField = appliedLense.lookupField(encodedField.field); DexType newHolderType = substituteType(appliedField.clazz, null); DexType newFieldType = substituteType(appliedField.type, null); @@ -370,9 +371,8 @@ newField = appliedField; } // Explicitly fix members. - fields[i] = encodedField.toTypeSubstitutedField(newField); + setter.setField(i, encodedField.toTypeSubstitutedField(newField)); } - return fields; } private DexProto substituteTypesIn(DexProto proto, DexEncodedMethod context) {
diff --git a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java index 42b7e21..9d68c91 100644 --- a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java +++ b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
@@ -31,7 +31,6 @@ import com.google.common.collect.Multiset.Entry; import com.google.common.collect.Streams; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -259,14 +258,14 @@ if (appView.appInfo().neverMerge.contains(clazz.type)) { return MergeGroup.DONT_MERGE; } - if (clazz.staticFields().length + clazz.directMethods().size() + clazz.virtualMethods().size() + if (clazz.staticFields().size() + clazz.directMethods().size() + clazz.virtualMethods().size() == 0) { return MergeGroup.DONT_MERGE; } - if (clazz.instanceFields().length > 0) { + if (clazz.instanceFields().size() > 0) { return MergeGroup.DONT_MERGE; } - if (Arrays.stream(clazz.staticFields()) + if (clazz.staticFields().stream() .anyMatch(field -> appView.appInfo().isPinned(field.field))) { return MergeGroup.DONT_MERGE; } @@ -284,7 +283,7 @@ // TODO(christofferqa): Remove the invariant that the graph lense should not // modify any methods from the sets alwaysInline and noSideEffects. || appView.appInfo().alwaysInline.contains(method.method) - || appView.appInfo().noSideEffects.keySet().contains(method))) { + || appView.appInfo().noSideEffects.keySet().contains(method.method))) { return MergeGroup.DONT_MERGE; } if (clazz.classInitializationMayHaveSideEffects(appView.appInfo())) { @@ -311,6 +310,7 @@ // Disallow interfaces from being representatives, since interface methods require desugaring. return !clazz.isInterface(); } + private boolean merge(DexProgramClass clazz, MergeGroup group) { assert satisfiesMergeCriteria(clazz) == group; assert group != MergeGroup.DONT_MERGE; @@ -451,15 +451,15 @@ .allMatch(method -> method.accessFlags.isPrivate() || method.accessFlags.isPublic())) { return false; } - if (!Arrays.stream(clazz.staticFields()) - .allMatch(method -> method.accessFlags.isPrivate() || method.accessFlags.isPublic())) { + if (!clazz.staticFields().stream() + .allMatch(field -> field.accessFlags.isPrivate() || field.accessFlags.isPublic())) { return false; } // Note that a class is only considered a candidate if it has no instance fields and all of its // virtual methods are private. Therefore, we don't need to consider check if there are any // package-private or protected instance fields or virtual methods here. - assert Arrays.stream(clazz.instanceFields()).count() == 0; + assert clazz.instanceFields().size() == 0; assert clazz.virtualMethods().stream().allMatch(method -> method.accessFlags.isPrivate()); // Check that no methods access package-private or protected members. @@ -485,8 +485,8 @@ } assert targetClass.accessFlags.isAtLeastAsVisibleAs(sourceClass.accessFlags); - assert sourceClass.instanceFields().length == 0; - assert targetClass.instanceFields().length == 0; + assert sourceClass.instanceFields().size() == 0; + assert targetClass.instanceFields().size() == 0; numberOfMergedClasses++; @@ -534,27 +534,31 @@ } private DexEncodedField[] mergeFields( - DexEncodedField[] sourceFields, DexEncodedField[] targetFields, DexProgramClass targetClass) { - DexEncodedField[] result = new DexEncodedField[sourceFields.length + targetFields.length]; + List<DexEncodedField> sourceFields, + List<DexEncodedField> targetFields, + DexProgramClass targetClass) { + DexEncodedField[] result = new DexEncodedField[sourceFields.size() + targetFields.size()]; // Move all target fields to result. - System.arraycopy(targetFields, 0, result, 0, targetFields.length); + int index = 0; + for (DexEncodedField targetField : targetFields) { + result[index++] = targetField; + } // Move source fields to result one by one, renaming them if needed. FieldSignatureEquivalence equivalence = FieldSignatureEquivalence.get(); Set<Wrapper<DexField>> existingFields = - Arrays.stream(targetFields) + targetFields.stream() .map(targetField -> equivalence.wrap(targetField.field)) .collect(Collectors.toSet()); Predicate<DexField> availableFieldSignatures = field -> !existingFields.contains(equivalence.wrap(field)); - int i = targetFields.length; for (DexEncodedField sourceField : sourceFields) { DexEncodedField sourceFieldAfterMove = renameFieldIfNeeded(sourceField, targetClass, availableFieldSignatures); - result[i++] = sourceFieldAfterMove; + result[index++] = sourceFieldAfterMove; DexField originalField = fieldMapping.inverse().getOrDefault(sourceField.field, sourceField.field); @@ -563,6 +567,7 @@ existingFields.add(equivalence.wrap(sourceFieldAfterMove.field)); } + assert index == result.length; return result; }
diff --git a/src/main/java/com/android/tools/r8/shaking/TreePruner.java b/src/main/java/com/android/tools/r8/shaking/TreePruner.java index 8017878..833cac3 100644 --- a/src/main/java/com/android/tools/r8/shaking/TreePruner.java +++ b/src/main/java/com/android/tools/r8/shaking/TreePruner.java
@@ -21,7 +21,6 @@ import com.android.tools.r8.utils.StringDiagnostic; import com.google.common.collect.Sets; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -109,8 +108,14 @@ if (reachableVirtualMethods != null) { clazz.setVirtualMethods(reachableVirtualMethods); } - clazz.setInstanceFields(reachableFields(clazz.instanceFields())); - clazz.setStaticFields(reachableFields(clazz.staticFields())); + DexEncodedField[] reachableInstanceFields = reachableFields(clazz.instanceFields()); + if (reachableInstanceFields != null) { + clazz.setInstanceFields(reachableInstanceFields); + } + DexEncodedField[] reachableStaticFields = reachableFields(clazz.staticFields()); + if (reachableStaticFields != null) { + clazz.setStaticFields(reachableStaticFields); + } clazz.removeInnerClasses(this::isAttributeReferencingPrunedType); clazz.removeEnclosingMethod(this::isAttributeReferencingPrunedItem); usagePrinter.visited(); @@ -221,36 +226,37 @@ usagePrinter.printUnusedMethod(method); } } - return reachableMethods.toArray(new DexEncodedMethod[reachableMethods.size()]); + return reachableMethods.isEmpty() + ? DexEncodedMethod.EMPTY_ARRAY + : reachableMethods.toArray(DexEncodedMethod.EMPTY_ARRAY); } - private DexEncodedField[] reachableFields(DexEncodedField[] fields) { + private DexEncodedField[] reachableFields(List<DexEncodedField> fields) { Predicate<DexField> isReachableOrReferencedField = field -> appInfo.liveFields.contains(field) || appInfo.isFieldRead(field) || appInfo.isFieldWritten(field); - int firstUnreachable = - firstUnreachableIndex(Arrays.asList(fields), isReachableOrReferencedField); + int firstUnreachable = firstUnreachableIndex(fields, isReachableOrReferencedField); // Return the original array if all fields are used. if (firstUnreachable == -1) { - return fields; + return null; } if (Log.ENABLED) { - Log.debug(getClass(), "Removing field: " + fields[firstUnreachable]); + Log.debug(getClass(), "Removing field %s.", fields.get(firstUnreachable)); } - usagePrinter.printUnusedField(fields[firstUnreachable]); - ArrayList<DexEncodedField> reachableOrReferencedFields = new ArrayList<>(fields.length); + usagePrinter.printUnusedField(fields.get(firstUnreachable)); + ArrayList<DexEncodedField> reachableOrReferencedFields = new ArrayList<>(fields.size()); for (int i = 0; i < firstUnreachable; i++) { - reachableOrReferencedFields.add(fields[i]); + reachableOrReferencedFields.add(fields.get(i)); } - for (int i = firstUnreachable + 1; i < fields.length; i++) { - DexEncodedField field = fields[i]; + for (int i = firstUnreachable + 1; i < fields.size(); i++) { + DexEncodedField field = fields.get(i); if (isReachableOrReferencedField.test(field.field)) { reachableOrReferencedFields.add(field); } else { if (Log.ENABLED) { - Log.debug(getClass(), "Removing field: " + field); + Log.debug(getClass(), "Removing field %s.", field.field); } usagePrinter.printUnusedField(field); }
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java index acfc42f..f6941d3 100644 --- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java +++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -14,6 +14,7 @@ import com.android.tools.r8.graph.DexAnnotationSet; import com.android.tools.r8.graph.DexApplication; import com.android.tools.r8.graph.DexClass; +import com.android.tools.r8.graph.DexClass.FieldSetter; import com.android.tools.r8.graph.DexClass.MethodSetter; import com.android.tools.r8.graph.DexEncodedField; import com.android.tools.r8.graph.DexEncodedMethod; @@ -1259,11 +1260,11 @@ } private DexEncodedField[] mergeFields( - DexEncodedField[] sourceFields, - DexEncodedField[] targetFields, + Collection<DexEncodedField> sourceFields, + Collection<DexEncodedField> targetFields, Predicate<DexField> availableFieldSignatures, Set<DexString> existingFieldNames) { - DexEncodedField[] result = new DexEncodedField[sourceFields.length + targetFields.length]; + DexEncodedField[] result = new DexEncodedField[sourceFields.size() + targetFields.size()]; // Add fields from source int i = 0; for (DexEncodedField field : sourceFields) { @@ -1274,21 +1275,10 @@ i++; } // Add fields from target. - System.arraycopy(targetFields, 0, result, i, targetFields.length); - return result; - } - - private DexEncodedMethod[] mergeMethods( - Collection<DexEncodedMethod> sourceMethods, List<DexEncodedMethod> targetMethods) { - DexEncodedMethod[] result = new DexEncodedMethod[sourceMethods.size() + targetMethods.size()]; - // Add methods from source. - int i = 0; - for (DexEncodedMethod method : sourceMethods) { - result[i] = method; + for (DexEncodedField field : targetFields) { + result[i] = field; i++; } - // Add methods from target. - System.arraycopy(targetMethods, 0, result, i, targetMethods.size()); return result; } @@ -1434,8 +1424,8 @@ for (DexProgramClass clazz : appInfo.classes()) { fixupMethods(clazz.directMethods(), clazz::setDirectMethod); fixupMethods(clazz.virtualMethods(), clazz::setVirtualMethod); - clazz.setStaticFields(substituteTypesIn(clazz.staticFields())); - clazz.setInstanceFields(substituteTypesIn(clazz.instanceFields())); + fixupFields(clazz.staticFields(), clazz::setStaticField); + fixupFields(clazz.instanceFields(), clazz::setInstanceField); } for (SynthesizedBridgeCode synthesizedBridge : synthesizedBridges) { synthesizedBridge.updateMethodSignatures(this::fixupMethod); @@ -1462,22 +1452,21 @@ } } - private DexEncodedField[] substituteTypesIn(DexEncodedField[] fields) { + private void fixupFields(List<DexEncodedField> fields, FieldSetter setter) { if (fields == null) { - return null; + return; } - for (int i = 0; i < fields.length; i++) { - DexEncodedField encodedField = fields[i]; + for (int i = 0; i < fields.size(); i++) { + DexEncodedField encodedField = fields.get(i); DexField field = encodedField.field; DexType newType = fixupType(field.type); DexType newHolder = fixupType(field.clazz); DexField newField = application.dexItemFactory.createField(newHolder, newType, field.name); if (newField != encodedField.field) { lense.move(encodedField.field, newField); - fields[i] = encodedField.toTypeSubstitutedField(newField); + setter.setField(i, encodedField.toTypeSubstitutedField(newField)); } } - return fields; } private DexMethod fixupMethod(DexMethod method) {
diff --git a/src/test/java/com/android/tools/r8/invalid/DuplicateDefinitionsTest.java b/src/test/java/com/android/tools/r8/invalid/DuplicateDefinitionsTest.java index 9c9e785..e92e0f3 100644 --- a/src/test/java/com/android/tools/r8/invalid/DuplicateDefinitionsTest.java +++ b/src/test/java/com/android/tools/r8/invalid/DuplicateDefinitionsTest.java
@@ -94,7 +94,7 @@ assertThat(clazz, isPresent()); // Redundant fields have been removed. - assertEquals(1, clazz.getDexClass().instanceFields().length); - assertEquals(1, clazz.getDexClass().staticFields().length); + assertEquals(1, clazz.getDexClass().instanceFields().size()); + assertEquals(1, clazz.getDexClass().staticFields().size()); } }
diff --git a/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java b/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java index 7e9269d..82f8688 100644 --- a/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java +++ b/src/test/java/com/android/tools/r8/kotlin/KotlinLambdaMergingTest.java
@@ -242,7 +242,7 @@ private static int getLambdaSingletons(DexClass clazz) { assertEquals(1, clazz.interfaces.size()); - return clazz.staticFields().length; + return clazz.staticFields().size(); } private static boolean isLambdaOrGroup(DexClass clazz) {
diff --git a/src/test/java/com/android/tools/r8/naming/IdentifierMinifierTest.java b/src/test/java/com/android/tools/r8/naming/IdentifierMinifierTest.java index d06809e..e247098 100644 --- a/src/test/java/com/android/tools/r8/naming/IdentifierMinifierTest.java +++ b/src/test/java/com/android/tools/r8/naming/IdentifierMinifierTest.java
@@ -310,8 +310,8 @@ } private static int countRenamedClassIdentifier( - CodeInspector inspector, DexEncodedField[] fields) { - return Arrays.stream(fields) + CodeInspector inspector, List<DexEncodedField> fields) { + return fields.stream() .filter(encodedField -> encodedField.getStaticValue() instanceof DexValueString) .reduce(0, (cnt, encodedField) -> { String cnstString =
diff --git a/src/test/java/com/android/tools/r8/shaking/PrintUsageTest.java b/src/test/java/com/android/tools/r8/shaking/PrintUsageTest.java index e5f3517..6196bb7 100644 --- a/src/test/java/com/android/tools/r8/shaking/PrintUsageTest.java +++ b/src/test/java/com/android/tools/r8/shaking/PrintUsageTest.java
@@ -6,8 +6,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import com.android.tools.r8.OutputMode; -import com.android.tools.r8.R8Command; import com.android.tools.r8.TestBase; import com.android.tools.r8.ToolHelper; import com.android.tools.r8.shaking.PrintUsageTest.PrintUsageInspector.ClassSubject; @@ -21,20 +19,16 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.concurrent.ExecutionException; import java.util.function.Consumer; import java.util.stream.Stream; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; @@ -50,9 +44,6 @@ private final List<String> keepRulesFiles; private final Consumer<PrintUsageInspector> inspection; - @Rule - public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest(); - public PrintUsageTest( Backend backend, String test, @@ -68,33 +59,19 @@ @Before public void runR8andGetPrintUsage() throws Exception { Path out = temp.getRoot().toPath(); - R8Command.Builder builder = - ToolHelper.addProguardConfigurationConsumer( - R8Command.builder(), - pgConfig -> { - pgConfig.setPrintUsage(true); - pgConfig.setPrintUsageFile(out.resolve(test + PRINT_USAGE_FILE_SUFFIX)); - }) - .addProgramFiles(Paths.get(programFile)) - .addProguardConfigurationFiles(ListUtils.map(keepRulesFiles, Paths::get)); - - if (backend == Backend.DEX) { - builder - .setOutput(out, OutputMode.DexIndexed) - .addLibraryFiles(ToolHelper.getDefaultAndroidJar()); - } else { - builder.setOutput(out, OutputMode.ClassFile).addLibraryFiles(ToolHelper.getJava8RuntimeJar()); - } - ToolHelper.runR8( - builder.build(), - options -> { - // Disable inlining to make this test not depend on inlining decisions. - options.enableInlining = false; - }); + testForR8(backend) + .addProgramFiles(Paths.get(programFile)) + .addKeepRuleFiles(ListUtils.map(keepRulesFiles, Paths::get)) + .addKeepRules( + "-printusage " + out.resolve(test + PRINT_USAGE_FILE_SUFFIX) + ) + // Disable inlining to make this test not depend on inlining decisions. + .addOptionsModification(o -> o.enableInlining = false) + .compile(); } @Test - public void printUsageTest() throws IOException, ExecutionException { + public void printUsageTest() throws IOException { Path out = temp.getRoot().toPath(); Path printUsageFile = out.resolve(test + PRINT_USAGE_FILE_SUFFIX); if (inspection != null) { @@ -118,20 +95,20 @@ List<Object[]> testCases = new ArrayList<>(); for (Backend backend : Backend.values()) { - Set<String> usedInspections = new HashSet<>(); - for (String test : tests) { - File[] keepFiles = new File(ToolHelper.EXAMPLES_DIR + test) - .listFiles(file -> file.isFile() && file.getName().endsWith(".txt")); - for (File keepFile : keepFiles) { - String keepName = keepFile.getName(); - Consumer<PrintUsageInspector> inspection = - getTestOptionalParameter(inspections, usedInspections, test, keepName); - if (inspection != null) { + Set<String> usedInspections = new HashSet<>(); + for (String test : tests) { + File[] keepFiles = new File(ToolHelper.EXAMPLES_DIR + test) + .listFiles(file -> file.isFile() && file.getName().endsWith(".txt")); + for (File keepFile : keepFiles) { + String keepName = keepFile.getName(); + Consumer<PrintUsageInspector> inspection = + getTestOptionalParameter(inspections, usedInspections, test, keepName); + if (inspection != null) { testCases.add( new Object[] {backend, test, ImmutableList.of(keepFile.getPath()), inspection}); + } } } - } assert usedInspections.size() == inspections.size(); } return testCases; @@ -152,6 +129,9 @@ } private static void inspectShaking1(PrintUsageInspector inspector) { + Optional<ClassSubject> shaking1 = inspector.clazz("shaking1.Shaking"); + assertTrue(shaking1.isPresent()); + assertTrue(shaking1.get().method("void", "<init>", ImmutableList.of())); assertTrue(inspector.clazz("shaking1.Unused").isPresent()); assertTrue(inspector.clazz("shaking1.Used").isPresent()); ClassSubject used = inspector.clazz("shaking1.Used").get(); @@ -165,10 +145,10 @@ assertTrue(staticFields.get().field("int", "unused")); Optional<ClassSubject> subClass1 = inspector.clazz("shaking2.SubClass1"); assertTrue(subClass1.isPresent()); - assertTrue(subClass1.get().method("void", "unusedVirtualMethod", Collections.emptyList())); + assertTrue(subClass1.get().method("void", "unusedVirtualMethod", ImmutableList.of())); Optional<ClassSubject> superClass = inspector.clazz("shaking2.SuperClass"); assertTrue(superClass.isPresent()); - assertTrue(superClass.get().method("void", "unusedStaticMethod", Collections.emptyList())); + assertTrue(superClass.get().method("void", "unusedStaticMethod", ImmutableList.of())); } private static void inspectShaking4(PrintUsageInspector inspector) { @@ -188,15 +168,15 @@ assertFalse(superClass.isPresent()); Optional<ClassSubject> subClass = inspector.clazz("shaking9.Subclass"); assertTrue(subClass.isPresent()); - assertTrue(subClass.get().method("void", "aMethod", Collections.emptyList())); - assertFalse(subClass.get().method("void", "<init>", Collections.emptyList())); + assertTrue(subClass.get().method("void", "aMethod", ImmutableList.of())); + assertFalse(subClass.get().method("void", "<init>", ImmutableList.of())); } private static void inspectShaking12(PrintUsageInspector inspector) { assertFalse(inspector.clazz("shaking12.PeopleClass").isPresent()); Optional<ClassSubject> animal = inspector.clazz("shaking12.AnimalClass"); assertTrue(animal.isPresent()); - assertTrue(animal.get().method("java.lang.String", "getName", Collections.emptyList())); + assertTrue(animal.get().method("java.lang.String", "getName", ImmutableList.of())); } static class PrintUsageInspector {
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java index ebaf5af..058a19d 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FoundClassSubject.java
@@ -109,12 +109,12 @@ @Override public void forAllFields(Consumer<FoundFieldSubject> inspection) { CodeInspector.forAll( - Arrays.asList(dexClass.staticFields()), + dexClass.staticFields(), (dexField, clazz) -> new FoundFieldSubject(codeInspector, dexField, clazz), this, inspection); CodeInspector.forAll( - Arrays.asList(dexClass.instanceFields()), + dexClass.instanceFields(), (dexField, clazz) -> new FoundFieldSubject(codeInspector, dexField, clazz), this, inspection); @@ -175,7 +175,7 @@ return dexClass.accessFlags.isAnnotation(); } - private DexEncodedField findField(DexEncodedField[] fields, DexField dexField) { + private DexEncodedField findField(List<DexEncodedField> fields, DexField dexField) { for (DexEncodedField field : fields) { if (field.field.equals(dexField)) { return field;