Revert "Add implementation of hashCode and equals to desugared library wrappers" This reverts commit 0ecfae9f587838a137ef74208366d6b9eb2ed1cb. Reason for revert: Doesn't compile!!! Change-Id: Iaa01c810ac254dfce92612013e0f3601d58baa03
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/apiconversion/DesugaredLibraryConversionCfProvider.java b/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/apiconversion/DesugaredLibraryConversionCfProvider.java index 29ecfd2..ea0f8c0 100644 --- a/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/apiconversion/DesugaredLibraryConversionCfProvider.java +++ b/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/apiconversion/DesugaredLibraryConversionCfProvider.java
@@ -41,8 +41,6 @@ import com.android.tools.r8.ir.desugar.desugaredlibrary.apiconversion.DesugaredLibraryWrapperSynthesizerEventConsumer.DesugaredLibraryClasspathWrapperSynthesizeEventConsumer; import com.android.tools.r8.ir.desugar.desugaredlibrary.apiconversion.DesugaredLibraryWrapperSynthesizerEventConsumer.DesugaredLibraryL8ProgramWrapperSynthesizerEventConsumer; import com.android.tools.r8.ir.synthetic.apiconverter.APIConversionCfCodeProvider; -import com.android.tools.r8.ir.synthetic.apiconverter.EqualsCfCodeProvider; -import com.android.tools.r8.ir.synthetic.apiconverter.HashCodeCfCodeProvider; import com.android.tools.r8.utils.OptionalBool; import java.util.ArrayList; import java.util.Collection; @@ -99,33 +97,6 @@ return wrapperSynthesizer.newSynthesizedMethod(methodToInstall, cfCode); } - public DexEncodedMethod generateWrapperHashCode(DexField wrapperField) { - return wrapperSynthesizer.newSynthesizedMethod( - appView - .dexItemFactory() - .createMethod( - wrapperField.getHolderType(), - appView.dexItemFactory().createProto(appView.dexItemFactory().intType), - appView.dexItemFactory().hashCodeMethodName), - new HashCodeCfCodeProvider(appView, wrapperField.getHolderType(), wrapperField) - .generateCfCode()); - } - - public DexEncodedMethod generateWrapperEquals(DexField wrapperField) { - return wrapperSynthesizer.newSynthesizedMethod( - appView - .dexItemFactory() - .createMethod( - wrapperField.getHolderType(), - appView - .dexItemFactory() - .createProto( - appView.dexItemFactory().booleanType, appView.dexItemFactory().objectType), - appView.dexItemFactory().equalsMethodName), - new EqualsCfCodeProvider(appView, wrapperField.getHolderType(), wrapperField) - .generateCfCode()); - } - public DexEncodedMethod generateVivifiedWrapperConversionWithoutCode( DexMethod method, DexField wrapperField) { DexMethod methodToInstall =
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/apiconversion/DesugaredLibraryWrapperSynthesizer.java b/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/apiconversion/DesugaredLibraryWrapperSynthesizer.java index a930b12..5a34df5 100644 --- a/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/apiconversion/DesugaredLibraryWrapperSynthesizer.java +++ b/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/apiconversion/DesugaredLibraryWrapperSynthesizer.java
@@ -42,7 +42,6 @@ import com.android.tools.r8.synthesis.SyntheticItems.SyntheticKindSelector; import com.android.tools.r8.synthesis.SyntheticMethodBuilder; import com.android.tools.r8.utils.StringDiagnostic; -import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -633,13 +632,6 @@ return generatedMethods; } - private Collection<DexEncodedMethod> synthesizeHashCodeAndEquals( - DexProgramClass wrapper, DexEncodedField wrapperField) { - return ImmutableList.of( - conversionCfProvider.generateWrapperHashCode(wrapperField.getReference()), - conversionCfProvider.generateWrapperEquals(wrapperField.getReference())); - } - DexEncodedMethod newSynthesizedMethod(DexMethod methodToInstall, Code code) { MethodAccessFlags newFlags = MethodAccessFlags.fromSharedAccessFlags( @@ -760,7 +752,6 @@ field, eventConsumer, () -> processingContext.createUniqueContext(wrapper)))); - wrapper.addVirtualMethods(synthesizeHashCodeAndEquals(wrapper, wrapperField)); DexProgramClass vivifiedWrapper = getExistingProgramWrapper(context, kinds -> kinds.VIVIFIED_WRAPPER); DexEncodedField vivifiedWrapperField = getWrapperUniqueEncodedField(vivifiedWrapper); @@ -774,7 +765,5 @@ field, eventConsumer, () -> processingContext.createUniqueContext(wrapper)))); - vivifiedWrapper.addVirtualMethods( - synthesizeHashCodeAndEquals(vivifiedWrapper, vivifiedWrapperField)); } }
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/specificationconversion/HumanToMachineWrapperConverter.java b/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/specificationconversion/HumanToMachineWrapperConverter.java index c0d9157..18147b5 100644 --- a/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/specificationconversion/HumanToMachineWrapperConverter.java +++ b/src/main/java/com/android/tools/r8/ir/desugar/desugaredlibrary/specificationconversion/HumanToMachineWrapperConverter.java
@@ -70,11 +70,7 @@ while (!workList.isEmpty()) { DexClass dexClass = workList.removeFirst(); for (DexEncodedMethod virtualMethod : dexClass.virtualMethods()) { - if (!virtualMethod.isPrivateMethod() - // Don't include hashCode and equals overrides, as hashCode and equals are added to - // all wrappers regardless. - && (!appInfo.dexItemFactory().objectMembers.hashCode.match(virtualMethod)) - && (!appInfo.dexItemFactory().objectMembers.equals.match(virtualMethod))) { + if (!virtualMethod.isPrivateMethod()) { assert virtualMethod.isProtectedMethod() || virtualMethod.isPublicMethod(); boolean alreadyAdded = wrappers.contains(equivalence.wrap(virtualMethod.getReference())); // This looks quadratic but given the size of the collections met in practice for
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/apiconverter/EqualsCfCodeProvider.java b/src/main/java/com/android/tools/r8/ir/synthetic/apiconverter/EqualsCfCodeProvider.java deleted file mode 100644 index d6d86ce..0000000 --- a/src/main/java/com/android/tools/r8/ir/synthetic/apiconverter/EqualsCfCodeProvider.java +++ /dev/null
@@ -1,88 +0,0 @@ -// Copyright (c) 2022, the R8 project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -package com.android.tools.r8.ir.synthetic.apiconverter; - -import com.android.tools.r8.cf.code.CfCheckCast; -import com.android.tools.r8.cf.code.CfFrame; -import com.android.tools.r8.cf.code.CfFrame.FrameType; -import com.android.tools.r8.cf.code.CfGoto; -import com.android.tools.r8.cf.code.CfIf; -import com.android.tools.r8.cf.code.CfInstanceFieldRead; -import com.android.tools.r8.cf.code.CfInstanceOf; -import com.android.tools.r8.cf.code.CfInstruction; -import com.android.tools.r8.cf.code.CfInvoke; -import com.android.tools.r8.cf.code.CfLabel; -import com.android.tools.r8.cf.code.CfLoad; -import com.android.tools.r8.cf.code.CfReturn; -import com.android.tools.r8.graph.AppView; -import com.android.tools.r8.graph.CfCode; -import com.android.tools.r8.graph.DexField; -import com.android.tools.r8.graph.DexType; -import com.android.tools.r8.ir.code.If; -import com.android.tools.r8.ir.code.ValueType; -import com.android.tools.r8.ir.synthetic.SyntheticCfCodeProvider; -import it.unimi.dsi.fastutil.ints.Int2ObjectAVLTreeMap; -import java.util.ArrayDeque; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import org.objectweb.asm.Opcodes; - -public class EqualsCfCodeProvider extends SyntheticCfCodeProvider { - - private final DexField wrapperField; - - public EqualsCfCodeProvider(AppView<?> appView, DexType holder, DexField wrapperField) { - super(appView, holder); - this.wrapperField = wrapperField; - } - - @Override - public CfCode generateCfCode() { - // return wrapperField.equals( - // other instanceof WrapperType ? ((WrapperType) other).wrapperField : other); - CfLabel label1 = new CfLabel(); - CfLabel label2 = new CfLabel(); - List<CfInstruction> instructions = new ArrayList<>(); - instructions.add(new CfLoad(ValueType.OBJECT, 0)); - instructions.add(new CfInstanceFieldRead(wrapperField)); - instructions.add(new CfLoad(ValueType.OBJECT, 1)); - DexType wrapperType = wrapperField.getHolderType(); - instructions.add(new CfInstanceOf(wrapperType)); - instructions.add(new CfIf(If.Type.EQ, ValueType.INT, label1)); - instructions.add(new CfLoad(ValueType.OBJECT, 1)); - instructions.add(new CfCheckCast(wrapperType)); - instructions.add(new CfInstanceFieldRead(wrapperField)); - instructions.add(new CfGoto(label2)); - instructions.add(label1); - instructions.add( - new CfFrame( - new Int2ObjectAVLTreeMap<>( - new int[] {0, 1}, - new FrameType[] { - FrameType.initialized(wrapperType), - FrameType.initialized(appView.dexItemFactory().objectType) - }), - new ArrayDeque<>(Arrays.asList(FrameType.initialized(wrapperType))))); - instructions.add(new CfLoad(ValueType.OBJECT, 1)); - instructions.add(label2); - instructions.add( - new CfFrame( - new Int2ObjectAVLTreeMap<>( - new int[] {0, 1}, - new FrameType[] { - FrameType.initialized(wrapperType), - FrameType.initialized(appView.dexItemFactory().objectType) - }), - new ArrayDeque<>( - Arrays.asList( - FrameType.initialized(wrapperType), - FrameType.initialized(appView.dexItemFactory().objectType))))); - instructions.add( - new CfInvoke(Opcodes.INVOKEVIRTUAL, appView.dexItemFactory().objectMembers.equals, false)); - instructions.add(new CfReturn(ValueType.INT)); - return standardCfCodeFromInstructions(instructions); - } -}
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/apiconverter/HashCodeCfCodeProvider.java b/src/main/java/com/android/tools/r8/ir/synthetic/apiconverter/HashCodeCfCodeProvider.java deleted file mode 100644 index 3c5f717..0000000 --- a/src/main/java/com/android/tools/r8/ir/synthetic/apiconverter/HashCodeCfCodeProvider.java +++ /dev/null
@@ -1,42 +0,0 @@ -// Copyright (c) 2022, the R8 project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -package com.android.tools.r8.ir.synthetic.apiconverter; - -import com.android.tools.r8.cf.code.CfInstanceFieldRead; -import com.android.tools.r8.cf.code.CfInstruction; -import com.android.tools.r8.cf.code.CfInvoke; -import com.android.tools.r8.cf.code.CfLoad; -import com.android.tools.r8.cf.code.CfReturn; -import com.android.tools.r8.graph.AppView; -import com.android.tools.r8.graph.CfCode; -import com.android.tools.r8.graph.DexField; -import com.android.tools.r8.graph.DexType; -import com.android.tools.r8.ir.code.ValueType; -import com.android.tools.r8.ir.synthetic.SyntheticCfCodeProvider; -import java.util.ArrayList; -import java.util.List; -import org.objectweb.asm.Opcodes; - -public class HashCodeCfCodeProvider extends SyntheticCfCodeProvider { - - private final DexField wrapperField; - - public HashCodeCfCodeProvider(AppView<?> appView, DexType holder, DexField wrapperField) { - super(appView, holder); - this.wrapperField = wrapperField; - } - - @Override - public CfCode generateCfCode() { - List<CfInstruction> instructions = new ArrayList<>(); - instructions.add(new CfLoad(ValueType.OBJECT, 0)); - instructions.add(new CfInstanceFieldRead(wrapperField)); - instructions.add( - new CfInvoke( - Opcodes.INVOKEVIRTUAL, appView.dexItemFactory().objectMembers.hashCode, false)); - instructions.add(new CfReturn(ValueType.INT)); - return standardCfCodeFromInstructions(instructions); - } -}
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/ClockAPIConversionTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/ClockAPIConversionTest.java index 53edf4c..7827d8a 100644 --- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/ClockAPIConversionTest.java +++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/ClockAPIConversionTest.java
@@ -34,9 +34,10 @@ private static final AndroidApiLevel MIN_SUPPORTED = AndroidApiLevel.O; private static final String EXPECTED_RESULT = StringUtils.lines("Z", "Z", "true", "Z", "Z", "true", "true", "true", "true", "true", "true"); - private static final String DESUGARED_LIBRARY_EXPECTED_RESULT = + // TODO(b/230800107): There should not be any unexpected results. + private static final String UNEXPECTED_RESULT = StringUtils.lines( - "Z", "Z", "true", "Z", "Z", "true", "true", "false", "false", "true", "true"); + "Z", "Z", "true", "Z", "Z", "false", "true", "false", "false", "false", "false"); @Parameters(name = "{0}, spec: {1}, {2}") public static List<Object[]> data() { @@ -63,7 +64,7 @@ new CustomLibrarySpecification(CustomLibClass.class, MIN_SUPPORTED)) .addKeepMainRule(Executor.class) .run(parameters.getRuntime(), Executor.class) - .assertSuccessWithOutput(DESUGARED_LIBRARY_EXPECTED_RESULT); + .assertSuccessWithOutput(UNEXPECTED_RESULT); } @Test @@ -115,8 +116,8 @@ System.out.println(CustomLibClass.getClockss()[0][0].getZone()); System.out.println(clock1.equals(CustomLibClass.getClock())); System.out.println(localClock.equals(Clock.systemUTC())); - System.out.println(localClock.equals(clock1)); // Prints false with desugared library. - System.out.println(clock1.equals(localClock)); // Prints false with desugared library. + System.out.println(localClock.equals(clock1)); + System.out.println(clock1.equals(localClock)); System.out.println(clock1.equals(CustomLibClass.getClocks()[0])); System.out.println(clock1.equals(CustomLibClass.getClockss()[0][0])); }
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/WrapperEqualityTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/WrapperEqualityTest.java index 1353145..9135130 100644 --- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/WrapperEqualityTest.java +++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/conversiontests/WrapperEqualityTest.java
@@ -17,7 +17,6 @@ import com.android.tools.r8.desugar.desugaredlibrary.test.LibraryDesugaringSpecification; import com.android.tools.r8.utils.AndroidApiLevel; import com.android.tools.r8.utils.StringUtils; -import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -37,13 +36,10 @@ private static final AndroidApiLevel MIN_SUPPORTED = AndroidApiLevel.N; private static final String EXPECTED_RESULT = - StringUtils.lines( - "true", "true", "true", "true", "true", "true", "true", "true", "false", "false", "1", - "1", "2", "2", "1", "1", "1", "0", "true", "true", "true", "true"); - private static final String DESUGARED_LIBRARY_EXPECTED_RESULT = - StringUtils.lines( - "true", "true", "true", "true", "false", "true", "false", "true", "false", "false", "1", - "1", "2", "2", "1", "1", "1", "0", "false", "true", "false", "true"); + StringUtils.lines("true", "true", "1", "1", "2", "2", "1", "1", "1", "0"); + // TODO(b/230800107): There should not be any unexpected results. + private static final String UNEXPECTED_RESULT = + StringUtils.lines("false", "false", "1", "2", "3", "4", "2", "2", "4", "4"); @Parameters(name = "{0}, spec: {1}, {2}") public static List<Object[]> data() { @@ -71,7 +67,7 @@ .addKeepMainRule(Executor.class) .compile() .run(parameters.getRuntime(), Executor.class) - .assertSuccessWithOutput(DESUGARED_LIBRARY_EXPECTED_RESULT); + .assertSuccessWithOutput(UNEXPECTED_RESULT); } @Test @@ -97,22 +93,8 @@ public static void main(String[] args) { Consumer<Boolean> consumer = b -> {}; Supplier<Boolean> supplier = () -> Boolean.TRUE; - // Prints true for desugared library as the same wrapper is used for both arguments. - System.out.println(CustomLibClass.same(consumer, consumer)); System.out.println(CustomLibClass.equals(consumer, consumer)); - // Prints true for desugared library as the same wrapper is used for both arguments. - System.out.println(CustomLibClass.same(supplier, supplier)); System.out.println(CustomLibClass.equals(supplier, supplier)); - - CustomLibClass.setConsumer(consumer); - CustomLibClass.setSupplier(supplier); - System.out.println(CustomLibClass.same(consumer)); - System.out.println(CustomLibClass.equals(consumer)); - System.out.println(CustomLibClass.same(supplier)); - System.out.println(CustomLibClass.equals(supplier)); - System.out.println(CustomLibClass.equalsWithObject(consumer, new HashMap<>())); - System.out.println(CustomLibClass.equalsWithObject(supplier, new ArrayList<>())); - CustomLibClass.register(consumer, new Object()); System.out.println(CustomLibClass.registrations()); CustomLibClass.register(consumer, new Object()); @@ -127,19 +109,6 @@ System.out.println(CustomLibClass.registrations()); CustomLibClass.unregister(supplier); System.out.println(CustomLibClass.registrations()); - - // Prints false for desugared library as wrappers does not keep identity. - System.out.println( - CustomLibClass.getConsumerFromPlatform() == CustomLibClass.getConsumerFromPlatform()); - System.out.println( - CustomLibClass.getConsumerFromPlatform() - .equals(CustomLibClass.getConsumerFromPlatform())); - // Prints false for desugared library as wrappers does not keep identity. - System.out.println( - CustomLibClass.getSupplierFromPlatform() == CustomLibClass.getSupplierFromPlatform()); - System.out.println( - CustomLibClass.getSupplierFromPlatform() - .equals(CustomLibClass.getSupplierFromPlatform())); } } @@ -147,59 +116,14 @@ // This class is convenient for easy testing. Each method plays the role of methods in the // platform APIs for which argument/return values need conversion. static class CustomLibClass { - private static final Map<Object, Object> map = new HashMap<>(); - private static final Consumer<Boolean> consumer = b -> {}; - private static final Supplier<Boolean> supplier = () -> Boolean.TRUE; + static Map<Object, Object> map = new HashMap<>(); - private static Consumer<Boolean> appConsumer; - private static Supplier<Boolean> appSupplier; - - public static boolean equals(Consumer<Boolean> consumer1, Consumer<Boolean> consumer2) { - return consumer1.equals(consumer2) && consumer2.equals(consumer1); + public static boolean equals(Consumer<Boolean> listener1, Consumer<Boolean> listene2) { + return listener1.equals(listene2) && listene2.equals(listener1); } - public static boolean equals(Supplier<Boolean> supplier1, Supplier<Boolean> supplier2) { - return supplier1.equals(supplier2) && supplier2.equals(supplier1); - } - - public static boolean same(Consumer<Boolean> consumer1, Consumer<Boolean> consumer2) { - return consumer1.equals(consumer2) && consumer2.equals(consumer1); - } - - public static boolean same(Supplier<Boolean> supplier1, Supplier<Boolean> supplier2) { - return supplier1.equals(supplier2) && supplier2.equals(supplier1); - } - - public static void setConsumer(Consumer<Boolean> consumer) { - appConsumer = consumer; - } - - public static void setSupplier(Supplier supplier) { - appSupplier = supplier; - } - - public static boolean equals(Consumer<Boolean> consumer) { - return consumer.equals(appConsumer) && appConsumer.equals(consumer); - } - - public static boolean equals(Supplier<Boolean> supplier) { - return supplier.equals(appSupplier) && appSupplier.equals(supplier); - } - - public static boolean same(Consumer<Boolean> consumer) { - return appConsumer == consumer; - } - - public static boolean same(Supplier supplier) { - return appSupplier == supplier; - } - - public static boolean equalsWithObject(Consumer<Boolean> consumer, Object object) { - return consumer.equals(object); - } - - public static boolean equalsWithObject(Supplier<Boolean> supplier, Object object) { - return supplier.equals(object); + public static boolean equals(Supplier<Boolean> listener1, Supplier<Boolean> listene2) { + return listener1.equals(listene2) && listene2.equals(listener1); } public static void register(Consumer<Boolean> listener, Object context) { @@ -229,13 +153,5 @@ public static long suppliers() { return map.keySet().stream().filter(k -> k instanceof Supplier).count(); } - - public static Supplier<Boolean> getSupplierFromPlatform() { - return supplier; - } - - public static Consumer<Boolean> getConsumerFromPlatform() { - return consumer; - } } }