Merge commit 'a2cce1cdf11c888a85941ce82f8446bc53d093cb' into dev-release
diff --git a/src/main/java/com/android/tools/r8/BaseCommand.java b/src/main/java/com/android/tools/r8/BaseCommand.java index 9bfdfcd..036dd57 100644 --- a/src/main/java/com/android/tools/r8/BaseCommand.java +++ b/src/main/java/com/android/tools/r8/BaseCommand.java
@@ -289,10 +289,10 @@ /** * Add main-dex classes. * - * Add classes to keep in the primary dex file (<code>classes.dex</code>). + * <p>Add classes to keep in the primary dex file (<code>classes.dex</code>). * - * NOTE: The name of the classes is specified using the Java fully qualified names format - * (e.g. "com.example.MyClass"), and <i>not</i> the format used by the main-dex list file. + * <p>NOTE: The name of the classes is specified using the Java fully qualified names format + * (e.g. "com.example.MyClass$A"), and <i>not</i> the format used by the main-dex list file. */ public B addMainDexClasses(String... classes) { guard(() -> app.addMainDexClasses(classes));
diff --git a/src/main/java/com/android/tools/r8/CompatDxHelper.java b/src/main/java/com/android/tools/r8/CompatDxHelper.java index beec3dc..13cd385 100644 --- a/src/main/java/com/android/tools/r8/CompatDxHelper.java +++ b/src/main/java/com/android/tools/r8/CompatDxHelper.java
@@ -25,8 +25,4 @@ public static void ignoreDexInArchive(BaseCommand.Builder builder) { builder.setIgnoreDexInArchive(true); } - - public static void enableDesugarBackportStatics(D8Command.Builder builder) { - builder.enableDesugarBackportStatics(); - } }
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java index ab0d4dd..0a38fb5 100644 --- a/src/main/java/com/android/tools/r8/D8.java +++ b/src/main/java/com/android/tools/r8/D8.java
@@ -33,6 +33,7 @@ import com.android.tools.r8.origin.CommandLineOrigin; import com.android.tools.r8.origin.Origin; import com.android.tools.r8.synthesis.SyntheticFinalization; +import com.android.tools.r8.synthesis.SyntheticItems; import com.android.tools.r8.utils.AndroidApp; import com.android.tools.r8.utils.CfgPrinter; import com.android.tools.r8.utils.ExceptionUtils; @@ -180,6 +181,7 @@ options.disableGlobalOptimizations(); AppView<AppInfo> appView = readApp(inputApp, options, executor, timing); + SyntheticItems.collectSyntheticInputs(appView); final CfgPrinter printer = options.printCfg ? new CfgPrinter() : null;
diff --git a/src/main/java/com/android/tools/r8/D8Command.java b/src/main/java/com/android/tools/r8/D8Command.java index cb00bb7..9e462dd 100644 --- a/src/main/java/com/android/tools/r8/D8Command.java +++ b/src/main/java/com/android/tools/r8/D8Command.java
@@ -163,12 +163,6 @@ return self(); } - // Internal helper for compat tools to make them only desugar backports. - Builder enableDesugarBackportStatics() { - this.desugarState = DesugarState.ONLY_BACKPORT_STATICS; - return self(); - } - @Override Builder self() { return this;
diff --git a/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java b/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java index f6a10a9..07569a7 100644 --- a/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java +++ b/src/main/java/com/android/tools/r8/GenerateMainDexListCommand.java
@@ -223,7 +223,7 @@ @Override InternalOptions getInternalOptions() { InternalOptions internal = new InternalOptions(factory, reporter); - internal.programConsumer = ClassFileConsumer.emptyConsumer(); + internal.programConsumer = DexIndexedConsumer.emptyConsumer(); internal.mainDexKeepRules = mainDexKeepRules; internal.mainDexListConsumer = mainDexListConsumer; internal.mainDexKeptGraphConsumer = mainDexKeptGraphConsumer;
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java index c98bafd..16d3642 100644 --- a/src/main/java/com/android/tools/r8/R8.java +++ b/src/main/java/com/android/tools/r8/R8.java
@@ -978,7 +978,7 @@ } private static boolean verifyOriginalMethodInPosition(CfCode code, DexMethod originalMethod) { - for (CfInstruction instruction : code.instructions) { + for (CfInstruction instruction : code.getInstructions()) { if (!instruction.isPosition()) { continue; }
diff --git a/src/main/java/com/android/tools/r8/cf/code/CfInvoke.java b/src/main/java/com/android/tools/r8/cf/code/CfInvoke.java index 4256db3..5dfc5cb 100644 --- a/src/main/java/com/android/tools/r8/cf/code/CfInvoke.java +++ b/src/main/java/com/android/tools/r8/cf/code/CfInvoke.java
@@ -120,7 +120,7 @@ registry.registerInvokeInterface(method); break; case STATIC: - registry.registerInvokeStatic(method); + registry.registerInvokeStatic(method, itf); break; case SUPER: registry.registerInvokeSuper(method);
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 bb5706d..5b45b7b 100644 --- a/src/main/java/com/android/tools/r8/dex/FileWriter.java +++ b/src/main/java/com/android/tools/r8/dex/FileWriter.java
@@ -42,6 +42,7 @@ import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.DexTypeList; import com.android.tools.r8.graph.DexValue; +import com.android.tools.r8.graph.GraphLens; import com.android.tools.r8.graph.IndexedDexItem; import com.android.tools.r8.graph.ObjectToOffsetMapping; import com.android.tools.r8.graph.ParameterAnnotationsList; @@ -100,6 +101,7 @@ private final MethodToCodeObjectMapping codeMapping; private final DexApplication application; private final InternalOptions options; + private final GraphLens graphLens; private final NamingLens namingLens; private final DexOutputBuffer dest; private final MixedSectionOffsets mixedSectionOffsets; @@ -118,6 +120,7 @@ this.codeMapping = codeMapping; this.application = application; this.options = options; + this.graphLens = mapping.getGraphLens(); this.namingLens = namingLens; this.dest = new DexOutputBuffer(provider); this.mixedSectionOffsets = new MixedSectionOffsets(options, codeMapping); @@ -416,8 +419,7 @@ result += LebUtils .sizeAsSleb128(hasCatchAll ? -handler.pairs.length : handler.pairs.length); for (TypeAddrPair pair : handler.pairs) { - - result += sizeAsUleb128(mapping.getOffsetFor(pair.type)); + result += sizeAsUleb128(mapping.getOffsetFor(pair.getType(graphLens))); result += sizeAsUleb128(pair.addr); } if (hasCatchAll) { @@ -525,9 +527,9 @@ boolean hasCatchAll = handler.catchAllAddr != TryHandler.NO_HANDLER; dest.putSleb128(hasCatchAll ? -handler.pairs.length : handler.pairs.length); for (TypeAddrPair pair : handler.pairs) { - dest.putUleb128(mapping.getOffsetFor(pair.type)); + dest.putUleb128(mapping.getOffsetFor(pair.getType(graphLens))); dest.putUleb128(pair.addr); - desugaredLibraryCodeToKeep.recordClass(pair.type); + desugaredLibraryCodeToKeep.recordClass(pair.getType(graphLens)); } if (hasCatchAll) { dest.putUleb128(handler.catchAllAddr);
diff --git a/src/main/java/com/android/tools/r8/dex/JumboStringRewriter.java b/src/main/java/com/android/tools/r8/dex/JumboStringRewriter.java index 128ea1d..d1f8749 100644 --- a/src/main/java/com/android/tools/r8/dex/JumboStringRewriter.java +++ b/src/main/java/com/android/tools/r8/dex/JumboStringRewriter.java
@@ -206,7 +206,7 @@ TypeAddrPair[] newPairs = new TypeAddrPair[handler.pairs.length]; for (int j = 0; j < handler.pairs.length; j++) { TypeAddrPair pair = handler.pairs[j]; - newPairs[j] = new TypeAddrPair(pair.type, it.next().getOffset()); + newPairs[j] = new TypeAddrPair(pair.getType(), it.next().getOffset()); } result[i] = new TryHandler(newPairs, catchAllAddr); }
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfo.java b/src/main/java/com/android/tools/r8/graph/AppInfo.java index d2d137b..114bc8a 100644 --- a/src/main/java/com/android/tools/r8/graph/AppInfo.java +++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -32,8 +32,7 @@ public static AppInfo createInitialAppInfo( DexApplication application, MainDexClasses mainDexClasses) { - return new AppInfo( - SyntheticItems.createInitialSyntheticItems().commit(application), mainDexClasses); + return new AppInfo(SyntheticItems.createInitialSyntheticItems(application), mainDexClasses); } public AppInfo(CommittedItems committedItems, MainDexClasses mainDexClasses) {
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java index a1e3d15..ace1d56 100644 --- a/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java +++ b/src/main/java/com/android/tools/r8/graph/AppInfoWithClassHierarchy.java
@@ -54,7 +54,7 @@ ClassToFeatureSplitMap classToFeatureSplitMap, MainDexClasses mainDexClasses) { return new AppInfoWithClassHierarchy( - SyntheticItems.createInitialSyntheticItems().commit(application), + SyntheticItems.createInitialSyntheticItems(application), classToFeatureSplitMap, mainDexClasses); }
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java index f4407b4..3850a41 100644 --- a/src/main/java/com/android/tools/r8/graph/AppView.java +++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -78,6 +78,7 @@ private HorizontallyMergedClasses horizontallyMergedClasses; private VerticallyMergedClasses verticallyMergedClasses; private EnumValueInfoMapCollection unboxedEnums = EnumValueInfoMapCollection.empty(); + // TODO(b/169115389): Remove private Set<DexMethod> cfByteCodePassThrough = ImmutableSet.of(); private Map<DexClass, DexValueString> sourceDebugExtensions = new IdentityHashMap<>();
diff --git a/src/main/java/com/android/tools/r8/graph/CfCode.java b/src/main/java/com/android/tools/r8/graph/CfCode.java index b6355dc..02af4ee 100644 --- a/src/main/java/com/android/tools/r8/graph/CfCode.java +++ b/src/main/java/com/android/tools/r8/graph/CfCode.java
@@ -126,7 +126,7 @@ private final int maxStack; private int maxLocals; - public List<CfInstruction> instructions; + private List<CfInstruction> instructions; private final List<CfTryCatch> tryCatchRanges; private final List<LocalVariableInfo> localVariables; @@ -169,6 +169,10 @@ return Collections.unmodifiableList(instructions); } + public void setInstructions(List<CfInstruction> instructions) { + this.instructions = instructions; + } + public List<LocalVariableInfo> getLocalVariables() { return Collections.unmodifiableList(localVariables); } @@ -242,6 +246,13 @@ } private boolean shouldAddParameterNames(DexEncodedMethod method, AppView<?> appView) { + // In cf to cf desugar we do pass through of code and don't move around methods. + // TODO(b/169115389): Remove when we have a way to determine if we need parameter names per + // method. + if (appView.options().cfToCfDesugar) { + return false; + } + // Don't add parameter information if the code already has full debug information. // Note: This fast path can cause a method to loose its parameter info, if the debug info turned // out to be invalid during IR building.
diff --git a/src/main/java/com/android/tools/r8/graph/DexAnnotation.java b/src/main/java/com/android/tools/r8/graph/DexAnnotation.java index ade8d06..5bdddd9 100644 --- a/src/main/java/com/android/tools/r8/graph/DexAnnotation.java +++ b/src/main/java/com/android/tools/r8/graph/DexAnnotation.java
@@ -77,6 +77,7 @@ } if (annotation == options.itemFactory.dalvikFastNativeAnnotation || annotation == options.itemFactory.dalvikCriticalNativeAnnotation + || annotation == options.itemFactory.annotationSynthesizedClass || annotation == options.itemFactory.annotationSynthesizedClassMap) { return true; } @@ -347,7 +348,7 @@ throw new CompilationError(getInvalidSynthesizedClassMapMessage(clazz, foundAnnotation)); } DexAnnotationElement value = foundAnnotation.annotation.elements[0]; - if (!value.name.toSourceString().equals("value")) { + if (value.name != dexItemFactory.valueString) { throw new CompilationError(getInvalidSynthesizedClassMapMessage(clazz, foundAnnotation)); } DexValueArray existingList = value.value.asDexValueArray(); @@ -375,6 +376,43 @@ + ": " + invalidAnnotation.toString(); } + public static DexAnnotation createAnnotationSynthesizedClass( + DexType synthesizingContext, DexItemFactory dexItemFactory) { + DexValueType value = new DexValueType(synthesizingContext); + DexAnnotationElement element = new DexAnnotationElement(dexItemFactory.valueString, value); + return new DexAnnotation( + VISIBILITY_BUILD, + new DexEncodedAnnotation( + dexItemFactory.annotationSynthesizedClass, new DexAnnotationElement[] {element})); + } + + public static boolean hasSynthesizedClassAnnotation( + DexAnnotationSet annotations, DexItemFactory factory) { + return getSynthesizedClassAnnotationContextType(annotations, factory) != null; + } + + public static DexType getSynthesizedClassAnnotationContextType( + DexAnnotationSet annotations, DexItemFactory factory) { + if (annotations.size() != 1) { + return null; + } + DexAnnotation annotation = annotations.annotations[0]; + if (annotation.annotation.type != factory.annotationSynthesizedClass) { + return null; + } + if (annotation.annotation.elements.length != 1) { + return null; + } + DexAnnotationElement element = annotation.annotation.elements[0]; + if (element.name != factory.valueString) { + return null; + } + if (!element.value.isDexValueType()) { + return null; + } + return element.value.asDexValueType().getValue(); + } + public static DexAnnotation createAnnotationSynthesizedClassMap( TreeSet<DexType> synthesized, DexItemFactory dexItemFactory) {
diff --git a/src/main/java/com/android/tools/r8/graph/DexCode.java b/src/main/java/com/android/tools/r8/graph/DexCode.java index ce47f1e..eaf8d7f 100644 --- a/src/main/java/com/android/tools/r8/graph/DexCode.java +++ b/src/main/java/com/android/tools/r8/graph/DexCode.java
@@ -573,7 +573,7 @@ public static class TypeAddrPair extends DexItem implements Comparable<TypeAddrPair> { - public final DexType type; + private final DexType type; public final /* offset */ int addr; public TypeAddrPair(DexType type, int addr) { @@ -581,8 +581,16 @@ this.addr = addr; } + public DexType getType() { + return type; + } + + public DexType getType(GraphLens lens) { + return lens.lookupType(type); + } + public void collectIndexedItems(IndexedItemCollection indexedItems, GraphLens graphLens) { - DexType rewritten = graphLens.lookupType(type); + DexType rewritten = getType(graphLens); rewritten.collectIndexedItems(indexedItems); }
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java index 4936ceb..f049ec5 100644 --- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java +++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -281,8 +281,8 @@ // TODO(b/158159959): Implement a more precise hashing on code objects. if (code.isCfCode()) { CfCode cfCode = code.asCfCode(); - hasher.putInt(cfCode.instructions.size()); - for (CfInstruction instruction : cfCode.instructions) { + hasher.putInt(cfCode.getInstructions().size()); + for (CfInstruction instruction : cfCode.getInstructions()) { hasher.putInt(instruction.getClass().hashCode()); } } else {
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java index 4c3779e..18ebdb4 100644 --- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java +++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -297,6 +297,8 @@ public final DexString throwableArrayDescriptor = createString("[Ljava/lang/Throwable;"); + public final DexString valueString = createString("value"); + public final DexType booleanType = createStaticallyKnownType(booleanDescriptor); public final DexType byteType = createStaticallyKnownType(byteDescriptor); public final DexType charType = createStaticallyKnownType(charDescriptor); @@ -543,6 +545,8 @@ public final DexType annotationSourceDebugExtension = createStaticallyKnownType("Ldalvik/annotation/SourceDebugExtension;"); public final DexType annotationThrows = createStaticallyKnownType("Ldalvik/annotation/Throws;"); + public final DexType annotationSynthesizedClass = + createStaticallyKnownType("Lcom/android/tools/r8/annotations/SynthesizedClass;"); public final DexType annotationSynthesizedClassMap = createStaticallyKnownType("Lcom/android/tools/r8/annotations/SynthesizedClassMap;"); public final DexType annotationCovariantReturnType =
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLens.java b/src/main/java/com/android/tools/r8/graph/GraphLens.java index 7bfa6cb..b276437 100644 --- a/src/main/java/com/android/tools/r8/graph/GraphLens.java +++ b/src/main/java/com/android/tools/r8/graph/GraphLens.java
@@ -986,12 +986,16 @@ // TODO(sgjesse): Should we always do interface to virtual mapping? Is it a performance win // that only subclasses which are known to need it actually do it? DexMethod rewrittenReboundReference = previous.getRewrittenReboundReference(methodMap); - return MethodLookupResult.builder(this) - .setReboundReference(rewrittenReboundReference) - .setReference( + DexMethod rewrittenReference = + previous.getReference() == previous.getReboundReference() + ? rewrittenReboundReference + : // This assumes that the holder will always be moved in lock-step with the method! rewrittenReboundReference.withHolder( internalDescribeLookupClassType(previous.getReference().getHolderType()), - dexItemFactory)) + dexItemFactory); + return MethodLookupResult.builder(this) + .setReboundReference(rewrittenReboundReference) + .setReference(rewrittenReference) .setPrototypeChanges( internalDescribePrototypeChanges( previous.getPrototypeChanges(), rewrittenReboundReference))
diff --git a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java index 263a21b..f783b63 100644 --- a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java +++ b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
@@ -248,7 +248,9 @@ Origin origin, boolean shouldApplyCodeRewritings) { if (!cfCode.verifyFrames(method, appView, origin, shouldApplyCodeRewritings)) { - cfCode.instructions.removeIf(CfInstruction::isFrame); + ArrayList<CfInstruction> newInstructions = new ArrayList<>(cfCode.getInstructions()); + newInstructions.removeIf(CfInstruction::isFrame); + cfCode.setInstructions(newInstructions); } return cfCode; }
diff --git a/src/main/java/com/android/tools/r8/graph/UseRegistry.java b/src/main/java/com/android/tools/r8/graph/UseRegistry.java index 5830357..033182e 100644 --- a/src/main/java/com/android/tools/r8/graph/UseRegistry.java +++ b/src/main/java/com/android/tools/r8/graph/UseRegistry.java
@@ -45,6 +45,10 @@ registerInstanceFieldWrite(field); } + public void registerInvokeStatic(DexMethod method, boolean itf) { + registerInvokeStatic(method); + } + public abstract void registerNewInstance(DexType type); public abstract void registerStaticFieldRead(DexField field);
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java b/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java index c57a3da..d60a589 100644 --- a/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java +++ b/src/main/java/com/android/tools/r8/graph/analysis/ClassInitializerAssertionEnablingAnalysis.java
@@ -102,8 +102,8 @@ ImmutableList.of(CfConstNumber.class, CfLogicalBinop.class, CfFieldInstruction.class); private boolean hasJavacClinitAssertionCode(CfCode code) { - for (int i = 0; i < code.instructions.size(); i++) { - CfInstruction instruction = code.instructions.get(i); + for (int i = 0; i < code.getInstructions().size(); i++) { + CfInstruction instruction = code.getInstructions().get(i); if (instruction.isInvoke()) { // Check for the generated instruction sequence by looking for the call to // desiredAssertionStatus() followed by the expected instruction types and finally checking @@ -132,8 +132,9 @@ private boolean hasKotlincClinitAssertionCode(ProgramMethod method) { if (method.getHolderType() == dexItemFactory.kotlin.assertions.type) { CfCode code = method.getDefinition().getCode().asCfCode(); - for (int i = 1; i < code.instructions.size(); i++) { - CfInstruction instruction = code.instructions.get(i - 1); + List<CfInstruction> instructions = code.getInstructions(); + for (int i = 1; i < instructions.size(); i++) { + CfInstruction instruction = instructions.get(i - 1); if (instruction.isInvoke()) { // Check for the generated instruction sequence by looking for the call to // desiredAssertionStatus() followed by the expected instruction types and finally @@ -141,8 +142,8 @@ CfInvoke invoke = instruction.asInvoke(); if (invoke.getOpcode() == Opcodes.INVOKEVIRTUAL && invoke.getMethod() == dexItemFactory.classMethods.desiredAssertionStatus) { - if (code.instructions.get(i).isFieldInstruction()) { - CfFieldInstruction fieldInstruction = code.instructions.get(i).asFieldInstruction(); + if (instructions.get(i).isFieldInstruction()) { + CfFieldInstruction fieldInstruction = instructions.get(i).asFieldInstruction(); if (fieldInstruction.getOpcode() == Opcodes.PUTSTATIC && fieldInstruction.getField().name == kotlinAssertionsEnabled) { return true; @@ -164,9 +165,9 @@ int nextExpectedInstructionIndex = 0; CfInstruction instruction = null; for (int i = fromIndex; - i < code.instructions.size() && nextExpectedInstructionIndex < sequence.size(); + i < code.getInstructions().size() && nextExpectedInstructionIndex < sequence.size(); i++) { - instruction = code.instructions.get(i); + instruction = code.getInstructions().get(i); if (instruction.isLabel() || instruction.isFrame()) { // Just ignore labels and frames. continue; @@ -186,9 +187,9 @@ int nextExpectedInstructionIndex = 0; CfInstruction instruction = null; for (int i = fromIndex; - i < code.instructions.size() && nextExpectedInstructionIndex < sequence.size(); + i < code.getInstructions().size() && nextExpectedInstructionIndex < sequence.size(); i++) { - instruction = code.instructions.get(i); + instruction = code.getInstructions().get(i); if (instruction.isStore() || instruction.isLoad()) { // Just ignore stores and loads. continue;
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/CanOnlyMergeIntoClassPolicy.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/CanOnlyMergeIntoClassPolicy.java new file mode 100644 index 0000000..fa9873a --- /dev/null +++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/CanOnlyMergeIntoClassPolicy.java
@@ -0,0 +1,17 @@ +// Copyright (c) 2020, 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.horizontalclassmerging; + +import com.android.tools.r8.graph.DexProgramClass; + +public abstract class CanOnlyMergeIntoClassPolicy extends SingleClassPolicy { + public abstract boolean canOnlyMergeInto(DexProgramClass clazz); + + @Override + public boolean canMerge(DexProgramClass program) { + // TODO(b/165577835): Allow merging of classes that must be the target of their merge group. + return !canOnlyMergeInto(program); + } +}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java index c993870..d33b15d 100644 --- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java +++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -17,8 +17,10 @@ import com.android.tools.r8.horizontalclassmerging.policies.NoStaticClassInitializer; import com.android.tools.r8.horizontalclassmerging.policies.NotEntryPoint; import com.android.tools.r8.horizontalclassmerging.policies.NotMatchedByNoHorizontalClassMerging; +import com.android.tools.r8.horizontalclassmerging.policies.PreventChangingVisibility; import com.android.tools.r8.horizontalclassmerging.policies.PreventMergeIntoMainDex; import com.android.tools.r8.horizontalclassmerging.policies.RespectPackageBoundaries; +import com.android.tools.r8.horizontalclassmerging.policies.SameFeatureSplit; import com.android.tools.r8.horizontalclassmerging.policies.SameParentClass; import com.android.tools.r8.shaking.AppInfoWithLiveness; import com.android.tools.r8.shaking.ClassMergingEnqueuerExtension; @@ -56,6 +58,8 @@ new NotEntryPoint(appView.dexItemFactory()), new PreventMergeIntoMainDex(appView, mainDexTracingResult), new SameParentClass(), + new PreventChangingVisibility(), + new SameFeatureSplit(appView), new RespectPackageBoundaries(appView), new DontMergeSynchronizedClasses(appView) // TODO: add policies
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/MultiClassSameReferencePolicy.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/MultiClassSameReferencePolicy.java new file mode 100644 index 0000000..3407306 --- /dev/null +++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/MultiClassSameReferencePolicy.java
@@ -0,0 +1,26 @@ +// Copyright (c) 2020, 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.horizontalclassmerging; + +import com.android.tools.r8.graph.DexProgramClass; +import java.util.Collection; +import java.util.IdentityHashMap; +import java.util.LinkedList; +import java.util.Map; + +public abstract class MultiClassSameReferencePolicy<T> extends MultiClassPolicy { + + @Override + public final Collection<Collection<DexProgramClass>> apply(Collection<DexProgramClass> group) { + Map<T, Collection<DexProgramClass>> groups = new IdentityHashMap<>(); + for (DexProgramClass clazz : group) { + groups.computeIfAbsent(getMergeKey(clazz), ignore -> new LinkedList<>()).add(clazz); + } + removeTrivialGroups(groups.values()); + return groups.values(); + } + + public abstract T getMergeKey(DexProgramClass clazz); +}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/Policy.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/Policy.java index d8dea98..8d6fc72 100644 --- a/src/main/java/com/android/tools/r8/horizontalclassmerging/Policy.java +++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/Policy.java
@@ -8,4 +8,8 @@ * The super class of all horizontal class merging policies. Most classes will either implement * {@link SingleClassPolicy} or {@link MultiClassPolicy}. */ -public abstract class Policy {} +public abstract class Policy { + public boolean shouldSkipPolicy() { + return false; + } +}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/SimplePolicyExecutor.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/SimplePolicyExecutor.java index 322958f..f0ac605 100644 --- a/src/main/java/com/android/tools/r8/horizontalclassmerging/SimplePolicyExecutor.java +++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/SimplePolicyExecutor.java
@@ -54,6 +54,10 @@ } for (Policy policy : policies) { + if (policy.shouldSkipPolicy()) { + continue; + } + if (policy instanceof SingleClassPolicy) { linkedGroups = applySingleClassPolicy((SingleClassPolicy) policy, linkedGroups); } else if (policy instanceof MultiClassPolicy) {
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoServiceLoaders.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoServiceLoaders.java new file mode 100644 index 0000000..18be993 --- /dev/null +++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/NoServiceLoaders.java
@@ -0,0 +1,23 @@ +// Copyright (c) 2020, 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.horizontalclassmerging.policies; + +import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.graph.DexProgramClass; +import com.android.tools.r8.horizontalclassmerging.SingleClassPolicy; +import com.android.tools.r8.shaking.AppInfoWithLiveness; + +public class NoServiceLoaders extends SingleClassPolicy { + private final AppView<AppInfoWithLiveness> appView; + + public NoServiceLoaders(AppView<AppInfoWithLiveness> appView) { + this.appView = appView; + } + + @Override + public boolean canMerge(DexProgramClass program) { + return !appView.appServices().allServiceTypes().contains(program.getType()); + } +}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreventChangingVisibility.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreventChangingVisibility.java new file mode 100644 index 0000000..3c317eb --- /dev/null +++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/PreventChangingVisibility.java
@@ -0,0 +1,78 @@ +// Copyright (c) 2020, 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.horizontalclassmerging.policies; + +import com.android.tools.r8.graph.DexEncodedMethod; +import com.android.tools.r8.graph.DexMethod; +import com.android.tools.r8.graph.DexProgramClass; +import com.android.tools.r8.graph.MethodAccessFlags; +import com.android.tools.r8.horizontalclassmerging.MultiClassPolicy; +import com.android.tools.r8.utils.MethodSignatureEquivalence; +import com.google.common.base.Equivalence.Wrapper; +import com.google.common.collect.Iterables; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; + +public class PreventChangingVisibility extends MultiClassPolicy { + public PreventChangingVisibility() {} + + public static class TargetGroup { + private final Collection<DexProgramClass> group = new LinkedList<>(); + private final Map<Wrapper<DexMethod>, MethodAccessFlags> methodMap = new HashMap<>(); + + public Collection<DexProgramClass> getGroup() { + return group; + } + + public boolean tryAdd(DexProgramClass clazz) { + Map<Wrapper<DexMethod>, MethodAccessFlags> newMethods = new HashMap<>(); + for (DexEncodedMethod method : clazz.methods()) { + Wrapper<DexMethod> methodSignature = + MethodSignatureEquivalence.get().wrap(method.getReference()); + MethodAccessFlags flags = methodMap.get(methodSignature); + + if (flags == null) { + newMethods.put(methodSignature, method.getAccessFlags()); + } else { + if (!flags.isSameVisibility(method.getAccessFlags())) { + return false; + } + } + } + + methodMap.putAll(newMethods); + group.add(clazz); + return true; + } + } + + @Override + public Collection<Collection<DexProgramClass>> apply(Collection<DexProgramClass> group) { + List<TargetGroup> groups = new ArrayList<>(); + + for (DexProgramClass clazz : group) { + boolean added = Iterables.any(groups, targetGroup -> targetGroup.tryAdd(clazz)); + if (!added) { + TargetGroup newGroup = new TargetGroup(); + added = newGroup.tryAdd(clazz); + assert added; + groups.add(newGroup); + } + } + + Collection<Collection<DexProgramClass>> newGroups = new ArrayList<>(); + for (TargetGroup newGroup : groups) { + if (!isTrivial(newGroup.getGroup())) { + newGroups.add(newGroup.getGroup()); + } + } + + return newGroups; + } +}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFeatureSplit.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFeatureSplit.java new file mode 100644 index 0000000..9e4dd2b --- /dev/null +++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/policies/SameFeatureSplit.java
@@ -0,0 +1,24 @@ +// Copyright (c) 2020, 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.horizontalclassmerging.policies; + +import com.android.tools.r8.FeatureSplit; +import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.graph.DexProgramClass; +import com.android.tools.r8.horizontalclassmerging.MultiClassSameReferencePolicy; +import com.android.tools.r8.shaking.AppInfoWithLiveness; + +public class SameFeatureSplit extends MultiClassSameReferencePolicy<FeatureSplit> { + private final AppView<AppInfoWithLiveness> appView; + + public SameFeatureSplit(AppView<AppInfoWithLiveness> appView) { + this.appView = appView; + } + + @Override + public FeatureSplit getMergeKey(DexProgramClass clazz) { + return appView.appInfo().getClassToFeatureSplitMap().getFeatureSplit(clazz); + } +}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/TypeChecker.java b/src/main/java/com/android/tools/r8/ir/analysis/TypeChecker.java index 8c8eeb9..5926ecc 100644 --- a/src/main/java/com/android/tools/r8/ir/analysis/TypeChecker.java +++ b/src/main/java/com/android/tools/r8/ir/analysis/TypeChecker.java
@@ -26,14 +26,17 @@ * that it is dead. * * <p>Pruning code that does not verify is necessary in order to be able to assert that the types - * are sound using {@link Instruction#verifyTypes(AppView)}. + * are sound using {@link Instruction#verifyTypes(AppView, VerifyTypesHelper)}. */ public class TypeChecker { private final AppView<? extends AppInfoWithClassHierarchy> appView; + private final VerifyTypesHelper verifyTypesHelper; - public TypeChecker(AppView<? extends AppInfoWithClassHierarchy> appView) { + public TypeChecker( + AppView<? extends AppInfoWithClassHierarchy> appView, VerifyTypesHelper verifyTypesHelper) { this.appView = appView; + this.verifyTypesHelper = verifyTypesHelper; } public boolean check(IRCode code) { @@ -70,7 +73,7 @@ TypeElement valueType = instruction.returnValue().getType(); TypeElement returnType = TypeElement.fromDexType(method.method.proto.returnType, Nullability.maybeNull(), appView); - if (isSubtypeOf(valueType, returnType)) { + if (verifyTypesHelper.isAssignable(valueType, returnType)) { return true; } @@ -93,7 +96,7 @@ TypeElement valueType = instruction.value().getType(); TypeElement fieldType = TypeElement.fromDexType(instruction.getField().type, valueType.nullability(), appView); - if (isSubtypeOf(valueType, fieldType)) { + if (verifyTypesHelper.isAssignable(valueType, fieldType)) { return true; } @@ -112,12 +115,6 @@ TypeElement throwableType = TypeElement.fromDexType( appView.dexItemFactory().throwableType, valueType.nullability(), appView); - return isSubtypeOf(valueType, throwableType); - } - - private boolean isSubtypeOf(TypeElement expectedSubtype, TypeElement expectedSupertype) { - return (expectedSubtype.isNullType() && expectedSupertype.isReferenceType()) - || expectedSubtype.lessThanOrEqual(expectedSupertype, appView) - || expectedSubtype.isBasedOnMissingClass(appView); + return verifyTypesHelper.isAssignable(valueType, throwableType); } }
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/VerifyTypesHelper.java b/src/main/java/com/android/tools/r8/ir/analysis/VerifyTypesHelper.java new file mode 100644 index 0000000..d8c1bb1 --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/analysis/VerifyTypesHelper.java
@@ -0,0 +1,54 @@ +// Copyright (c) 2020, 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.analysis; + +import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.ir.analysis.type.TypeElement; + +public class VerifyTypesHelper { + + private final AppView<?> appView; + + private VerifyTypesHelper(AppView<?> appView) { + this.appView = appView; + } + + public static VerifyTypesHelper create(AppView<?> appView) { + return new VerifyTypesHelper(appView); + } + + public boolean isAssignable(TypeElement one, TypeElement other) { + if (one.isPrimitiveType() != other.isPrimitiveType()) { + return false; + } + if (one.isPrimitiveType()) { + assert other.isPrimitiveType(); + return one.equals(other); + } + assert one.isReferenceType() && other.isReferenceType(); + if (one.isNullType() && other.isReferenceType()) { + return true; + } + if (one.isArrayType() != other.isArrayType()) { + return one.isArrayType() + && other.asClassType().getClassType() == appView.dexItemFactory().objectType; + } + if (one.isArrayType()) { + assert other.isArrayType(); + return isAssignable(one.asArrayType().getMemberType(), other.asArrayType().getMemberType()); + } + assert one.isClassType() && other.isClassType(); + if (appView.enableWholeProgramOptimizations()) { + return one.lessThanOrEqual(other, appView) + || one.isBasedOnMissingClass(appView.withClassHierarchy()); + } else { + // If we do not have whole program knowledge, we can only do the most basic check. + if (one.asClassType().getClassType() == appView.dexItemFactory().objectType) { + return other.asClassType().getClassType() == appView.dexItemFactory().objectType; + } + } + return true; + } +}
diff --git a/src/main/java/com/android/tools/r8/ir/code/Assume.java b/src/main/java/com/android/tools/r8/ir/code/Assume.java index 7aed1d9..4674e14 100644 --- a/src/main/java/com/android/tools/r8/ir/code/Assume.java +++ b/src/main/java/com/android/tools/r8/ir/code/Assume.java
@@ -9,6 +9,7 @@ import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.ProgramMethod; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.type.ClassTypeElement; import com.android.tools.r8.ir.analysis.type.TypeElement; import com.android.tools.r8.ir.conversion.CfBuilder; @@ -239,8 +240,8 @@ } @Override - public boolean verifyTypes(AppView<?> appView) { - assert super.verifyTypes(appView); + public boolean verifyTypes(AppView<?> appView, VerifyTypesHelper verifyTypesHelper) { + assert super.verifyTypes(appView, verifyTypesHelper); TypeElement inType = src().getType(); assert inType.isReferenceType() : inType;
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java index fa3c34c..c4823bc 100644 --- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java +++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -12,6 +12,7 @@ import com.android.tools.r8.graph.DexItemFactory; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.GraphLens; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.type.Nullability; import com.android.tools.r8.ir.analysis.type.TypeElement; import com.android.tools.r8.ir.code.Phi.RegisterReadType; @@ -81,8 +82,9 @@ return true; } - public boolean verifyTypes(AppView<?> appView) { - assert instructions.stream().allMatch(instruction -> instruction.verifyTypes(appView)); + public boolean verifyTypes(AppView<?> appView, VerifyTypesHelper verifyTypesHelper) { + assert instructions.stream() + .allMatch(instruction -> instruction.verifyTypes(appView, verifyTypesHelper)); return true; }
diff --git a/src/main/java/com/android/tools/r8/ir/code/CheckCast.java b/src/main/java/com/android/tools/r8/ir/code/CheckCast.java index 7085e80..6722cfb 100644 --- a/src/main/java/com/android/tools/r8/ir/code/CheckCast.java +++ b/src/main/java/com/android/tools/r8/ir/code/CheckCast.java
@@ -17,6 +17,7 @@ import com.android.tools.r8.graph.DexClass; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.ProgramMethod; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.type.TypeElement; import com.android.tools.r8.ir.conversion.CfBuilder; import com.android.tools.r8.ir.conversion.DexBuilder; @@ -165,8 +166,8 @@ } @Override - public boolean verifyTypes(AppView<?> appView) { - assert super.verifyTypes(appView); + public boolean verifyTypes(AppView<?> appView, VerifyTypesHelper verifyTypesHelper) { + assert super.verifyTypes(appView, verifyTypesHelper); TypeElement inType = object().getType();
diff --git a/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java b/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java index dc66e3c..763d520 100644 --- a/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java +++ b/src/main/java/com/android/tools/r8/ir/code/ConstNumber.java
@@ -19,6 +19,7 @@ import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.ProgramMethod; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.constant.Bottom; import com.android.tools.r8.ir.analysis.constant.ConstLatticeElement; import com.android.tools.r8.ir.analysis.constant.LatticeElement; @@ -323,8 +324,8 @@ } @Override - public boolean verifyTypes(AppView<?> appView) { - assert super.verifyTypes(appView); + public boolean verifyTypes(AppView<?> appView, VerifyTypesHelper verifyTypesHelper) { + assert super.verifyTypes(appView, verifyTypesHelper); assert !isZero() || getOutType().isPrimitiveType() || getOutType().isNullType(); return true; }
diff --git a/src/main/java/com/android/tools/r8/ir/code/ConstString.java b/src/main/java/com/android/tools/r8/ir/code/ConstString.java index 0a76de9..93b95e7 100644 --- a/src/main/java/com/android/tools/r8/ir/code/ConstString.java +++ b/src/main/java/com/android/tools/r8/ir/code/ConstString.java
@@ -13,6 +13,7 @@ import com.android.tools.r8.graph.DexString; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.ProgramMethod; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.type.TypeElement; import com.android.tools.r8.ir.analysis.value.AbstractValue; import com.android.tools.r8.ir.analysis.value.UnknownValue; @@ -177,8 +178,8 @@ } @Override - public boolean verifyTypes(AppView<?> appView) { - assert super.verifyTypes(appView); + public boolean verifyTypes(AppView<?> appView, VerifyTypesHelper verifyTypesHelper) { + assert super.verifyTypes(appView, verifyTypesHelper); TypeElement expectedType = TypeElement.stringClassType(appView, definitelyNotNull()); assert getOutType().equals(expectedType); return true;
diff --git a/src/main/java/com/android/tools/r8/ir/code/DebugLocalWrite.java b/src/main/java/com/android/tools/r8/ir/code/DebugLocalWrite.java index f8a36db..6817315 100644 --- a/src/main/java/com/android/tools/r8/ir/code/DebugLocalWrite.java +++ b/src/main/java/com/android/tools/r8/ir/code/DebugLocalWrite.java
@@ -8,6 +8,7 @@ import com.android.tools.r8.cf.code.CfStore; import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexType; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.conversion.CfBuilder; /** @@ -77,9 +78,9 @@ } @Override - public boolean verifyTypes(AppView<?> appView) { - super.verifyTypes(appView); - assert src().getType().lessThanOrEqual(getOutType(), appView); + public boolean verifyTypes(AppView<?> appView, VerifyTypesHelper verifyTypesHelper) { + super.verifyTypes(appView, verifyTypesHelper); + assert verifyTypesHelper.isAssignable(src().getType(), getOutType()); return true; } }
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java index 476135f..90e6097 100644 --- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java +++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -15,6 +15,7 @@ import com.android.tools.r8.graph.ProgramMethod; import com.android.tools.r8.graph.classmerging.VerticallyMergedClasses; import com.android.tools.r8.ir.analysis.TypeChecker; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.type.ClassTypeElement; import com.android.tools.r8.ir.analysis.type.Nullability; import com.android.tools.r8.ir.analysis.type.TypeElement; @@ -616,11 +617,12 @@ public boolean verifyTypes(AppView<?> appView) { // We can only type check the program if we have subtyping information. Therefore, we do not // require that the program type checks in D8. + VerifyTypesHelper verifyTypesHelper = VerifyTypesHelper.create(appView); if (appView.enableWholeProgramOptimizations()) { assert validAssumeInstructions(appView); - assert new TypeChecker(appView.withLiveness()).check(this); + assert new TypeChecker(appView.withLiveness(), verifyTypesHelper).check(this); } - assert blocks.stream().allMatch(block -> block.verifyTypes(appView)); + assert blocks.stream().allMatch(block -> block.verifyTypes(appView, verifyTypesHelper)); return true; }
diff --git a/src/main/java/com/android/tools/r8/ir/code/Instruction.java b/src/main/java/com/android/tools/r8/ir/code/Instruction.java index 1334a46..6a42c80 100644 --- a/src/main/java/com/android/tools/r8/ir/code/Instruction.java +++ b/src/main/java/com/android/tools/r8/ir/code/Instruction.java
@@ -13,6 +13,7 @@ import com.android.tools.r8.graph.ProgramMethod; import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis.AnalysisAssumption; import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis.Query; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.constant.Bottom; import com.android.tools.r8.ir.analysis.constant.ConstRangeLatticeElement; import com.android.tools.r8.ir.analysis.constant.LatticeElement; @@ -1393,8 +1394,7 @@ "Implement type lattice evaluation for: " + getInstructionName()); } - public boolean verifyTypes(AppView<?> appView) { - // TODO(b/72693244): for instructions with invariant out type, we can verify type directly here. + public boolean verifyTypes(AppView<?> appView, VerifyTypesHelper verifyTypesHelper) { if (outValue != null) { TypeElement outTypeElement = outValue.getType(); if (outTypeElement.isArrayType()) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java index 79f7152..b006d6a 100644 --- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java +++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
@@ -14,6 +14,7 @@ import com.android.tools.r8.graph.ProgramMethod; import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult; import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.type.ClassTypeElement; import com.android.tools.r8.ir.analysis.type.TypeAnalysis; import com.android.tools.r8.ir.analysis.type.TypeElement; @@ -115,8 +116,8 @@ } @Override - public boolean verifyTypes(AppView<?> appView) { - assert super.verifyTypes(appView); + public boolean verifyTypes(AppView<?> appView, VerifyTypesHelper verifyTypesHelper) { + assert super.verifyTypes(appView, verifyTypesHelper); Value receiver = getReceiver(); TypeElement receiverType = receiver.getType();
diff --git a/src/main/java/com/android/tools/r8/ir/code/Move.java b/src/main/java/com/android/tools/r8/ir/code/Move.java index f531071..58e1507 100644 --- a/src/main/java/com/android/tools/r8/ir/code/Move.java +++ b/src/main/java/com/android/tools/r8/ir/code/Move.java
@@ -9,6 +9,7 @@ import com.android.tools.r8.errors.Unreachable; import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.ProgramMethod; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.type.TypeElement; import com.android.tools.r8.ir.conversion.CfBuilder; import com.android.tools.r8.ir.conversion.DexBuilder; @@ -124,8 +125,8 @@ } @Override - public boolean verifyTypes(AppView<?> appView) { - super.verifyTypes(appView); + public boolean verifyTypes(AppView<?> appView, VerifyTypesHelper verifyTypesHelper) { + super.verifyTypes(appView, verifyTypesHelper); // DebugLocalWrite defines it's own verification of types but should be allowed to call super. if (!this.isDebugLocalWrite()) { assert src().getType().equals(getOutType());
diff --git a/src/main/java/com/android/tools/r8/ir/code/NewInstance.java b/src/main/java/com/android/tools/r8/ir/code/NewInstance.java index ff7a054..0c6f609 100644 --- a/src/main/java/com/android/tools/r8/ir/code/NewInstance.java +++ b/src/main/java/com/android/tools/r8/ir/code/NewInstance.java
@@ -18,6 +18,7 @@ import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis; import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis.AnalysisAssumption; import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis.Query; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.type.Nullability; import com.android.tools.r8.ir.analysis.type.TypeElement; import com.android.tools.r8.ir.conversion.CfBuilder; @@ -225,7 +226,7 @@ } @Override - public boolean verifyTypes(AppView<?> appView) { + public boolean verifyTypes(AppView<?> appView, VerifyTypesHelper verifyTypesHelper) { TypeElement type = getOutType(); assert type.isClassType(); assert type.asClassType().getClassType() == clazz || appView.options().testing.allowTypeErrors;
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java index 47db073..6595192 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -172,7 +172,7 @@ if (appView.options().testing.allowInvokeErrors) { return true; } - for (CfInstruction instruction : code.instructions) { + for (CfInstruction instruction : code.getInstructions()) { if (instruction instanceof CfInvoke) { CfInvoke invoke = (CfInvoke) instruction; if (invoke.getMethod().holder.isClassType()) {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java index f7e8a90..36c4111 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
@@ -779,7 +779,7 @@ return canonicalPositions.getExceptionalExitPosition( appView.options().debug, () -> - code.instructions.stream() + code.getInstructions().stream() .filter(insn -> insn instanceof CfPosition) .map(insn -> ((CfPosition) insn).getPosition()) .collect(Collectors.toList()),
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java index cd9bcd5..facf351 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
@@ -414,8 +414,8 @@ Try tryRange, DexItemFactory factory, BiConsumer<DexType, Integer> fn) { TryHandler handler = code.handlers[tryRange.handlerIndex]; for (TypeAddrPair pair : handler.pairs) { - fn.accept(pair.type, pair.addr); - if (pair.type == factory.throwableType) { + fn.accept(pair.getType(), pair.addr); + if (pair.getType() == factory.throwableType) { return; } }
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java index 32da612..100131c 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -9,6 +9,7 @@ import com.android.tools.r8.errors.CompilationError; import com.android.tools.r8.errors.Unreachable; import com.android.tools.r8.graph.AppInfo; +import com.android.tools.r8.graph.AppInfoWithClassHierarchy; import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.CfCode; import com.android.tools.r8.graph.Code; @@ -24,6 +25,7 @@ import com.android.tools.r8.graph.GraphLens; import com.android.tools.r8.graph.ProgramMethod; import com.android.tools.r8.ir.analysis.TypeChecker; +import com.android.tools.r8.ir.analysis.VerifyTypesHelper; import com.android.tools.r8.ir.analysis.constant.SparseConditionalConstantPropagation; import com.android.tools.r8.ir.analysis.fieldaccess.FieldAccessAnalysis; import com.android.tools.r8.ir.analysis.fieldaccess.TrivialFieldAccessReprocessor; @@ -44,6 +46,7 @@ import com.android.tools.r8.ir.desugar.D8NestBasedAccessDesugaring; import com.android.tools.r8.ir.desugar.DesugaredLibraryAPIConverter; import com.android.tools.r8.ir.desugar.DesugaredLibraryAPIConverter.Mode; +import com.android.tools.r8.ir.desugar.DesugaredLibraryConfiguration; import com.android.tools.r8.ir.desugar.DesugaredLibraryRetargeter; import com.android.tools.r8.ir.desugar.InterfaceMethodRewriter; import com.android.tools.r8.ir.desugar.InterfaceMethodRewriter.Flavor; @@ -310,7 +313,7 @@ } this.devirtualizer = options.enableDevirtualization ? new Devirtualizer(appViewWithLiveness) : null; - this.typeChecker = new TypeChecker(appViewWithLiveness); + this.typeChecker = new TypeChecker(appViewWithLiveness, VerifyTypesHelper.create(appView)); this.d8NestBasedAccessDesugaring = null; this.serviceLoaderRewriter = options.enableServiceLoaderRewriting @@ -523,6 +526,9 @@ return; } checkPrefixMerging(method); + if (!needsIRConversion(definition.getCode(), method)) { + return; + } if (options.isGeneratingClassFiles() || !(options.passthroughDexCode && definition.getCode().isDexCode())) { // We do not process in call graph order, so anything could be a leaf. @@ -536,6 +542,35 @@ } } + private boolean needsIRConversion(Code code, ProgramMethod method) { + if (options.isDesugaredLibraryCompilation()) { + // TODO(b/169035524): Create method for evaluating if a code object needs rewriting for + // library desugaring. + return true; + } + if (options.desugaredLibraryConfiguration != DesugaredLibraryConfiguration.empty()) { + // TODO(b/169035524): Create method for evaluating if a code object needs rewriting for + // library desugaring. + return true; + } + + if (!options.cfToCfDesugar) { + return true; + } + if (method.getHolder().isInANest()) { + return true; + } + + NeedsIRDesugarUseRegistry useRegistry = + new NeedsIRDesugarUseRegistry(appView, backportedMethodRewriter); + method.registerCodeReferences(useRegistry); + + if (useRegistry.needsDesugaring()) { + return true; + } + return false; + } + private void checkPrefixMerging(ProgramMethod method) { if (!appView.options().enableNeverMergePrefixes) { return; @@ -1066,6 +1101,14 @@ method.toSourceString(), logCode(options, method.getDefinition())); } + boolean didDesugar = desugar(method); + if (Log.ENABLED && didDesugar) { + Log.debug( + getClass(), + "Desugared code for %s:\n%s", + method.toSourceString(), + logCode(options, method.getDefinition())); + } if (options.testing.hookInIrConversion != null) { options.testing.hookInIrConversion.run(); } @@ -1081,6 +1124,21 @@ return optimize(code, feedback, methodProcessor, methodProcessingId); } + private boolean desugar(ProgramMethod method) { + if (options.desugarState != DesugarState.ON) { + return false; + } + if (!method.getDefinition().getCode().isCfCode()) { + return false; + } + boolean didDesugar = false; + if (lambdaRewriter != null) { + AppInfoWithClassHierarchy appInfo = appView.appInfoForDesugaring(); + didDesugar |= lambdaRewriter.desugarLambdas(method, appInfo) > 0; + } + return didDesugar; + } + // TODO(b/140766440): Convert all sub steps an implementer of CodeOptimization private Timing optimize( IRCode code, @@ -1129,13 +1187,6 @@ || !appView.enableWholeProgramOptimizations() || !appView.appInfo().withLiveness().neverReprocess.contains(method.method); - if (!method.isProcessed() && lambdaRewriter != null) { - timing.begin("Desugar lambdas"); - lambdaRewriter.desugarLambdas(code); - timing.end(); - assert code.isConsistentSSA(); - } - if (lambdaMerger != null) { timing.begin("Merge lambdas"); lambdaMerger.rewriteCode(code.context(), code, inliner, methodProcessor);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/NeedsIRDesugarUseRegistry.java b/src/main/java/com/android/tools/r8/ir/conversion/NeedsIRDesugarUseRegistry.java new file mode 100644 index 0000000..9d88a1f --- /dev/null +++ b/src/main/java/com/android/tools/r8/ir/conversion/NeedsIRDesugarUseRegistry.java
@@ -0,0 +1,97 @@ +// Copyright (c) 2020, 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.conversion; + +import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.graph.DexCallSite; +import com.android.tools.r8.graph.DexField; +import com.android.tools.r8.graph.DexMethod; +import com.android.tools.r8.graph.DexType; +import com.android.tools.r8.graph.UseRegistry; +import com.android.tools.r8.ir.desugar.BackportedMethodRewriter; +import com.android.tools.r8.ir.desugar.TwrCloseResourceRewriter; + +class NeedsIRDesugarUseRegistry extends UseRegistry { + + private boolean needsDesugarging = false; + private final AppView appView; + private final BackportedMethodRewriter backportedMethodRewriter; + + public NeedsIRDesugarUseRegistry( + AppView appView, BackportedMethodRewriter backportedMethodRewriter) { + super(appView.dexItemFactory()); + this.appView = appView; + this.backportedMethodRewriter = backportedMethodRewriter; + } + + public boolean needsDesugaring() { + return needsDesugarging; + } + + @Override + public void registerInitClass(DexType type) {} + + @Override + public void registerInvokeVirtual(DexMethod method) { + if (backportedMethodRewriter.needsDesugaring(method)) { + needsDesugarging = true; + } + } + + @Override + public void registerInvokeDirect(DexMethod method) {} + + @Override + public void registerInvokeStatic(DexMethod method) { + if (TwrCloseResourceRewriter.isSynthesizedCloseResourceMethod(method, appView)) { + needsDesugarging = true; + } + + if (backportedMethodRewriter.needsDesugaring(method)) { + needsDesugarging = true; + } + } + + @Override + public void registerInvokeInterface(DexMethod method) {} + + @Override + public void registerInvokeStatic(DexMethod method, boolean itf) { + if (itf) { + needsDesugarging = true; + } + registerInvokeStatic(method); + } + + @Override + public void registerCallSite(DexCallSite callSite) { + super.registerCallSite(callSite); + needsDesugarging = true; + } + + @Override + public void registerInvokeSuper(DexMethod method) {} + + @Override + public void registerInstanceFieldRead(DexField field) {} + + @Override + public void registerInstanceFieldWrite(DexField field) {} + + @Override + public void registerNewInstance(DexType type) {} + + @Override + public void registerStaticFieldRead(DexField field) {} + + @Override + public void registerStaticFieldWrite(DexField field) {} + + @Override + public void registerTypeReference(DexType type) {} + + @Override + public void registerInstanceOf(DexType type) {} +}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java index 8cc0ffa..da1a799 100644 --- a/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java +++ b/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
@@ -74,12 +74,15 @@ // by the Android Platform build (which normally use an API level of 10000) there will be // no rewriting of backported methods. See b/147480264. this.enabled = - (appView.options().desugarState == DesugarState.ON - || appView.options().desugarState == DesugarState.ONLY_BACKPORT_STATICS) + appView.options().desugarState == DesugarState.ON && !this.rewritableMethods.isEmpty() && appView.options().minApiLevel <= AndroidApiLevel.LATEST.getLevel(); } + public boolean needsDesugaring(DexMethod method) { + return rewritableMethods.getProvider(method) != null; + } + public static List<DexMethod> generateListOfBackportedMethods( AndroidApp androidApp, InternalOptions options, ExecutorService executor) throws IOException { List<DexMethod> methods = new ArrayList<>(); @@ -121,11 +124,6 @@ } InvokeMethod invoke = instruction.asInvokeMethod(); - if (appView.options().desugarState == DesugarState.ONLY_BACKPORT_STATICS - && !invoke.isInvokeStatic()) { - continue; - } - DexMethod invokedMethod = invoke.getInvokedMethod(); MethodProvider provider = getMethodProviderOrNull(invokedMethod); if (provider != null) {
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java index 494831e..542c5d7 100644 --- a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java +++ b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java
@@ -36,6 +36,20 @@ public class DesugaredLibraryConfiguration { public static final String FALL_BACK_SYNTHESIZED_CLASSES_PACKAGE_PREFIX = "j$/"; + public static final DesugaredLibraryConfiguration EMPTY_DESUGARED_LIBRARY_CONFIGURATION = + new DesugaredLibraryConfiguration( + AndroidApiLevel.B, + false, + FALL_BACK_SYNTHESIZED_CLASSES_PACKAGE_PREFIX, + null, + ImmutableMap.of(), + ImmutableMap.of(), + ImmutableMap.of(), + ImmutableMap.of(), + ImmutableMap.of(), + ImmutableSet.of(), + ImmutableList.of(), + ImmutableList.of()); // TODO(b/158632510): should use DexString, DexType, DexMethod or so on when possible. private final AndroidApiLevel requiredCompilationAPILevel; @@ -73,19 +87,7 @@ } public static DesugaredLibraryConfiguration empty() { - return new DesugaredLibraryConfiguration( - AndroidApiLevel.B, - false, - FALL_BACK_SYNTHESIZED_CLASSES_PACKAGE_PREFIX, - null, - ImmutableMap.of(), - ImmutableMap.of(), - ImmutableMap.of(), - ImmutableMap.of(), - ImmutableMap.of(), - ImmutableSet.of(), - ImmutableList.of(), - ImmutableList.of()); + return EMPTY_DESUGARED_LIBRARY_CONFIGURATION; } private DesugaredLibraryConfiguration(
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java index 2faa07a..7bb8d44 100644 --- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java +++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaRewriter.java
@@ -4,9 +4,21 @@ package com.android.tools.r8.ir.desugar; +import com.android.tools.r8.cf.code.CfFieldInstruction; +import com.android.tools.r8.cf.code.CfInstruction; +import com.android.tools.r8.cf.code.CfInvoke; +import com.android.tools.r8.cf.code.CfInvokeDynamic; +import com.android.tools.r8.cf.code.CfLoad; +import com.android.tools.r8.cf.code.CfNew; +import com.android.tools.r8.cf.code.CfStackInstruction; +import com.android.tools.r8.cf.code.CfStackInstruction.Opcode; +import com.android.tools.r8.cf.code.CfStore; +import com.android.tools.r8.graph.AppInfoWithClassHierarchy; import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.graph.CfCode; import com.android.tools.r8.graph.DexApplication.Builder; import com.android.tools.r8.graph.DexCallSite; +import com.android.tools.r8.graph.DexEncodedMethod; import com.android.tools.r8.graph.DexField; import com.android.tools.r8.graph.DexItemFactory; import com.android.tools.r8.graph.DexMethod; @@ -18,23 +30,22 @@ import com.android.tools.r8.graph.GraphLens.NestedGraphLens; import com.android.tools.r8.graph.ProgramMethod; import com.android.tools.r8.ir.analysis.type.Nullability; -import com.android.tools.r8.ir.analysis.type.TypeAnalysis; import com.android.tools.r8.ir.analysis.type.TypeElement; import com.android.tools.r8.ir.code.BasicBlock; import com.android.tools.r8.ir.code.IRCode; -import com.android.tools.r8.ir.code.Instruction; import com.android.tools.r8.ir.code.InstructionListIterator; import com.android.tools.r8.ir.code.InvokeCustom; import com.android.tools.r8.ir.code.InvokeDirect; import com.android.tools.r8.ir.code.NewInstance; import com.android.tools.r8.ir.code.StaticGet; import com.android.tools.r8.ir.code.Value; +import com.android.tools.r8.ir.code.ValueType; import com.android.tools.r8.ir.conversion.IRConverter; import com.android.tools.r8.utils.DescriptorUtils; import com.android.tools.r8.utils.collections.SortedProgramMethodSet; +import com.google.common.base.Suppliers; import com.google.common.collect.BiMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.Collection; import java.util.IdentityHashMap; @@ -44,6 +55,9 @@ import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; +import java.util.function.Function; +import java.util.function.Supplier; +import org.objectweb.asm.Opcodes; /** * Lambda desugaring rewriter. @@ -105,41 +119,75 @@ * * <p>NOTE: this method can be called concurrently for several different methods. */ - public void desugarLambdas(IRCode code) { - Set<Value> affectedValues = Sets.newIdentityHashSet(); - ProgramMethod context = code.context(); - ListIterator<BasicBlock> blocks = code.listIterator(); - while (blocks.hasNext()) { - BasicBlock block = blocks.next(); - InstructionListIterator instructions = block.listIterator(code); - while (instructions.hasNext()) { - Instruction instruction = instructions.next(); - if (instruction.isInvokeCustom()) { - InvokeCustom invoke = instruction.asInvokeCustom(); - LambdaDescriptor descriptor = inferLambdaDescriptor(invoke.getCallSite(), context); - if (descriptor == LambdaDescriptor.MATCH_FAILED) { - continue; + public int desugarLambdas(ProgramMethod method, AppInfoWithClassHierarchy appInfo) { + return desugarLambdas( + method.getDefinition(), + callsite -> { + LambdaDescriptor descriptor = LambdaDescriptor.tryInfer(callsite, appInfo, method); + if (descriptor == null) { + return null; } + return getOrCreateLambdaClass(descriptor, method); + }); + } - // We have a descriptor, get the lambda class. In D8, we synthesize the lambda classes - // during IR processing, and therefore we may need to create it now. - LambdaClass lambdaClass = - appView.enableWholeProgramOptimizations() - ? getKnownLambdaClass(descriptor, context) - : getOrCreateLambdaClass(descriptor, context); - assert lambdaClass != null; - - // We rely on patch performing its work in a way which - // keeps both `instructions` and `blocks` iterators in - // valid state so that we can continue iteration. - patchInstruction(invoke, lambdaClass, code, blocks, instructions, affectedValues); + // Same as above, but where lambdas are always known to exist for the call sites. + public static int desugarLambdas( + DexEncodedMethod method, Function<DexCallSite, LambdaClass> callSites) { + CfCode code = method.getCode().asCfCode(); + List<CfInstruction> instructions = code.getInstructions(); + Supplier<List<CfInstruction>> lazyNewInstructions = + Suppliers.memoize(() -> new ArrayList<>(instructions)); + int replaced = 0; + int maxTemp = 0; + int newInstructionsDelta = 0; + for (int i = 0; i < instructions.size(); i++) { + CfInstruction instruction = instructions.get(i); + if (instruction instanceof CfInvokeDynamic) { + LambdaClass lambdaClass = callSites.apply(((CfInvokeDynamic) instruction).getCallSite()); + if (lambdaClass == null) { + continue; } + int newInstructionsIndex = i + newInstructionsDelta; + if (lambdaClass.isStateless()) { + CfFieldInstruction getStaticLambdaInstance = + new CfFieldInstruction( + Opcodes.GETSTATIC, lambdaClass.lambdaField, lambdaClass.lambdaField); + lazyNewInstructions.get().set(newInstructionsIndex, getStaticLambdaInstance); + } else { + List<CfInstruction> replacement = new ArrayList<>(); + int arguments = lambdaClass.descriptor.captures.size(); + int temp = code.getMaxLocals(); + for (int j = arguments - 1; j >= 0; j--) { + ValueType type = ValueType.fromDexType(lambdaClass.descriptor.captures.values[j]); + replacement.add(new CfStore(type, temp)); + temp += type.requiredRegisters(); + } + maxTemp = Math.max(temp, maxTemp); + replacement.add(new CfNew(lambdaClass.type)); + replacement.add(new CfStackInstruction(Opcode.Dup)); + for (int j = 0; j < arguments; j++) { + ValueType type = ValueType.fromDexType(lambdaClass.descriptor.captures.values[j]); + temp -= type.requiredRegisters(); + replacement.add(new CfLoad(type, temp)); + } + replacement.add(new CfInvoke(Opcodes.INVOKESPECIAL, lambdaClass.constructor, false)); + List<CfInstruction> newInstructions = lazyNewInstructions.get(); + newInstructions.remove(newInstructionsIndex); + newInstructions.addAll(newInstructionsIndex, replacement); + newInstructionsDelta += replacement.size() - 1; + } + ++replaced; } } - if (!affectedValues.isEmpty()) { - new TypeAnalysis(appView).narrowing(affectedValues); + if (maxTemp > 0) { + assert maxTemp > code.getMaxLocals(); + code.setMaxLocals(maxTemp); } - assert code.isConsistentSSA(); + if (replaced > 0) { + code.setInstructions(lazyNewInstructions.get()); + } + return replaced; } /** Remove lambda deserialization methods. */
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java index d4adf44..fbe8514 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxer.java
@@ -330,7 +330,8 @@ } for (Phi phi : value.uniquePhiUsers()) { for (Value operand : phi.getOperands()) { - if (getEnumUnboxingCandidateOrNull(operand.getType()) != enumClass) { + if (!operand.getType().isNullType() + && getEnumUnboxingCandidateOrNull(operand.getType()) != enumClass) { markEnumAsUnboxable(Reason.INVALID_PHI, enumClass); return Reason.INVALID_PHI; }
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java index c67c231..e09dab4 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java
@@ -31,6 +31,7 @@ import com.android.tools.r8.ir.analysis.type.TypeElement; import com.android.tools.r8.ir.analysis.value.AbstractValue; import com.android.tools.r8.ir.code.ArrayAccess; +import com.android.tools.r8.ir.code.BasicBlock; import com.android.tools.r8.ir.code.ConstNumber; import com.android.tools.r8.ir.code.IRCode; import com.android.tools.r8.ir.code.InstanceGet; @@ -55,6 +56,7 @@ import java.util.Collections; import java.util.IdentityHashMap; import java.util.List; +import java.util.ListIterator; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -148,182 +150,213 @@ assert code.isConsistentSSABeforeTypesAreCorrect(); Map<Instruction, DexType> convertedEnums = new IdentityHashMap<>(); Set<Phi> affectedPhis = Sets.newIdentityHashSet(); - InstructionListIterator iterator = code.instructionListIterator(); - while (iterator.hasNext()) { - Instruction instruction = iterator.next(); - // Rewrites specific enum methods, such as ordinal, into their corresponding enum unboxed - // counterpart. - if (instruction.isInvokeMethodWithReceiver()) { - InvokeMethodWithReceiver invokeMethod = instruction.asInvokeMethodWithReceiver(); - DexMethod invokedMethod = invokeMethod.getInvokedMethod(); - DexType enumType = getEnumTypeOrNull(invokeMethod.getReceiver(), convertedEnums); - if (enumType != null) { - if (invokedMethod == factory.enumMembers.ordinalMethod - || invokedMethod == factory.enumMembers.hashCode) { - replaceEnumInvoke( - iterator, invokeMethod, ordinalUtilityMethod, m -> synthesizeOrdinalMethod()); - continue; - } else if (invokedMethod == factory.enumMembers.equals) { - replaceEnumInvoke( - iterator, invokeMethod, equalsUtilityMethod, m -> synthesizeEqualsMethod()); - continue; - } else if (invokedMethod == factory.enumMembers.compareTo) { - replaceEnumInvoke( - iterator, invokeMethod, compareToUtilityMethod, m -> synthesizeCompareToMethod()); - continue; - } else if (invokedMethod == factory.enumMembers.nameMethod - || invokedMethod == factory.enumMembers.toString) { - DexMethod toStringMethod = - computeInstanceFieldUtilityMethod(enumType, factory.enumMembers.nameField); - iterator.replaceCurrentInstruction( - new InvokeStatic( - toStringMethod, invokeMethod.outValue(), invokeMethod.arguments())); - continue; - } else if (invokedMethod == factory.objectMembers.getClass) { - assert !invokeMethod.hasOutValue() || !invokeMethod.outValue().hasAnyUsers(); - replaceEnumInvoke( - iterator, invokeMethod, zeroCheckMethod, m -> synthesizeZeroCheckMethod()); - } - } - // TODO(b/147860220): rewrite also other enum methods. - } else if (instruction.isInvokeStatic()) { - InvokeStatic invokeStatic = instruction.asInvokeStatic(); - DexMethod invokedMethod = invokeStatic.getInvokedMethod(); - if (invokedMethod == factory.enumMembers.valueOf - && invokeStatic.inValues().get(0).isConstClass()) { - DexType enumType = - invokeStatic.inValues().get(0).getConstInstruction().asConstClass().getValue(); - if (enumsToUnbox.containsEnum(enumType)) { - DexMethod valueOfMethod = computeValueOfUtilityMethod(enumType); - Value outValue = invokeStatic.outValue(); - Value rewrittenOutValue = null; - if (outValue != null) { - rewrittenOutValue = code.createValue(TypeElement.getInt()); - affectedPhis.addAll(outValue.uniquePhiUsers()); + ListIterator<BasicBlock> blocks = code.listIterator(); + Value zeroConstValue = null; + while (blocks.hasNext()) { + BasicBlock block = blocks.next(); + zeroConstValue = fixNullsInBlockPhis(code, block, zeroConstValue); + InstructionListIterator iterator = block.listIterator(code); + while (iterator.hasNext()) { + Instruction instruction = iterator.next(); + // Rewrites specific enum methods, such as ordinal, into their corresponding enum unboxed + // counterpart. + if (instruction.isInvokeMethodWithReceiver()) { + InvokeMethodWithReceiver invokeMethod = instruction.asInvokeMethodWithReceiver(); + DexMethod invokedMethod = invokeMethod.getInvokedMethod(); + DexType enumType = getEnumTypeOrNull(invokeMethod.getReceiver(), convertedEnums); + if (enumType != null) { + if (invokedMethod == factory.enumMembers.ordinalMethod + || invokedMethod == factory.enumMembers.hashCode) { + replaceEnumInvoke( + iterator, invokeMethod, ordinalUtilityMethod, m -> synthesizeOrdinalMethod()); + continue; + } else if (invokedMethod == factory.enumMembers.equals) { + replaceEnumInvoke( + iterator, invokeMethod, equalsUtilityMethod, m -> synthesizeEqualsMethod()); + continue; + } else if (invokedMethod == factory.enumMembers.compareTo) { + replaceEnumInvoke( + iterator, invokeMethod, compareToUtilityMethod, m -> synthesizeCompareToMethod()); + continue; + } else if (invokedMethod == factory.enumMembers.nameMethod + || invokedMethod == factory.enumMembers.toString) { + DexMethod toStringMethod = + computeInstanceFieldUtilityMethod(enumType, factory.enumMembers.nameField); + iterator.replaceCurrentInstruction( + new InvokeStatic( + toStringMethod, invokeMethod.outValue(), invokeMethod.arguments())); + continue; + } else if (invokedMethod == factory.objectMembers.getClass) { + assert !invokeMethod.hasOutValue() || !invokeMethod.outValue().hasAnyUsers(); + replaceEnumInvoke( + iterator, invokeMethod, zeroCheckMethod, m -> synthesizeZeroCheckMethod()); } - InvokeStatic invoke = - new InvokeStatic( - valueOfMethod, - rewrittenOutValue, - Collections.singletonList(invokeStatic.inValues().get(1))); - iterator.replaceCurrentInstruction(invoke); - convertedEnums.put(invoke, enumType); - continue; } - } else if (invokedMethod == factory.javaLangSystemMethods.identityHashCode) { - assert invokeStatic.arguments().size() == 1; - Value argument = invokeStatic.getArgument(0); - DexType enumType = getEnumTypeOrNull(argument, convertedEnums); - if (enumType != null) { - invokeStatic.outValue().replaceUsers(argument); - iterator.removeOrReplaceByDebugLocalRead(); - } - } else if (invokedMethod == factory.stringMembers.valueOf) { - assert invokeStatic.arguments().size() == 1; - Value argument = invokeStatic.getArgument(0); - DexType enumType = getEnumTypeOrNull(argument, convertedEnums); - if (enumType != null) { - DexMethod stringValueOfMethod = computeStringValueOfUtilityMethod(enumType); - iterator.replaceCurrentInstruction( - new InvokeStatic( - stringValueOfMethod, invokeStatic.outValue(), invokeStatic.arguments())); - continue; - } - } else if (invokedMethod == factory.objectsMethods.requireNonNull) { - assert invokeStatic.arguments().size() == 1; - Value argument = invokeStatic.getArgument(0); - DexType enumType = getEnumTypeOrNull(argument, convertedEnums); - if (enumType != null) { - replaceEnumInvoke( - iterator, invokeStatic, zeroCheckMethod, m -> synthesizeZeroCheckMethod()); - } - } else if (invokedMethod == factory.objectsMethods.requireNonNullWithMessage) { - assert invokeStatic.arguments().size() == 2; - Value argument = invokeStatic.getArgument(0); - DexType enumType = getEnumTypeOrNull(argument, convertedEnums); - if (enumType != null) { - replaceEnumInvoke( - iterator, - invokeStatic, - zeroCheckMessageMethod, - m -> synthesizeZeroCheckMessageMethod()); + // TODO(b/147860220): rewrite also other enum methods. + } else if (instruction.isInvokeStatic()) { + InvokeStatic invokeStatic = instruction.asInvokeStatic(); + DexMethod invokedMethod = invokeStatic.getInvokedMethod(); + if (invokedMethod == factory.enumMembers.valueOf + && invokeStatic.inValues().get(0).isConstClass()) { + DexType enumType = + invokeStatic.inValues().get(0).getConstInstruction().asConstClass().getValue(); + if (enumsToUnbox.containsEnum(enumType)) { + DexMethod valueOfMethod = computeValueOfUtilityMethod(enumType); + Value outValue = invokeStatic.outValue(); + Value rewrittenOutValue = null; + if (outValue != null) { + rewrittenOutValue = code.createValue(TypeElement.getInt()); + affectedPhis.addAll(outValue.uniquePhiUsers()); + } + InvokeStatic invoke = + new InvokeStatic( + valueOfMethod, + rewrittenOutValue, + Collections.singletonList(invokeStatic.inValues().get(1))); + iterator.replaceCurrentInstruction(invoke); + convertedEnums.put(invoke, enumType); + continue; + } + } else if (invokedMethod == factory.javaLangSystemMethods.identityHashCode) { + assert invokeStatic.arguments().size() == 1; + Value argument = invokeStatic.getArgument(0); + DexType enumType = getEnumTypeOrNull(argument, convertedEnums); + if (enumType != null) { + invokeStatic.outValue().replaceUsers(argument); + iterator.removeOrReplaceByDebugLocalRead(); + } + } else if (invokedMethod == factory.stringMembers.valueOf) { + assert invokeStatic.arguments().size() == 1; + Value argument = invokeStatic.getArgument(0); + DexType enumType = getEnumTypeOrNull(argument, convertedEnums); + if (enumType != null) { + DexMethod stringValueOfMethod = computeStringValueOfUtilityMethod(enumType); + iterator.replaceCurrentInstruction( + new InvokeStatic( + stringValueOfMethod, invokeStatic.outValue(), invokeStatic.arguments())); + continue; + } + } else if (invokedMethod == factory.objectsMethods.requireNonNull) { + assert invokeStatic.arguments().size() == 1; + Value argument = invokeStatic.getArgument(0); + DexType enumType = getEnumTypeOrNull(argument, convertedEnums); + if (enumType != null) { + replaceEnumInvoke( + iterator, invokeStatic, zeroCheckMethod, m -> synthesizeZeroCheckMethod()); + } + } else if (invokedMethod == factory.objectsMethods.requireNonNullWithMessage) { + assert invokeStatic.arguments().size() == 2; + Value argument = invokeStatic.getArgument(0); + DexType enumType = getEnumTypeOrNull(argument, convertedEnums); + if (enumType != null) { + replaceEnumInvoke( + iterator, + invokeStatic, + zeroCheckMessageMethod, + m -> synthesizeZeroCheckMessageMethod()); + } } } - } - if (instruction.isStaticGet()) { - StaticGet staticGet = instruction.asStaticGet(); - DexType holder = staticGet.getField().holder; - if (enumsToUnbox.containsEnum(holder)) { - if (staticGet.outValue() == null) { - iterator.removeOrReplaceByDebugLocalRead(); - continue; + if (instruction.isStaticGet()) { + StaticGet staticGet = instruction.asStaticGet(); + DexType holder = staticGet.getField().holder; + if (enumsToUnbox.containsEnum(holder)) { + if (staticGet.outValue() == null) { + iterator.removeOrReplaceByDebugLocalRead(); + continue; + } + EnumValueInfoMap enumValueInfoMap = enumsToUnbox.getEnumValueInfoMap(holder); + assert enumValueInfoMap != null; + affectedPhis.addAll(staticGet.outValue().uniquePhiUsers()); + EnumValueInfo enumValueInfo = enumValueInfoMap.getEnumValueInfo(staticGet.getField()); + if (enumValueInfo == null && staticGet.getField().name == factory.enumValuesFieldName) { + utilityMethods.computeIfAbsent( + valuesUtilityMethod, m -> synthesizeValuesUtilityMethod()); + DexField fieldValues = createValuesField(holder); + utilityFields.computeIfAbsent(fieldValues, this::computeValuesEncodedField); + DexMethod methodValues = createValuesMethod(holder); + utilityMethods.computeIfAbsent( + methodValues, + m -> computeValuesEncodedMethod(m, fieldValues, enumValueInfoMap.size())); + Value rewrittenOutValue = + code.createValue( + ArrayTypeElement.create(TypeElement.getInt(), definitelyNotNull())); + InvokeStatic invoke = + new InvokeStatic(methodValues, rewrittenOutValue, ImmutableList.of()); + iterator.replaceCurrentInstruction(invoke); + convertedEnums.put(invoke, holder); + } else { + // Replace by ordinal + 1 for null check (null is 0). + assert enumValueInfo != null + : "Invalid read to " + staticGet.getField().name + ", error during enum analysis"; + ConstNumber intConstant = code.createIntConstant(enumValueInfo.convertToInt()); + iterator.replaceCurrentInstruction(intConstant); + convertedEnums.put(intConstant, holder); + } } - EnumValueInfoMap enumValueInfoMap = enumsToUnbox.getEnumValueInfoMap(holder); - assert enumValueInfoMap != null; - affectedPhis.addAll(staticGet.outValue().uniquePhiUsers()); - EnumValueInfo enumValueInfo = enumValueInfoMap.getEnumValueInfo(staticGet.getField()); - if (enumValueInfo == null && staticGet.getField().name == factory.enumValuesFieldName) { - utilityMethods.computeIfAbsent( - valuesUtilityMethod, m -> synthesizeValuesUtilityMethod()); - DexField fieldValues = createValuesField(holder); - utilityFields.computeIfAbsent(fieldValues, this::computeValuesEncodedField); - DexMethod methodValues = createValuesMethod(holder); - utilityMethods.computeIfAbsent( - methodValues, - m -> computeValuesEncodedMethod(m, fieldValues, enumValueInfoMap.size())); + } + + if (instruction.isInstanceGet()) { + InstanceGet instanceGet = instruction.asInstanceGet(); + DexType holder = instanceGet.getField().holder; + if (enumsToUnbox.containsEnum(holder)) { + DexMethod fieldMethod = computeInstanceFieldMethod(instanceGet.getField()); Value rewrittenOutValue = code.createValue( - ArrayTypeElement.create(TypeElement.getInt(), definitelyNotNull())); + TypeElement.fromDexType( + fieldMethod.proto.returnType, Nullability.maybeNull(), appView)); InvokeStatic invoke = - new InvokeStatic(methodValues, rewrittenOutValue, ImmutableList.of()); + new InvokeStatic( + fieldMethod, rewrittenOutValue, ImmutableList.of(instanceGet.object())); iterator.replaceCurrentInstruction(invoke); - convertedEnums.put(invoke, holder); - } else { - // Replace by ordinal + 1 for null check (null is 0). - assert enumValueInfo != null - : "Invalid read to " + staticGet.getField().name + ", error during enum analysis"; - ConstNumber intConstant = code.createIntConstant(enumValueInfo.convertToInt()); - iterator.replaceCurrentInstruction(intConstant); - convertedEnums.put(intConstant, holder); + if (enumsToUnbox.containsEnum(instanceGet.getField().type)) { + convertedEnums.put(invoke, instanceGet.getField().type); + } } } - } - if (instruction.isInstanceGet()) { - InstanceGet instanceGet = instruction.asInstanceGet(); - DexType holder = instanceGet.getField().holder; - if (enumsToUnbox.containsEnum(holder)) { - DexMethod fieldMethod = computeInstanceFieldMethod(instanceGet.getField()); - Value rewrittenOutValue = - code.createValue( - TypeElement.fromDexType( - fieldMethod.proto.returnType, Nullability.maybeNull(), appView)); - InvokeStatic invoke = - new InvokeStatic( - fieldMethod, rewrittenOutValue, ImmutableList.of(instanceGet.object())); - iterator.replaceCurrentInstruction(invoke); - if (enumsToUnbox.containsEnum(instanceGet.getField().type)) { - convertedEnums.put(invoke, instanceGet.getField().type); + // Rewrite array accesses from MyEnum[] (OBJECT) to int[] (INT). + if (instruction.isArrayAccess()) { + ArrayAccess arrayAccess = instruction.asArrayAccess(); + DexType enumType = getEnumTypeOrNull(arrayAccess); + if (enumType != null) { + instruction = arrayAccess.withMemberType(MemberType.INT); + iterator.replaceCurrentInstruction(instruction); + convertedEnums.put(instruction, enumType); } + assert validateArrayAccess(arrayAccess); } } - - // Rewrite array accesses from MyEnum[] (OBJECT) to int[] (INT). - if (instruction.isArrayAccess()) { - ArrayAccess arrayAccess = instruction.asArrayAccess(); - DexType enumType = getEnumTypeOrNull(arrayAccess); - if (enumType != null) { - instruction = arrayAccess.withMemberType(MemberType.INT); - iterator.replaceCurrentInstruction(instruction); - convertedEnums.put(instruction, enumType); - } - assert validateArrayAccess(arrayAccess); - } } assert code.isConsistentSSABeforeTypesAreCorrect(); return affectedPhis; } + private Value fixNullsInBlockPhis(IRCode code, BasicBlock block, Value zeroConstValue) { + for (Phi phi : block.getPhis()) { + if (getEnumTypeOrNull(phi.getType()) != null) { + for (int i = 0; i < phi.getOperands().size(); i++) { + Value operand = phi.getOperand(i); + if (operand.getType().isNullType()) { + if (zeroConstValue == null) { + zeroConstValue = insertConstZero(code); + } + phi.replaceOperandAt(i, zeroConstValue); + } + } + } + } + return zeroConstValue; + } + + private Value insertConstZero(IRCode code) { + InstructionListIterator iterator = code.entryBlock().listIterator(code); + while (iterator.hasNext() && iterator.peekNext().isArgument()) { + iterator.next(); + } + return iterator.insertConstNumberInstruction(code, appView.options(), 0, TypeElement.getInt()); + } + private DexMethod computeInstanceFieldMethod(DexField field) { EnumInstanceFieldKnownData enumFieldKnownData = unboxedEnumsInstanceFieldData.getInstanceFieldData(field.holder, field); @@ -362,6 +395,10 @@ if (type.isInt()) { return convertedEnums.get(receiver.definition); } + return getEnumTypeOrNull(type); + } + + private DexType getEnumTypeOrNull(TypeElement type) { if (!type.isClassType()) { return null; }
diff --git a/src/main/java/com/android/tools/r8/naming/IdentifierMinifier.java b/src/main/java/com/android/tools/r8/naming/IdentifierMinifier.java index a0b1328..b9fc4cb 100644 --- a/src/main/java/com/android/tools/r8/naming/IdentifierMinifier.java +++ b/src/main/java/com/android/tools/r8/naming/IdentifierMinifier.java
@@ -23,6 +23,7 @@ import com.android.tools.r8.shaking.AppInfoWithLiveness; import com.android.tools.r8.shaking.ProguardClassFilter; import com.android.tools.r8.utils.ThreadUtils; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.concurrent.ExecutionException; @@ -143,7 +144,8 @@ if (staticValue instanceof DexItemBasedValueString) { DexItemBasedValueString cnst = (DexItemBasedValueString) staticValue; DexString replacement = - cnst.getNameComputationInfo().computeNameFor(cnst.getValue(), appView, lens); + cnst.getNameComputationInfo() + .computeNameFor(cnst.getValue(), appView, appView.graphLens(), lens); encodedField.setStaticValue(new DexValueString(replacement)); } } @@ -158,7 +160,8 @@ if (instruction.isDexItemBasedConstString()) { DexItemBasedConstString cnst = instruction.asDexItemBasedConstString(); DexString replacement = - cnst.getNameComputationInfo().computeNameFor(cnst.getItem(), appView, lens); + cnst.getNameComputationInfo() + .computeNameFor(cnst.getItem(), appView, appView.graphLens(), lens); ConstString constString = new ConstString(cnst.AA, replacement); constString.setOffset(instruction.getOffset()); instructions[i] = constString; @@ -166,16 +169,24 @@ } } else { assert code.isCfCode(); - List<CfInstruction> instructions = code.asCfCode().instructions; + List<CfInstruction> instructions = code.asCfCode().getInstructions(); + List<CfInstruction> newInstructions = null; for (int i = 0; i < instructions.size(); ++i) { CfInstruction instruction = instructions.get(i); if (instruction.isDexItemBasedConstString()) { CfDexItemBasedConstString cnst = instruction.asDexItemBasedConstString(); DexString replacement = - cnst.getNameComputationInfo().computeNameFor(cnst.getItem(), appView, lens); - instructions.set(i, new CfConstString(replacement)); + cnst.getNameComputationInfo() + .computeNameFor(cnst.getItem(), appView, appView.graphLens(), lens); + if (newInstructions == null) { + newInstructions = new ArrayList<>(instructions); + } + newInstructions.set(i, new CfConstString(replacement)); } } + if (newInstructions != null) { + code.asCfCode().setInstructions(newInstructions); + } } } }
diff --git a/src/main/java/com/android/tools/r8/naming/dexitembasedstring/NameComputationInfo.java b/src/main/java/com/android/tools/r8/naming/dexitembasedstring/NameComputationInfo.java index a53864f..67e574c 100644 --- a/src/main/java/com/android/tools/r8/naming/dexitembasedstring/NameComputationInfo.java +++ b/src/main/java/com/android/tools/r8/naming/dexitembasedstring/NameComputationInfo.java
@@ -7,23 +7,28 @@ import com.android.tools.r8.graph.DexDefinitionSupplier; import com.android.tools.r8.graph.DexReference; import com.android.tools.r8.graph.DexString; +import com.android.tools.r8.graph.GraphLens; import com.android.tools.r8.naming.NamingLens; public abstract class NameComputationInfo<T extends DexReference> { public final DexString computeNameFor( - DexReference reference, DexDefinitionSupplier definitions, NamingLens namingLens) { + DexReference reference, + DexDefinitionSupplier definitions, + GraphLens graphLens, + NamingLens namingLens) { + DexReference rewritten = graphLens.lookupReference(reference); if (needsToComputeName()) { if (isFieldNameComputationInfo()) { return asFieldNameComputationInfo() - .internalComputeNameFor(reference.asDexField(), definitions, namingLens); + .internalComputeNameFor(rewritten.asDexField(), definitions, namingLens); } if (isClassNameComputationInfo()) { return asClassNameComputationInfo() - .internalComputeNameFor(reference.asDexType(), definitions, namingLens); + .internalComputeNameFor(rewritten.asDexType(), definitions, namingLens); } } - return namingLens.lookupName(reference, definitions.dexItemFactory()); + return namingLens.lookupName(rewritten, definitions.dexItemFactory()); } abstract DexString internalComputeNameFor(
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java index 0afccd0..a210ef7 100644 --- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java +++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -10,15 +10,6 @@ import static com.android.tools.r8.shaking.AnnotationRemover.shouldKeepAnnotation; import com.android.tools.r8.Diagnostic; -import com.android.tools.r8.cf.code.CfFieldInstruction; -import com.android.tools.r8.cf.code.CfInstruction; -import com.android.tools.r8.cf.code.CfInvoke; -import com.android.tools.r8.cf.code.CfInvokeDynamic; -import com.android.tools.r8.cf.code.CfLoad; -import com.android.tools.r8.cf.code.CfNew; -import com.android.tools.r8.cf.code.CfStackInstruction; -import com.android.tools.r8.cf.code.CfStackInstruction.Opcode; -import com.android.tools.r8.cf.code.CfStore; import com.android.tools.r8.dex.IndexedItemCollection; import com.android.tools.r8.errors.Unreachable; import com.android.tools.r8.experimental.graphinfo.GraphConsumer; @@ -85,7 +76,6 @@ import com.android.tools.r8.ir.code.InvokeMethod; import com.android.tools.r8.ir.code.InvokeVirtual; import com.android.tools.r8.ir.code.Value; -import com.android.tools.r8.ir.code.ValueType; import com.android.tools.r8.ir.desugar.DesugaredLibraryAPIConverter; import com.android.tools.r8.ir.desugar.LambdaClass; import com.android.tools.r8.ir.desugar.LambdaDescriptor; @@ -145,7 +135,6 @@ import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; -import org.objectweb.asm.Opcodes; /** * Approximates the runtime dependencies for the given set of roots. @@ -3190,50 +3179,7 @@ private void rewriteLambdaCallSites( DexEncodedMethod method, Map<DexCallSite, LambdaClass> callSites) { assert !callSites.isEmpty(); - CfCode code = method.getCode().asCfCode(); - List<CfInstruction> instructions = code.instructions; - int replaced = 0; - int maxTemp = 0; - for (int i = 0; i < instructions.size(); i++) { - CfInstruction instruction = instructions.get(i); - if (instruction instanceof CfInvokeDynamic) { - LambdaClass lambdaClass = callSites.get(((CfInvokeDynamic) instruction).getCallSite()); - if (lambdaClass == null) { - continue; - } - if (lambdaClass.isStateless()) { - CfFieldInstruction getStaticLambdaInstance = - new CfFieldInstruction( - Opcodes.GETSTATIC, lambdaClass.lambdaField, lambdaClass.lambdaField); - instructions.set(i, getStaticLambdaInstance); - } else { - List<CfInstruction> replacement = new ArrayList<>(); - int arguments = lambdaClass.descriptor.captures.size(); - int temp = code.getMaxLocals(); - for (int j = arguments - 1; j >= 0; j--) { - ValueType type = ValueType.fromDexType(lambdaClass.descriptor.captures.values[j]); - replacement.add(new CfStore(type, temp)); - temp += type.requiredRegisters(); - } - maxTemp = Math.max(temp, maxTemp); - replacement.add(new CfNew(lambdaClass.type)); - replacement.add(new CfStackInstruction(Opcode.Dup)); - for (int j = 0; j < arguments; j++) { - ValueType type = ValueType.fromDexType(lambdaClass.descriptor.captures.values[j]); - temp -= type.requiredRegisters(); - replacement.add(new CfLoad(type, temp)); - } - replacement.add(new CfInvoke(Opcodes.INVOKESPECIAL, lambdaClass.constructor, false)); - instructions.remove(i); - instructions.addAll(i, replacement); - } - ++replaced; - } - } - if (maxTemp > 0) { - assert maxTemp > code.getMaxLocals(); - code.setMaxLocals(maxTemp); - } + int replaced = LambdaRewriter.desugarLambdas(method, callSites::get); assert replaced == callSites.size(); }
diff --git a/src/main/java/com/android/tools/r8/shaking/MainDexClasses.java b/src/main/java/com/android/tools/r8/shaking/MainDexClasses.java index 67cd3b8..a5fbd32 100644 --- a/src/main/java/com/android/tools/r8/shaking/MainDexClasses.java +++ b/src/main/java/com/android/tools/r8/shaking/MainDexClasses.java
@@ -49,8 +49,12 @@ } } + public boolean contains(DexType type) { + return mainDexClasses.contains(type); + } + public boolean contains(DexProgramClass clazz) { - return mainDexClasses.contains(clazz.getType()); + return contains(clazz.getType()); } public boolean containsAnyOf(Iterable<DexProgramClass> classes) {
diff --git a/src/main/java/com/android/tools/r8/shaking/MainDexDirectReferenceTracer.java b/src/main/java/com/android/tools/r8/shaking/MainDexDirectReferenceTracer.java index 973691f..91f9bbc 100644 --- a/src/main/java/com/android/tools/r8/shaking/MainDexDirectReferenceTracer.java +++ b/src/main/java/com/android/tools/r8/shaking/MainDexDirectReferenceTracer.java
@@ -8,6 +8,7 @@ import com.android.tools.r8.dex.IndexedItemCollection; import com.android.tools.r8.graph.AppInfoWithClassHierarchy; +import com.android.tools.r8.graph.DexAnnotation; import com.android.tools.r8.graph.DexAnnotationSet; import com.android.tools.r8.graph.DexCallSite; import com.android.tools.r8.graph.DexClass; @@ -47,7 +48,10 @@ assert clazz != null; consumer.accept(type); // Super and interfaces are live, no need to add them. - traceAnnotationsDirectDependencies(clazz.annotations()); + if (!DexAnnotation.hasSynthesizedClassAnnotation( + clazz.annotations(), appInfo.dexItemFactory())) { + traceAnnotationsDirectDependencies(clazz.annotations()); + } clazz.forEachField(field -> consumer.accept(field.field.type)); clazz.forEachProgramMethodMatching( definition -> {
diff --git a/src/main/java/com/android/tools/r8/synthesis/SynthesizingContext.java b/src/main/java/com/android/tools/r8/synthesis/SynthesizingContext.java index 380c2dc..93d1248 100644 --- a/src/main/java/com/android/tools/r8/synthesis/SynthesizingContext.java +++ b/src/main/java/com/android/tools/r8/synthesis/SynthesizingContext.java
@@ -3,34 +3,106 @@ // BSD-style license that can be found in the LICENSE file. package com.android.tools.r8.synthesis; +import com.android.tools.r8.graph.DexItemFactory; +import com.android.tools.r8.graph.DexProgramClass; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.GraphLens.NonIdentityGraphLens; import com.android.tools.r8.origin.Origin; +import com.android.tools.r8.shaking.MainDexClasses; +import java.util.Comparator; +import java.util.Set; /** - * A synthesizing context is the input type and origin that gives rise to a synthetic item. + * A synthesizing context is a description of the context that gives rise to a synthetic item. * * <p>Note that a context can only itself be a synthetic item if it was provided as an input that - * was marked as synthetic already, in which case it is its own context. In other words, - * - * <pre> - * for any synthetic item, I: - * context(I) == holder(I) iff I is a synthetic input - * </pre> + * was marked as synthetic already, in which case the context consists of the synthetic input type + * as well as the original synthesizing context type specified in it synthesis annotation. * * <p>This class is internal to the synthetic items collection, thus package-protected. */ -class SynthesizingContext { - final DexType type; - final Origin origin; +class SynthesizingContext implements Comparable<SynthesizingContext> { - SynthesizingContext(DexType type, Origin origin) { - this.type = type; - this.origin = origin; + // The synthesizing context is the type used for ensuring a hygienic placement of a synthetic. + // Thus this type will potentially be used as the prefix of a synthetic class. + private final DexType synthesizingContextType; + + // The input context is the program input type that is the actual context of a synthetic. + // In particular, if the synthetic type is itself a program input, then it will be its own + // input context but it will have a distinct synthesizing context (encoded in its annotation). + private final DexType inputContextType; + private final Origin inputContextOrigin; + + static SynthesizingContext fromNonSyntheticInputClass(DexProgramClass clazz) { + // A context that is itself non-synthetic is the single context, thus both the input context + // and synthesizing context coincide. + return new SynthesizingContext(clazz.type, clazz.type, clazz.origin); + } + + static SynthesizingContext fromSyntheticInputClass( + DexProgramClass clazz, DexType synthesizingContextType) { + assert synthesizingContextType != null; + // A context that is itself synthetic must denote a synthesizing context from which to ensure + // hygiene. This synthesizing context type is encoded on the synthetic for intermediate builds. + return new SynthesizingContext(synthesizingContextType, clazz.type, clazz.origin); + } + + private SynthesizingContext( + DexType synthesizingContextType, DexType inputContextType, Origin inputContextOrigin) { + this.synthesizingContextType = synthesizingContextType; + this.inputContextType = inputContextType; + this.inputContextOrigin = inputContextOrigin; + } + + @Override + public int compareTo(SynthesizingContext other) { + return Comparator + // The first item to compare is the synthesizing context type. This is the type used to + // choose the context prefix for items. + .comparing(SynthesizingContext::getSynthesizingContextType, DexType::slowCompareTo) + // To ensure that equals coincides with compareTo == 0, we then compare 'type'. + .thenComparing(c -> c.inputContextType, DexType::slowCompareTo) + .compare(this, other); + } + + Origin getInputContextOrigin() { + return inputContextOrigin; + } + + DexType createHygienicType(int syntheticId, DexItemFactory factory) { + // If the context is a synthetic input, then use its annotated context as the hygienic context. + String contextDesc = synthesizingContextType.toDescriptorString(); + String prefix = contextDesc.substring(0, contextDesc.length() - 1); + String suffix = SyntheticItems.INTERNAL_SYNTHETIC_CLASS_SEPARATOR + syntheticId + ";"; + return factory.createType(prefix + suffix); } SynthesizingContext rewrite(NonIdentityGraphLens lens) { - DexType rewritten = lens.lookupType(type); - return rewritten == type ? this : new SynthesizingContext(type, origin); + DexType rewrittenInputeContextType = lens.lookupType(inputContextType); + DexType rewrittenSynthesizingContextType = lens.lookupType(synthesizingContextType); + return rewrittenInputeContextType == inputContextType + && rewrittenSynthesizingContextType == synthesizingContextType + ? this + : new SynthesizingContext( + rewrittenSynthesizingContextType, rewrittenInputeContextType, inputContextOrigin); + } + + DexType getSynthesizingContextType() { + return synthesizingContextType; + } + + void addIfDerivedFromMainDexClass( + DexProgramClass externalSyntheticClass, + MainDexClasses mainDexClasses, + Set<DexType> allMainDexTypes, + Set<DexType> derivedMainDexTypesToIgnore) { + // The input context type (not the annotated context) determines if the derived class is to be + // in main dex. + // TODO(b/168584485): Once resolved allMainDexTypes == mainDexClasses. + if (allMainDexTypes.contains(inputContextType)) { + mainDexClasses.add(externalSyntheticClass); + // Mark the type as to be ignored when computing main-dex placement for legacy types. + derivedMainDexTypesToIgnore.add(inputContextType); + } } }
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticClassBuilder.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticClassBuilder.java index ad04a8d..1091d09 100644 --- a/src/main/java/com/android/tools/r8/synthesis/SyntheticClassBuilder.java +++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticClassBuilder.java
@@ -39,7 +39,7 @@ SyntheticClassBuilder(DexType type, SynthesizingContext context, DexItemFactory factory) { this.factory = factory; this.type = type; - this.origin = context.origin; + this.origin = context.getInputContextOrigin(); this.superType = factory.objectType; }
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java index daf936d..1d037cc 100644 --- a/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java +++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticDefinition.java
@@ -4,12 +4,10 @@ package com.android.tools.r8.synthesis; import com.android.tools.r8.graph.DexProgramClass; -import com.android.tools.r8.graph.DexType; -import com.android.tools.r8.origin.Origin; import com.google.common.hash.HashCode; /** - * Base type for the defintion of a synthetic item. + * Base type for the definition of a synthetic item. * * <p>This class is internal to the synthetic items collection, thus package-protected. */ @@ -26,14 +24,6 @@ return context; } - DexType getContextType() { - return context.type; - } - - Origin getContextOrigin() { - return context.origin; - } - abstract DexProgramClass getHolder(); abstract HashCode computeHash();
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java index 2996794..31a4afd 100644 --- a/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java +++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticFinalization.java
@@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ListMultimap; +import com.google.common.collect.Sets; import com.google.common.hash.HashCode; import java.util.ArrayList; import java.util.Collection; @@ -29,6 +30,7 @@ import java.util.IdentityHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TreeSet; import java.util.function.Predicate; @@ -103,19 +105,22 @@ Builder lensBuilder = NestedGraphLens.builder(); List<DexProgramClass> newProgramClasses = new ArrayList<>(); List<DexProgramClass> finalSyntheticClasses = new ArrayList<>(); + Set<DexType> derivedMainDexTypesToIgnore = Sets.newIdentityHashSet(); buildLensAndProgram( application, equivalences, syntheticItems::containsKey, mainDexClasses, lensBuilder, - options.itemFactory, + options, newProgramClasses, - finalSyntheticClasses); + finalSyntheticClasses, + derivedMainDexTypesToIgnore); newProgramClasses.addAll(finalSyntheticClasses); - handleSynthesizedClassMapping(finalSyntheticClasses, application, options, mainDexClasses); + handleSynthesizedClassMapping( + finalSyntheticClasses, application, options, mainDexClasses, derivedMainDexTypesToIgnore); DexApplication app = application.builder().replaceProgramClasses(newProgramClasses).build(); @@ -132,10 +137,9 @@ } private boolean verifyNoNestedSynthetics() { + // Check that a context is never itself synthetic class. for (SyntheticReference item : syntheticItems.values()) { - // Check that a context is never a synthetic unless it is an input, thus its own context. - assert item.getHolder() == item.getContextType() - || !syntheticItems.containsKey(item.getContextType()); + assert !syntheticItems.containsKey(item.getContext().getSynthesizingContextType()); } return true; } @@ -144,12 +148,14 @@ List<DexProgramClass> finalSyntheticClasses, DexApplication application, InternalOptions options, - MainDexClasses mainDexClasses) { - boolean includeSynthesizedClassMappingInOutput = options.intermediate && !options.cfToCfDesugar; + MainDexClasses mainDexClasses, + Set<DexType> derivedMainDexTypesToIgnore) { + boolean includeSynthesizedClassMappingInOutput = shouldAnnotateSynthetics(options); if (includeSynthesizedClassMappingInOutput) { updateSynthesizedClassMapping(application, finalSyntheticClasses); } - updateMainDexListWithSynthesizedClassMap(application, mainDexClasses); + updateMainDexListWithSynthesizedClassMap( + application, mainDexClasses, derivedMainDexTypesToIgnore); if (!includeSynthesizedClassMappingInOutput) { clearSynthesizedClassMapping(application); } @@ -194,7 +200,9 @@ } private void updateMainDexListWithSynthesizedClassMap( - DexApplication application, MainDexClasses mainDexClasses) { + DexApplication application, + MainDexClasses mainDexClasses, + Set<DexType> derivedMainDexTypesToIgnore) { if (mainDexClasses.isEmpty()) { return; } @@ -208,10 +216,12 @@ DexAnnotation.readAnnotationSynthesizedClassMap( programClass, application.dexItemFactory); for (DexType type : derived) { - DexProgramClass syntheticClass = - DexProgramClass.asProgramClassOrNull(application.definitionFor(type)); - if (syntheticClass != null) { - newMainDexClasses.add(syntheticClass); + if (!derivedMainDexTypesToIgnore.contains(type)) { + DexProgramClass syntheticClass = + DexProgramClass.asProgramClassOrNull(application.definitionFor(type)); + if (syntheticClass != null) { + newMainDexClasses.add(syntheticClass); + } } } } @@ -232,9 +242,11 @@ Predicate<DexType> isSyntheticType, MainDexClasses mainDexClasses, Builder lensBuilder, - DexItemFactory factory, + InternalOptions options, List<DexProgramClass> normalClasses, - List<DexProgramClass> newSyntheticClasses) { + List<DexProgramClass> newSyntheticClasses, + Set<DexType> derivedMainDexTypesToIgnore) { + DexItemFactory factory = options.itemFactory; for (DexProgramClass clazz : app.classes()) { if (!isSyntheticType.test(clazz.type)) { @@ -242,43 +254,78 @@ } } + // TODO(b/168584485): Remove this once class-mapping support is removed. + Set<DexType> derivedMainDexTypes = Sets.newIdentityHashSet(); + mainDexClasses.forEach( + mainDexType -> { + derivedMainDexTypes.add(mainDexType); + DexProgramClass mainDexClass = + DexProgramClass.asProgramClassOrNull(app.definitionFor(mainDexType)); + if (mainDexClass != null) { + derivedMainDexTypes.addAll( + DexAnnotation.readAnnotationSynthesizedClassMap(mainDexClass, options.itemFactory)); + } + }); + syntheticMethodGroups.forEach( (syntheticType, syntheticGroup) -> { - SyntheticMethodDefinition firstMember = syntheticGroup.getRepresentative(); - SynthesizingContext context = firstMember.getContext(); + SyntheticMethodDefinition representative = syntheticGroup.getRepresentative(); + SynthesizingContext context = representative.getContext(); SyntheticClassBuilder builder = new SyntheticClassBuilder(syntheticType, context, factory); // TODO(b/158159959): Support grouping multiple methods per synthetic class. builder.addMethod( methodBuilder -> { - DexEncodedMethod definition = firstMember.getMethod().getDefinition(); + DexEncodedMethod definition = representative.getMethod().getDefinition(); methodBuilder .setAccessFlags(definition.accessFlags) .setProto(definition.getProto()) .setCode(m -> definition.getCode()); }); DexProgramClass externalSyntheticClass = builder.build(); + if (shouldAnnotateSynthetics(options)) { + externalSyntheticClass.setAnnotations( + externalSyntheticClass + .annotations() + .getWithAddedOrReplaced( + DexAnnotation.createAnnotationSynthesizedClass( + context.getSynthesizingContextType(), factory))); + } assert externalSyntheticClass.getMethodCollection().size() == 1; DexEncodedMethod externalSyntheticMethod = externalSyntheticClass.methods().iterator().next(); newSyntheticClasses.add(externalSyntheticClass); for (SyntheticMethodDefinition member : syntheticGroup.getMembers()) { - lensBuilder.map( - member.getMethod().getHolder().getType(), externalSyntheticClass.getType()); - lensBuilder.map(member.getMethod().getReference(), externalSyntheticMethod.method); - DexType memberContext = member.getContextType(); + if (member.getMethod().getReference() != externalSyntheticMethod.method) { + lensBuilder.map(member.getMethod().getReference(), externalSyntheticMethod.method); + } + member + .getContext() + .addIfDerivedFromMainDexClass( + externalSyntheticClass, + mainDexClasses, + derivedMainDexTypes, + derivedMainDexTypesToIgnore); + // TODO(b/168584485): Remove this once class-mapping support is removed. DexProgramClass from = - DexProgramClass.asProgramClassOrNull(app.definitionFor(memberContext)); + DexProgramClass.asProgramClassOrNull( + app.definitionFor(member.getContext().getSynthesizingContextType())); if (from != null) { externalSyntheticClass.addSynthesizedFrom(from); - if (mainDexClasses.contains(from)) { - mainDexClasses.add(externalSyntheticClass); - } } } }); } + private static boolean shouldAnnotateSynthetics(InternalOptions options) { + // Only intermediate builds have annotated synthetics to allow later sharing. + // This is currently also disabled on CF to CF desugaring to avoid missing class references to + // the annotated classes. + // TODO(b/147485959): Find an alternative encoding for synthetics to avoid missing-class refs. + // TODO(b/168584485): Remove support for main-dex tracing with the class-map annotation. + return options.intermediate && !options.cfToCfDesugar; + } + private static <T extends SyntheticDefinition & Comparable<T>> Map<DexType, EquivalenceGroup<T>> computeActualEquivalences( Map<HashCode, List<T>> potentialEquivalences, DexItemFactory factory) { @@ -298,13 +345,15 @@ // The member becomes a new singleton group. // TODO(b/158159959): Consider checking for sub-groups of matching members. groupsPerContext - .computeIfAbsent(member.getContextType(), k -> new ArrayList<>()) + .computeIfAbsent( + member.getContext().getSynthesizingContextType(), k -> new ArrayList<>()) .add(new EquivalenceGroup<>(member)); } } } groupsPerContext - .computeIfAbsent(representative.getContextType(), k -> new ArrayList<>()) + .computeIfAbsent( + representative.getContext().getSynthesizingContextType(), k -> new ArrayList<>()) .add(new EquivalenceGroup<>(representative, group)); }); Map<DexType, EquivalenceGroup<T>> equivalences = new IdentityHashMap<>(); @@ -312,7 +361,9 @@ (context, groups) -> { groups.sort(EquivalenceGroup::compareTo); for (int i = 0; i < groups.size(); i++) { - equivalences.put(createExternalType(context, i, factory), groups.get(i)); + EquivalenceGroup<T> group = groups.get(i); + DexType representativeType = createExternalType(context, i, factory); + equivalences.put(representativeType, group); } }); return equivalences; @@ -355,10 +406,14 @@ List<SyntheticMethodDefinition> methods = new ArrayList<>(syntheticItems.size()); for (SyntheticReference reference : syntheticItems.values()) { SyntheticDefinition definition = reference.lookupDefinition(finalApp::definitionFor); - assert definition != null; - assert definition instanceof SyntheticMethodDefinition; - if (definition != null && definition instanceof SyntheticMethodDefinition) { - methods.add(((SyntheticMethodDefinition) definition)); + if (definition == null || !(definition instanceof SyntheticMethodDefinition)) { + // We expect pruned definitions to have been removed. + assert false; + continue; + } + SyntheticMethodDefinition method = (SyntheticMethodDefinition) definition; + if (SyntheticMethodBuilder.isValidSyntheticMethod(method.getMethod().getDefinition())) { + methods.add(method); } } return methods;
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticItems.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticItems.java index ef45975..6f5c49d 100644 --- a/src/main/java/com/android/tools/r8/synthesis/SyntheticItems.java +++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticItems.java
@@ -4,9 +4,14 @@ package com.android.tools.r8.synthesis; import com.android.tools.r8.errors.InternalCompilerError; +import com.android.tools.r8.graph.AppInfo; import com.android.tools.r8.graph.AppView; +import com.android.tools.r8.graph.ClassAccessFlags; +import com.android.tools.r8.graph.DexAnnotation; +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.DexEncodedMethod; import com.android.tools.r8.graph.DexItemFactory; import com.android.tools.r8.graph.DexProgramClass; import com.android.tools.r8.graph.DexType; @@ -46,7 +51,7 @@ public static final String EXTERNAL_SYNTHETIC_CLASS_SEPARATOR = "-$$ExternalSynthetic"; /** Method prefix when generating synthetic methods in a class. */ - static final String INTERNAL_SYNTHETIC_METHOD_PREFIX = "m"; + public static final String INTERNAL_SYNTHETIC_METHOD_PREFIX = "m"; public static boolean verifyNotInternalSynthetic(DexType type) { assert !type.toDescriptorString().contains(SyntheticItems.INTERNAL_SYNTHETIC_CLASS_SEPARATOR); @@ -58,13 +63,15 @@ /** * Thread safe collection of synthesized classes that are not yet committed to the application. - * TODO(b/158159959): Remove legacy support. + * + * <p>TODO(b/158159959): Remove legacy support. */ private final Map<DexType, DexProgramClass> legacyPendingClasses = new ConcurrentHashMap<>(); /** - * Immutable set of synthetic types in the application (eg, committed). TODO(b/158159959): Remove - * legacy support. + * Immutable set of synthetic types in the application (eg, committed). + * + * <p>TODO(b/158159959): Remove legacy support. */ private final ImmutableSet<DexType> legacySyntheticTypes; @@ -76,8 +83,9 @@ private final ImmutableMap<DexType, SyntheticReference> nonLecacySyntheticItems; // Only for use from initial AppInfo/AppInfoWithClassHierarchy create functions. */ - public static SyntheticItems createInitialSyntheticItems() { - return new SyntheticItems(0, ImmutableSet.of(), ImmutableMap.of()); + public static CommittedItems createInitialSyntheticItems(DexApplication application) { + return new CommittedItems( + 0, application, ImmutableSet.of(), ImmutableMap.of(), ImmutableList.of()); } // Only for conversion to a mutable synthetic items collection. @@ -92,12 +100,79 @@ this.nextSyntheticId = nextSyntheticId; this.legacySyntheticTypes = legacySyntheticTypes; this.nonLecacySyntheticItems = nonLecacySyntheticItems; - assert nonLecacySyntheticItems.keySet().stream() - .noneMatch( - t -> t.toDescriptorString().endsWith(getSyntheticDescriptorSuffix(nextSyntheticId))); assert Sets.intersection(nonLecacySyntheticItems.keySet(), legacySyntheticTypes).isEmpty(); } + public static void collectSyntheticInputs(AppView<AppInfo> appView) { + // Collecting synthetic items must be the very first task after application build. + SyntheticItems synthetics = appView.getSyntheticItems(); + assert synthetics.nextSyntheticId == 0; + assert synthetics.nonLecacySyntheticItems.isEmpty(); + assert !synthetics.hasPendingSyntheticClasses(); + if (appView.options().intermediate) { + // If the compilation is in intermediate mode the synthetics should just be passed through. + return; + } + ImmutableMap.Builder<DexType, SyntheticReference> pending = ImmutableMap.builder(); + // TODO(b/158159959): Consider identifying synthetics in the input reader to speed this up. + for (DexProgramClass clazz : appView.appInfo().classes()) { + DexType annotatedContextType = isSynthesizedMethodsContainer(clazz, appView.dexItemFactory()); + if (annotatedContextType == null) { + continue; + } + clazz.setAnnotations(DexAnnotationSet.empty()); + SynthesizingContext context = + SynthesizingContext.fromSyntheticInputClass(clazz, annotatedContextType); + clazz.forEachProgramMethod( + // TODO(b/158159959): Support having multiple methods per class. + method -> { + method.getDefinition().setAnnotations(DexAnnotationSet.empty()); + pending.put(clazz.type, new SyntheticMethodDefinition(context, method).toReference()); + }); + } + pending.putAll(synthetics.nonLecacySyntheticItems); + ImmutableMap<DexType, SyntheticReference> nonLegacySyntheticItems = pending.build(); + if (nonLegacySyntheticItems.isEmpty()) { + return; + } + CommittedItems commit = + new CommittedItems( + synthetics.nextSyntheticId, + appView.appInfo().app(), + synthetics.legacySyntheticTypes, + nonLegacySyntheticItems, + ImmutableList.of()); + appView.setAppInfo(new AppInfo(commit, appView.appInfo().getMainDexClasses())); + } + + private static DexType isSynthesizedMethodsContainer( + DexProgramClass clazz, DexItemFactory factory) { + ClassAccessFlags flags = clazz.accessFlags; + if (!flags.isSynthetic() || flags.isAbstract() || flags.isEnum()) { + return null; + } + DexType contextType = + DexAnnotation.getSynthesizedClassAnnotationContextType(clazz.annotations(), factory); + if (contextType == null) { + return null; + } + if (clazz.superType != factory.objectType) { + return null; + } + if (!clazz.interfaces.isEmpty()) { + return null; + } + if (clazz.annotations().size() != 1) { + return null; + } + for (DexEncodedMethod method : clazz.methods()) { + if (!SyntheticMethodBuilder.isValidSyntheticMethod(method)) { + return null; + } + } + return contextType; + } + // Internal synthetic id creation helpers. private synchronized int getNextSyntheticId() { @@ -110,14 +185,7 @@ private static DexType hygienicType( DexItemFactory factory, int syntheticId, SynthesizingContext context) { - String contextDesc = context.type.toDescriptorString(); - String prefix = contextDesc.substring(0, contextDesc.length() - 1); - String syntheticDesc = prefix + getSyntheticDescriptorSuffix(syntheticId); - return factory.createType(syntheticDesc); - } - - private static String getSyntheticDescriptorSuffix(int syntheticId) { - return INTERNAL_SYNTHETIC_CLASS_SEPARATOR + syntheticId + ";"; + return context.createHygienicType(syntheticId, factory); } // Predicates and accessors. @@ -184,7 +252,7 @@ SyntheticReference committedItemContext = nonLecacySyntheticItems.get(context.type); return committedItemContext != null ? committedItemContext.getContext() - : new SynthesizingContext(context.type, context.origin); + : SynthesizingContext.fromNonSyntheticInputClass(context); } // Addition and creation of synthetic items. @@ -201,11 +269,11 @@ public ProgramMethod createMethod( DexProgramClass context, DexItemFactory factory, Consumer<SyntheticMethodBuilder> fn) { // Obtain the outer synthesizing context in the case the context itself is synthetic. - // The is to ensure a flat input-type -> synthetic-item mapping. + // This is to ensure a flat input-type -> synthetic-item mapping. SynthesizingContext outerContext = getSynthesizingContext(context); - DexType type = hygienicType(factory, getNextSyntheticId(), outerContext); - DexProgramClass clazz = - new SyntheticClassBuilder(type, outerContext, factory).addMethod(fn).build(); + DexType type = outerContext.createHygienicType(getNextSyntheticId(), factory); + SyntheticClassBuilder classBuilder = new SyntheticClassBuilder(type, outerContext, factory); + DexProgramClass clazz = classBuilder.addMethod(fn).build(); ProgramMethod method = new ProgramMethod(clazz, clazz.methods().iterator().next()); addPendingDefinition(new SyntheticMethodDefinition(outerContext, method)); return method;
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodBuilder.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodBuilder.java index 90e593c..7cc0c32 100644 --- a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodBuilder.java +++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodBuilder.java
@@ -4,12 +4,14 @@ package com.android.tools.r8.synthesis; import com.android.tools.r8.graph.Code; +import com.android.tools.r8.graph.DexAnnotation; import com.android.tools.r8.graph.DexAnnotationSet; import com.android.tools.r8.graph.DexEncodedMethod; import com.android.tools.r8.graph.DexMethod; import com.android.tools.r8.graph.DexProto; import com.android.tools.r8.graph.MethodAccessFlags; import com.android.tools.r8.graph.ParameterAnnotationsList; +import java.util.List; public class SyntheticMethodBuilder { @@ -22,6 +24,7 @@ private DexProto proto = null; private SyntheticCodeGenerator codeGenerator = null; private MethodAccessFlags accessFlags = null; + private List<DexAnnotation> annotations = null; SyntheticMethodBuilder(SyntheticClassBuilder parent, String name) { this.parent = parent; @@ -46,13 +49,30 @@ DexEncodedMethod build() { boolean isCompilerSynthesized = true; DexMethod methodSignature = getMethodSignature(); - return new DexEncodedMethod( - methodSignature, - getAccessFlags(), - getAnnotations(), - getParameterAnnotations(), - getCodeObject(methodSignature), - isCompilerSynthesized); + DexEncodedMethod method = + new DexEncodedMethod( + methodSignature, + getAccessFlags(), + getAnnotations(), + getParameterAnnotations(), + getCodeObject(methodSignature), + isCompilerSynthesized); + assert isValidSyntheticMethod(method); + return method; + } + + /** + * Predicate for what is a "supported" synthetic method. + * + * <p>This method is used when identifying synthetic methods in the program input and should be as + * narrow as possible. + */ + public static boolean isValidSyntheticMethod(DexEncodedMethod method) { + return method.isStatic() + && method.isNonAbstractNonNativeMethod() + && method.isPublic() + && method.annotations().isEmpty() + && method.getParameterAnnotations().isEmpty(); } private DexMethod getMethodSignature() { @@ -64,7 +84,9 @@ } private DexAnnotationSet getAnnotations() { - return DexAnnotationSet.empty(); + return annotations == null + ? DexAnnotationSet.empty() + : new DexAnnotationSet(annotations.toArray(DexAnnotation.EMPTY_ARRAY)); } private ParameterAnnotationsList getParameterAnnotations() {
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodDefinition.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodDefinition.java index 6fb07c5..90bcf51 100644 --- a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodDefinition.java +++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodDefinition.java
@@ -5,7 +5,6 @@ import com.android.tools.r8.graph.DexEncodedMethod; import com.android.tools.r8.graph.DexProgramClass; -import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.ProgramMethod; import com.google.common.hash.HashCode; import com.google.common.hash.Hasher; @@ -60,7 +59,7 @@ // Since methods are sharable they must define an order from which representatives can be found. @Override public int compareTo(SyntheticMethodDefinition other) { - return Comparator.comparing(SyntheticMethodDefinition::getContextType, DexType::slowCompareTo) + return Comparator.comparing(SyntheticMethodDefinition::getContext) .thenComparing(m -> m.method.getDefinition(), DexEncodedMethod::syntheticCompareTo) .compare(this, other); }
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java index 6070e49..86704ea 100644 --- a/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java +++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticMethodReference.java
@@ -45,6 +45,6 @@ DexMethod rewritten = lens.lookupMethod(method); return context == getContext() && rewritten == method ? this - : new SyntheticMethodReference(context, method); + : new SyntheticMethodReference(context, rewritten); } }
diff --git a/src/main/java/com/android/tools/r8/synthesis/SyntheticReference.java b/src/main/java/com/android/tools/r8/synthesis/SyntheticReference.java index 9b95337..408324e 100644 --- a/src/main/java/com/android/tools/r8/synthesis/SyntheticReference.java +++ b/src/main/java/com/android/tools/r8/synthesis/SyntheticReference.java
@@ -6,7 +6,6 @@ import com.android.tools.r8.graph.DexClass; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.GraphLens.NonIdentityGraphLens; -import com.android.tools.r8.origin.Origin; import java.util.function.Function; /** @@ -27,14 +26,6 @@ return context; } - final DexType getContextType() { - return context.type; - } - - final Origin getContextOrigin() { - return context.origin; - } - abstract DexType getHolder(); abstract SyntheticReference rewrite(NonIdentityGraphLens lens);
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java index 1a53b74..df274fe 100644 --- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java +++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -91,8 +91,6 @@ public enum DesugarState { OFF, - // This is for use when desugar has run before, and backports have still not been desugared. - ONLY_BACKPORT_STATICS, ON }
diff --git a/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java b/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java index 8f2e9b1..0f62c75 100644 --- a/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java +++ b/src/test/java/com/android/tools/r8/GenerateMainDexListRunResult.java
@@ -4,6 +4,9 @@ package com.android.tools.r8; +import com.android.tools.r8.references.ClassReference; +import com.android.tools.r8.references.Reference; +import com.android.tools.r8.utils.ListUtils; import java.util.List; public class GenerateMainDexListRunResult @@ -16,6 +19,16 @@ this.mainDexList = mainDexList; } + public List<ClassReference> getMainDexList() { + return ListUtils.map( + mainDexList, + entry -> { + assert entry.endsWith(".class"); + String binaryName = entry.substring(0, entry.length() - ".class".length()); + return Reference.classFromBinaryName(binaryName); + }); + } + @Override protected GenerateMainDexListRunResult self() { return this;
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java index 85705d2..86776a3 100644 --- a/src/test/java/com/android/tools/r8/R8TestBuilder.java +++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -154,7 +154,7 @@ features); switch (allowedDiagnosticMessages) { case ALL: - compileResult.assertDiagnosticThatMatches(new IsAnything<>()); + compileResult.getDiagnosticMessages().assertAllDiagnosticsMatch(new IsAnything<>()); break; case ERROR: compileResult.assertOnlyErrors();
diff --git a/src/test/java/com/android/tools/r8/TestBaseBuilder.java b/src/test/java/com/android/tools/r8/TestBaseBuilder.java index 4388754..1fe43fb 100644 --- a/src/test/java/com/android/tools/r8/TestBaseBuilder.java +++ b/src/test/java/com/android/tools/r8/TestBaseBuilder.java
@@ -6,14 +6,16 @@ import com.android.tools.r8.ProgramResource.Kind; import com.android.tools.r8.origin.Origin; +import com.android.tools.r8.references.ClassReference; +import com.android.tools.r8.references.Reference; import com.android.tools.r8.utils.DescriptorUtils; +import com.android.tools.r8.utils.ListUtils; import com.google.common.collect.ImmutableMap; import java.nio.file.Path; import java.util.Arrays; import java.util.Collection; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; public abstract class TestBaseBuilder< C extends BaseCommand, @@ -69,14 +71,21 @@ return self(); } + public T addMainDexListClassReferences(ClassReference... classes) { + return addMainDexListClassReferences(Arrays.asList(classes)); + } + + public T addMainDexListClassReferences(Collection<ClassReference> classes) { + classes.forEach(c -> builder.addMainDexClasses(c.getTypeName())); + return self(); + } + public T addMainDexListClasses(Class<?>... classes) { return addMainDexListClasses(Arrays.asList(classes)); } public T addMainDexListClasses(Collection<Class<?>> classes) { - builder.addMainDexClasses( - classes.stream().map(Class::getTypeName).collect(Collectors.toList())); - return self(); + return addMainDexListClassReferences(ListUtils.map(classes, Reference::classFromClass)); } public T addMainDexListFiles(Collection<Path> files) {
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithFeatureSplitTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithFeatureSplitTest.java new file mode 100644 index 0000000..5907c150 --- /dev/null +++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithFeatureSplitTest.java
@@ -0,0 +1,130 @@ +// Copyright (c) 2020, 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.classmerging.horizontal; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static com.android.tools.r8.utils.codeinspector.Matchers.notIf; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; + +import com.android.tools.r8.NeverClassInline; +import com.android.tools.r8.R8TestCompileResult; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.dexsplitter.SplitterTestBase.RunInterface; +import com.android.tools.r8.utils.BooleanUtils; +import com.android.tools.r8.utils.codeinspector.CodeInspector; +import java.util.List; +import org.junit.Test; +import org.junit.runners.Parameterized; + +public class ClassesWithFeatureSplitTest extends HorizontalClassMergingTestBase { + public ClassesWithFeatureSplitTest( + TestParameters parameters, boolean enableHorizontalClassMerging) { + super(parameters, enableHorizontalClassMerging); + } + + @Parameterized.Parameters(name = "{0}, horizontalClassMerging:{1}") + public static List<Object[]> data() { + return buildParameters( + getTestParameters().withDexRuntimes().withAllApiLevels().build(), BooleanUtils.values()); + } + + @Test + public void testR8() throws Exception { + R8TestCompileResult compileResult = + testForR8(parameters.getBackend()) + .addProgramClasses(Base.class) + .addFeatureSplitRuntime() + .addFeatureSplit(Feature1Class1.class, Feature1Class2.class, Feature1Main.class) + .addFeatureSplit(Feature2Class.class, Feature2Main.class) + .addKeepFeatureMainRule(Feature1Main.class) + .addKeepFeatureMainRule(Feature2Main.class) + .addOptionsModification( + options -> options.enableHorizontalClassMerging = enableHorizontalClassMerging) + .enableNeverClassInliningAnnotations() + .setMinApi(parameters.getApiLevel()) + .compile() + .inspect(this::inspectBase, this::inspectFeature1, this::inspectFeature2); + + compileResult + .runFeature(parameters.getRuntime(), Feature1Main.class, compileResult.getFeature(0)) + .assertSuccessWithOutputLines("base", "feature 1 class 1", "feature 1 class 2"); + + compileResult + .runFeature(parameters.getRuntime(), Feature2Main.class, compileResult.getFeature(1)) + .assertSuccessWithOutputLines("base", "feature 2"); + } + + private void inspectBase(CodeInspector inspector) { + assertThat(inspector.clazz(Base.class), isPresent()); + assertThat(inspector.clazz(Feature1Class1.class), not(isPresent())); + assertThat(inspector.clazz(Feature1Class2.class), not(isPresent())); + assertThat(inspector.clazz(Feature2Class.class), not(isPresent())); + } + + private void inspectFeature1(CodeInspector inspector) { + assertThat(inspector.clazz(Feature1Main.class), isPresent()); + assertThat(inspector.clazz(Feature1Class1.class), isPresent()); + assertThat( + inspector.clazz(Feature1Class2.class), notIf(isPresent(), enableHorizontalClassMerging)); + assertThat(inspector.clazz(Feature2Main.class), not(isPresent())); + assertThat(inspector.clazz(Feature2Class.class), not(isPresent())); + } + + private void inspectFeature2(CodeInspector inspector) { + assertThat(inspector.clazz(Feature1Main.class), not(isPresent())); + assertThat(inspector.clazz(Feature1Class1.class), not(isPresent())); + assertThat(inspector.clazz(Feature1Class2.class), not(isPresent())); + assertThat(inspector.clazz(Feature2Main.class), isPresent()); + assertThat(inspector.clazz(Feature2Class.class), isPresent()); + } + + @NeverClassInline + public static class Base { + public Base() { + System.out.println("base"); + } + } + + @NeverClassInline + public static class Feature1Class1 { + public Feature1Class1() { + System.out.println("feature 1 class 1"); + } + } + + @NeverClassInline + public static class Feature1Class2 { + public Feature1Class2() { + System.out.println("feature 1 class 2"); + } + } + + @NeverClassInline + public static class Feature2Class { + public Feature2Class() { + System.out.println("feature 2"); + } + } + + public static class Feature1Main implements RunInterface { + + @Override + public void run() { + new Base(); + new Feature1Class1(); + new Feature1Class2(); + } + } + + public static class Feature2Main implements RunInterface { + + @Override + public void run() { + new Base(); + new Feature2Class(); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithOverlappingVisibilitiesTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithOverlappingVisibilitiesTest.java new file mode 100644 index 0000000..7dae8b6 --- /dev/null +++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClassesWithOverlappingVisibilitiesTest.java
@@ -0,0 +1,130 @@ +// Copyright (c) 2020, 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.classmerging.horizontal; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPackagePrivate; +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static com.android.tools.r8.utils.codeinspector.Matchers.isPublic; +import static com.android.tools.r8.utils.codeinspector.Matchers.notIf; +import static org.hamcrest.MatcherAssert.assertThat; + +import com.android.tools.r8.NeverClassInline; +import com.android.tools.r8.NeverInline; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.utils.codeinspector.ClassSubject; +import com.android.tools.r8.utils.codeinspector.MethodSubject; +import org.junit.Test; + +public class ClassesWithOverlappingVisibilitiesTest extends HorizontalClassMergingTestBase { + public ClassesWithOverlappingVisibilitiesTest( + TestParameters parameters, boolean enableHorizontalClassMerging) { + super(parameters, enableHorizontalClassMerging); + } + + @Test + public void testR8() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(getClass()) + .addKeepMainRule(Main.class) + .addOptionsModification( + options -> options.enableHorizontalClassMerging = enableHorizontalClassMerging) + .enableInliningAnnotations() + .enableNeverClassInliningAnnotations() + .setMinApi(parameters.getApiLevel()) + .run(parameters.getRuntime(), Main.class) + .assertSuccessWithOutputLines("foo A", "FOO B", "FOO C", "foo D", "FOO E") + .inspect( + codeInspector -> { + ClassSubject aClassSubject = codeInspector.clazz(A.class); + assertThat(aClassSubject, isPresent()); + MethodSubject methodSubject = aClassSubject.method("void", "foo"); + assertThat(methodSubject, isPackagePrivate()); + + ClassSubject bClassSubject = codeInspector.clazz(B.class); + assertThat(bClassSubject, isPresent()); + methodSubject = bClassSubject.method("void", "foo"); + assertThat(methodSubject, isPackagePrivate()); + + assertThat( + codeInspector.clazz(C.class), notIf(isPresent(), enableHorizontalClassMerging)); + + ClassSubject dClassSubject = codeInspector.clazz(D.class); + assertThat(dClassSubject, isPresent()); + methodSubject = dClassSubject.method("void", "foo"); + assertThat(methodSubject, isPublic()); + + ClassSubject eClassSubject = codeInspector.clazz(E.class); + assertThat(eClassSubject, notIf(isPresent(), enableHorizontalClassMerging)); + }); + } + + @NeverClassInline + public static class A { + @NeverInline + void foo() { + System.out.println("foo A"); + } + } + + @NeverClassInline + public static class B extends A { + public B() { + foo(); + } + + @Override + @NeverInline + void foo() { + System.out.println("FOO B"); + } + } + + // This class should be merged into B, as the method foo is package private both classes. + @NeverClassInline + public static class C extends A { + public C() { + foo(); + } + + @Override + @NeverInline + void foo() { + System.out.println("FOO C"); + } + } + + // This class can only be merged into E as D#foo is public, while A#foo is package private. + @NeverClassInline + public static class D { + @NeverInline + public void foo() { + System.out.println("foo D"); + } + } + + @NeverClassInline + public static class E { + public E() { + foo(); + } + + @NeverInline + public void foo() { + System.out.println("FOO E"); + } + } + + public static class Main { + public static void main(String[] args) { + A a = new A(); + a.foo(); + B b = new B(); + C c = new C(); + D d = new D(); + d.foo(); + E e = new E(); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/ServiceLoaderTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/ServiceLoaderTest.java new file mode 100644 index 0000000..ee6dc18 --- /dev/null +++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/ServiceLoaderTest.java
@@ -0,0 +1,76 @@ +// Copyright (c) 2020, 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.classmerging.horizontal; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static org.hamcrest.MatcherAssert.assertThat; + +import com.android.tools.r8.DataEntryResource; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.origin.Origin; +import com.android.tools.r8.utils.StringUtils; +import com.google.common.collect.Lists; +import java.util.List; +import java.util.ServiceLoader; +import org.junit.Test; + +public class ServiceLoaderTest extends HorizontalClassMergingTestBase { + public ServiceLoaderTest(TestParameters parameters, boolean enableHorizontalClassMerging) { + super(parameters, enableHorizontalClassMerging); + } + + @Test + public void testR8() throws Exception { + List<String> serviceImplementations = Lists.newArrayList(); + serviceImplementations.add(B.class.getTypeName()); + serviceImplementations.add(C.class.getTypeName()); + + testForR8(parameters.getBackend()) + .addInnerClasses(getClass()) + .addKeepMainRule(Main.class) + .addDataEntryResources( + DataEntryResource.fromBytes( + StringUtils.lines(serviceImplementations).getBytes(), + "META-INF/services/" + A.class.getTypeName(), + Origin.unknown())) + .addOptionsModification( + options -> options.enableHorizontalClassMerging = enableHorizontalClassMerging) + .setMinApi(parameters.getApiLevel()) + .run(parameters.getRuntime(), Main.class) + .assertSuccess() + .inspect( + codeInspector -> { + assertThat(codeInspector.clazz(A.class), isPresent()); + assertThat(codeInspector.clazz(B.class), isPresent()); + assertThat(codeInspector.clazz(C.class), isPresent()); + }); + } + + public interface A { + String foo(); + } + + public static class B implements A { + @Override + public String foo() { + return "B"; + } + } + + public static class C implements A { + @Override + public String foo() { + return "C"; + } + } + + public static class Main { + public static void main(String[] args) { + for (A a : ServiceLoader.load(A.class)) { + System.out.println(a.foo()); + } + } + } +}
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergingWithNonVisibleAnnotation.java b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergingWithNonVisibleAnnotation.java new file mode 100644 index 0000000..bb33801 --- /dev/null +++ b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergingWithNonVisibleAnnotation.java
@@ -0,0 +1,88 @@ +// Copyright (c) 2020, 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.classmerging.vertical; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; + +import com.android.tools.r8.NeverClassInline; +import com.android.tools.r8.NeverInline; +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; +import com.android.tools.r8.classmerging.vertical.testclasses.Outer; +import com.android.tools.r8.classmerging.vertical.testclasses.Outer.Base; +import com.android.tools.r8.shaking.ProguardKeepAttributes; +import com.android.tools.r8.utils.codeinspector.AnnotationSubject; +import com.android.tools.r8.utils.codeinspector.ClassSubject; +import com.android.tools.r8.utils.codeinspector.MethodSubject; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class VerticalClassMergingWithNonVisibleAnnotation extends TestBase { + + private final TestParameters parameters; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withAllRuntimesAndApiLevels().build(); + } + + public VerticalClassMergingWithNonVisibleAnnotation(TestParameters parameters) { + this.parameters = parameters; + } + + @Test + public void testR8() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(Outer.class) + .addProgramClasses(Sub.class) + .setMinApi(parameters.getApiLevel()) + .addKeepMainRule(Sub.class) + .addKeepClassRules(Outer.class.getPackage().getName() + ".Outer$Private* { *; }") + .addKeepAttributes(ProguardKeepAttributes.RUNTIME_VISIBLE_ANNOTATIONS) + .enableNeverClassInliningAnnotations() + .enableInliningAnnotations() + .run(parameters.getRuntime(), Sub.class) + .assertSuccessWithOutputLines("Base::foo()", "Sub::bar()") + .inspect( + codeInspector -> { + // Assert that base has been vertically merged into Sub. + assertThat(codeInspector.clazz(Base.class), not(isPresent())); + ClassSubject sub = codeInspector.clazz(Sub.class); + assertThat(sub, isPresent()); + // Assert that the merged class has no annotations from Base + assertTrue(sub.getDexProgramClass().annotations().isEmpty()); + // Assert that foo has the private annotation from the Base.foo + MethodSubject foo = sub.uniqueMethodWithName("foo"); + assertThat(foo, isPresent()); + AnnotationSubject privateMethodAnnotation = + foo.annotation( + Outer.class.getPackage().getName() + ".Outer$PrivateMethodAnnotation"); + assertThat(privateMethodAnnotation, isPresent()); + }); + } + + @NeverClassInline + public static class Sub extends Base { + + @Override + @NeverInline + public void bar() { + System.out.println("Sub::bar()"); + } + + public static void main(String[] args) { + Sub sub = new Sub(); + sub.foo(); + sub.bar(); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/testclasses/Outer.java b/src/test/java/com/android/tools/r8/classmerging/vertical/testclasses/Outer.java new file mode 100644 index 0000000..46ef8c8 --- /dev/null +++ b/src/test/java/com/android/tools/r8/classmerging/vertical/testclasses/Outer.java
@@ -0,0 +1,34 @@ +// Copyright (c) 2020, 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.classmerging.vertical.testclasses; + +import com.android.tools.r8.NeverInline; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +public class Outer { + + @Retention(RetentionPolicy.RUNTIME) + private @interface PrivateClassAnnotation {} + + @Retention(RetentionPolicy.RUNTIME) + private @interface PrivateMethodAnnotation { + Class<?> value(); + } + + private static class PrivateType {} + + @PrivateClassAnnotation + public abstract static class Base { + + @PrivateMethodAnnotation(PrivateType.class) + @NeverInline + public void foo() { + System.out.println("Base::foo()"); + } + + public abstract void bar(); + } +}
diff --git a/src/test/java/com/android/tools/r8/desugar/DesugarToClassFile.java b/src/test/java/com/android/tools/r8/desugar/DesugarToClassFile.java index 17e482f..c9377fe 100644 --- a/src/test/java/com/android/tools/r8/desugar/DesugarToClassFile.java +++ b/src/test/java/com/android/tools/r8/desugar/DesugarToClassFile.java
@@ -68,7 +68,7 @@ testForJvm() .addProgramFiles(jar) .run(parameters.getRuntime(), TestClass.class) - .assertSuccessWithOutputLines("Hello, world!", "I::foo"); + .assertSuccessWithOutputLines("Hello, world!", "I::foo", "J::bar", "42"); } else { assert parameters.getRuntime().isDex(); // Convert to DEX without desugaring. @@ -77,7 +77,7 @@ .setMinApi(parameters.getApiLevel()) .disableDesugaring() .run(parameters.getRuntime(), TestClass.class) - .assertSuccessWithOutputLines("Hello, world!", "I::foo"); + .assertSuccessWithOutputLines("Hello, world!", "I::foo", "J::bar", "42"); } } @@ -87,18 +87,41 @@ } } + interface J { + static void bar() { + System.out.println("J::bar"); + } + } + static class A implements I {} static class TestClass { public static void main(String[] args) { + lambda(); + defaultInterfaceMethod(); + staticInterfaceMethod(); + backport(); + } + + public static void lambda() { Runnable runnable = () -> { System.out.println("Hello, world!"); }; runnable.run(); + } + public static void defaultInterfaceMethod() { new A().foo(); } + + public static void staticInterfaceMethod() { + J.bar(); + } + + public static void backport() { + System.out.println(Long.hashCode(42)); + } } }
diff --git a/src/test/java/com/android/tools/r8/desugar/DesugarToClassFileBackport.java b/src/test/java/com/android/tools/r8/desugar/DesugarToClassFileBackport.java index e07089f..2297f3f 100644 --- a/src/test/java/com/android/tools/r8/desugar/DesugarToClassFileBackport.java +++ b/src/test/java/com/android/tools/r8/desugar/DesugarToClassFileBackport.java
@@ -64,7 +64,7 @@ MethodSubject methodSubject = inspector.clazz(TestClass.class).uniqueMethodWithName("main"); if (methodSubject.getProgramMethod().getDefinition().getCode().isCfCode()) { CfCode code = methodSubject.getProgramMethod().getDefinition().getCode().asCfCode(); - assertTrue(code.instructions.stream().noneMatch(this::isCfLAdd)); + assertTrue(code.getInstructions().stream().noneMatch(this::isCfLAdd)); } else { DexCode code = methodSubject.getProgramMethod().getDefinition().getCode().asDexCode(); assertTrue(Arrays.stream(code.instructions).noneMatch(this::isDexAddLong)); @@ -76,7 +76,7 @@ MethodSubject methodSubject = inspector.clazz(TestClass.class).uniqueMethodWithName("main"); if (methodSubject.getProgramMethod().getDefinition().getCode().isCfCode()) { CfCode code = methodSubject.getProgramMethod().getDefinition().getCode().asCfCode(); - assertTrue(code.instructions.stream().anyMatch(this::isCfLAdd)); + assertTrue(code.getInstructions().stream().anyMatch(this::isCfLAdd)); } else { DexCode code = methodSubject.getProgramMethod().getDefinition().getCode().asDexCode(); assertTrue(Arrays.stream(code.instructions).anyMatch(this::isDexAddLong));
diff --git a/src/test/java/com/android/tools/r8/desugar/backports/BackportDuplicationTest.java b/src/test/java/com/android/tools/r8/desugar/backports/BackportDuplicationTest.java index 2913fe5..31575df 100644 --- a/src/test/java/com/android/tools/r8/desugar/backports/BackportDuplicationTest.java +++ b/src/test/java/com/android/tools/r8/desugar/backports/BackportDuplicationTest.java
@@ -4,7 +4,6 @@ package com.android.tools.r8.desugar.backports; import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.hasItems; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; @@ -13,14 +12,20 @@ import com.android.tools.r8.TestParameters; import com.android.tools.r8.TestParametersCollection; import com.android.tools.r8.desugar.backports.AbstractBackportTest.MiniAssert; +import com.android.tools.r8.references.MethodReference; import com.android.tools.r8.synthesis.SyntheticItems; import com.android.tools.r8.utils.AndroidApiLevel; import com.android.tools.r8.utils.StringUtils; -import com.android.tools.r8.utils.codeinspector.ClassSubject; +import com.android.tools.r8.utils.SyntheticItemsTestUtils; import com.android.tools.r8.utils.codeinspector.CodeInspector; -import com.android.tools.r8.utils.codeinspector.FoundMethodSubject; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; +import com.google.common.collect.Sets.SetView; +import java.nio.file.Path; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import org.junit.Test; import org.junit.runner.RunWith; @@ -68,26 +73,86 @@ @Test public void testD8() throws Exception { - List<String> run1 = getClassesAfterD8CompileAndRun(); - List<String> run2 = getClassesAfterD8CompileAndRun(); - assertEquals("Non deterministic synthesis", run1, run2); - } - - private List<String> getClassesAfterD8CompileAndRun() throws Exception { - return testForD8(parameters.getBackend()) + testForD8(parameters.getBackend()) .addProgramClasses(CLASSES) .setMinApi(parameters.getApiLevel()) .run(parameters.getRuntime(), TestClass.class) .assertSuccessWithOutput(EXPECTED) .inspect(this::checkNoInternalSyntheticNames) - .inspect(this::checkExpectedOutput) - .inspector() - .allClasses() - .stream() - .filter(c -> !CLASS_TYPE_NAMES.contains(c.getFinalName())) - .flatMap(c -> c.allMethods().stream().map(m -> m.asMethodReference().toString())) - .sorted() - .collect(Collectors.toList()); + .inspect(this::checkExpectedSynthetics); + } + + @Test + public void testD8Merging() throws Exception { + boolean intermediate = true; + runD8Merging(intermediate); + } + + @Test + public void testD8MergingNonIntermediate() throws Exception { + boolean intermediate = false; + runD8Merging(intermediate); + } + + private void runD8Merging(boolean intermediate) throws Exception { + // Compile part 1 of the input (maybe intermediate) + Path out1 = + testForD8() + .addProgramClasses(User1.class) + .addClasspathClasses(CLASSES) + .setMinApi(parameters.getApiLevel()) + .setIntermediate(intermediate) + .compile() + .writeToZip(); + + // Compile part 2 of the input (maybe intermediate) + Path out2 = + testForD8() + .addProgramClasses(User2.class) + .addClasspathClasses(CLASSES) + .setMinApi(parameters.getApiLevel()) + .setIntermediate(intermediate) + .compile() + .writeToZip(); + + SetView<MethodReference> syntheticsInParts = + Sets.union( + getSyntheticMethods(new CodeInspector(out1)), + getSyntheticMethods(new CodeInspector(out2))); + + // Merge parts as an intermediate artifact. + // This will not merge synthetics regardless of the setting of intermediate. + Path out3 = temp.newFolder().toPath().resolve("out3.zip"); + testForD8() + .addProgramClasses(MiniAssert.class, TestClass.class) + .addProgramFiles(out1, out2) + .setMinApi(parameters.getApiLevel()) + .setIntermediate(true) + .compile() + .writeToZip(out3) + .run(parameters.getRuntime(), TestClass.class) + .assertSuccessWithOutput(EXPECTED) + .inspect(this::checkNoInternalSyntheticNames) + .inspect(inspector -> assertEquals(syntheticsInParts, getSyntheticMethods(inspector))); + + // Finally do a non-intermediate merge. + testForD8() + .addProgramFiles(out3) + .setMinApi(parameters.getApiLevel()) + .run(parameters.getRuntime(), TestClass.class) + .assertSuccessWithOutput(EXPECTED) + .inspect(this::checkNoInternalSyntheticNames) + .inspect( + inspector -> { + if (intermediate) { + // If all previous builds where intermediate then synthetics are merged. + checkExpectedSynthetics(inspector); + } else { + // Otherwise merging non-intermediate artifacts, synthetics will not be identified. + // Check that they are exactly as in the part inputs. + assertEquals(syntheticsInParts, getSyntheticMethods(inspector)); + } + }); } private void checkNoInternalSyntheticNames(CodeInspector inspector) { @@ -99,24 +164,28 @@ }); } - private void checkExpectedOutput(CodeInspector inspector) { - // TODO(b/158159959): Once synthetic methods can be grouped in classes this should become 1. - int expectedSynthesizedClasses = 3; - // Total number of synthetic methods should be 3 ({Boolean,Character,Long}.compare). - int expectedSynthesizedMethods = 3; - // Desugaring should add exactly one class with one desugared method. - assertEquals(expectedSynthesizedClasses, inspector.allClasses().size() - CLASSES.size()); - assertThat( - inspector.allClasses().stream() - .map(ClassSubject::getOriginalName) - .collect(Collectors.toList()), - hasItems(CLASS_TYPE_NAMES.toArray())); - List<FoundMethodSubject> methods = - inspector.allClasses().stream() - .filter(clazz -> !CLASS_TYPE_NAMES.contains(clazz.getOriginalName())) - .flatMap(clazz -> clazz.allMethods().stream()) - .collect(Collectors.toList()); - assertEquals(expectedSynthesizedMethods, methods.size()); + private Set<MethodReference> getSyntheticMethods(CodeInspector inspector) { + Set<MethodReference> methods = new HashSet<>(); + inspector.allClasses().stream() + .filter(c -> !CLASS_TYPE_NAMES.contains(c.getFinalName())) + .forEach(c -> c.allMethods().forEach(m -> methods.add(m.asMethodReference()))); + return methods; + } + + private void checkExpectedSynthetics(CodeInspector inspector) throws Exception { + // Hardcoded set of expected synthetics in a "final" build. This set could change if the + // compiler makes any changes to the naming, sorting or grouping of synthetics. It is hard-coded + // here to check that the compiler generates this deterministically for any single run or merge + // of intermediates. + Set<MethodReference> expectedSynthetics = + ImmutableSet.of( + SyntheticItemsTestUtils.syntheticMethod( + User1.class, 0, Character.class.getMethod("compare", char.class, char.class)), + SyntheticItemsTestUtils.syntheticMethod( + User1.class, 1, Boolean.class.getMethod("compare", boolean.class, boolean.class)), + SyntheticItemsTestUtils.syntheticMethod( + User2.class, 0, Integer.class.getMethod("compare", int.class, int.class))); + assertEquals(expectedSynthetics, getSyntheticMethods(inspector)); } static class User1 {
diff --git a/src/test/java/com/android/tools/r8/desugar/backports/BackportMainDexTest.java b/src/test/java/com/android/tools/r8/desugar/backports/BackportMainDexTest.java index 3be0a2c..424a48f 100644 --- a/src/test/java/com/android/tools/r8/desugar/backports/BackportMainDexTest.java +++ b/src/test/java/com/android/tools/r8/desugar/backports/BackportMainDexTest.java
@@ -5,7 +5,6 @@ import static com.android.tools.r8.synthesis.SyntheticItems.EXTERNAL_SYNTHETIC_CLASS_SEPARATOR; import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; -import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -15,19 +14,33 @@ import com.android.tools.r8.ByteDataView; import com.android.tools.r8.DexIndexedConsumer; import com.android.tools.r8.DiagnosticsHandler; +import com.android.tools.r8.GenerateMainDexListRunResult; import com.android.tools.r8.TestBase; import com.android.tools.r8.TestParameters; import com.android.tools.r8.TestParametersCollection; +import com.android.tools.r8.ToolHelper; import com.android.tools.r8.desugar.backports.AbstractBackportTest.MiniAssert; import com.android.tools.r8.origin.Origin; +import com.android.tools.r8.references.ClassReference; +import com.android.tools.r8.references.MethodReference; import com.android.tools.r8.references.Reference; import com.android.tools.r8.utils.AndroidApiLevel; import com.android.tools.r8.utils.AndroidApp; +import com.android.tools.r8.utils.ListUtils; import com.android.tools.r8.utils.StringUtils; +import com.android.tools.r8.utils.SyntheticItemsTestUtils; import com.android.tools.r8.utils.codeinspector.CodeInspector; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; +import com.google.common.collect.Streams; +import java.nio.file.Path; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -40,6 +53,9 @@ static final List<Class<?>> CLASSES = ImmutableList.of(MiniAssert.class, TestClass.class, User1.class, User2.class); + static final List<Class<?>> MAIN_DEX_LIST_CLASSES = + ImmutableList.of(MiniAssert.class, TestClass.class, User2.class); + static final String SyntheticUnderUser1 = User1.class.getTypeName() + EXTERNAL_SYNTHETIC_CLASS_SEPARATOR; static final String SyntheticUnderUser2 = @@ -78,15 +94,50 @@ .assertSuccessWithOutput(EXPECTED); } + private GenerateMainDexListRunResult traceMainDex( + Collection<Class<?>> classes, Collection<Path> files) throws Exception { + return testForMainDexListGenerator() + .addProgramClasses(classes) + .addProgramFiles(files) + .addLibraryFiles(ToolHelper.getFirstSupportedAndroidJar(parameters.getApiLevel())) + .addMainDexRules(keepMainProguardConfiguration(TestClass.class)) + .run(); + } + + @Test + public void testMainDexTracingCf() throws Exception { + assumeTrue(parameters.isDexRuntime()); + GenerateMainDexListRunResult mainDexListFromCf = traceMainDex(CLASSES, Collections.emptyList()); + assertEquals( + ListUtils.map(MAIN_DEX_LIST_CLASSES, Reference::classFromClass), + mainDexListFromCf.getMainDexList()); + } + + @Test + public void testMainDexTracingDex() throws Exception { + assumeTrue(parameters.isDexRuntime()); + Path out = + testForD8() + .addProgramClasses(CLASSES) + // Setting intermediate will annotate synthetics, which should not cause types in those + // to become main-dex included. + .setIntermediate(true) + .setMinApi(parameters.getApiLevel()) + .compile() + .writeToZip(); + GenerateMainDexListRunResult mainDexListFromDex = + traceMainDex(Collections.emptyList(), Collections.singleton(out)); + assertEquals( + Streams.concat( + MAIN_DEX_LIST_CLASSES.stream().map(Reference::classFromClass), + getMainDexExpectedSynthetics().stream().map(MethodReference::getHolderClass)) + .collect(Collectors.toSet()), + ImmutableSet.copyOf(mainDexListFromDex.getMainDexList())); + } + @Test public void testD8() throws Exception { assumeTrue(parameters.isDexRuntime()); - Set<String> mainDex1 = runD8(); - Set<String> mainDex2 = runD8(); - assertEquals("Expected deterministic main-dex lists", mainDex1, mainDex2); - } - - private Set<String> runD8() throws Exception { MainDexConsumer mainDexConsumer = new MainDexConsumer(); testForD8(parameters.getBackend()) .addProgramClasses(CLASSES) @@ -94,39 +145,89 @@ .addMainDexListClasses(MiniAssert.class, TestClass.class, User2.class) .setProgramConsumer(mainDexConsumer) .compile() - .inspect( - inspector -> { - // Note: This will change if we group methods in classes, in which case we should - // preferable not put non-main-dex referenced methods in the main-dex group. - // User1 has two synthetics, one shared with User2 and one for self. - assertEquals( - 2, - inspector.allClasses().stream() - .filter(c -> c.getFinalName().startsWith(SyntheticUnderUser1)) - .count()); - // User2 has one synthetic as the shared call is placed in User1. - assertEquals( - 1, - inspector.allClasses().stream() - .filter(c -> c.getFinalName().startsWith(SyntheticUnderUser2)) - .count()); - }) + .inspect(this::checkExpectedSynthetics) .run(parameters.getRuntime(), TestClass.class, getRunArgs()) .assertSuccessWithOutput(EXPECTED); checkMainDex(mainDexConsumer); - return mainDexConsumer.mainDexDescriptors; + } + + // TODO(b/168584485): This test should be removed once support is dropped. + @Test + public void testD8MergingWithTraceCf() throws Exception { + assumeTrue(parameters.isDexRuntime()); + Path out1 = + testForD8() + .addProgramClasses(User1.class) + .addClasspathClasses(CLASSES) + .setIntermediate(true) + .setMinApi(parameters.getApiLevel()) + .compile() + .writeToZip(); + + Path out2 = + testForD8() + .addProgramClasses(User2.class) + .addClasspathClasses(CLASSES) + .setIntermediate(true) + .setMinApi(parameters.getApiLevel()) + .compile() + .writeToZip(); + + MainDexConsumer mainDexConsumer = new MainDexConsumer(); + testForD8(parameters.getBackend()) + .addProgramClasses(TestClass.class, MiniAssert.class) + .addProgramFiles(out1, out2) + .setMinApi(parameters.getApiLevel()) + .addMainDexListClassReferences( + traceMainDex(CLASSES, Collections.emptyList()).getMainDexList()) + .setProgramConsumer(mainDexConsumer) + .compile() + .inspect(this::checkExpectedSynthetics) + .run(parameters.getRuntime(), TestClass.class, getRunArgs()) + .assertSuccessWithOutput(EXPECTED); + checkMainDex(mainDexConsumer); + } + + @Test + public void testD8MergingWithTraceDex() throws Exception { + assumeTrue(parameters.isDexRuntime()); + Path out1 = + testForD8() + .addProgramClasses(User1.class) + .addClasspathClasses(CLASSES) + .setIntermediate(true) + .setMinApi(parameters.getApiLevel()) + .compile() + .writeToZip(); + + Path out2 = + testForD8() + .addProgramClasses(User2.class) + .addClasspathClasses(CLASSES) + .setIntermediate(true) + .setMinApi(parameters.getApiLevel()) + .compile() + .writeToZip(); + + MainDexConsumer mainDexConsumer = new MainDexConsumer(); + List<Class<?>> classes = ImmutableList.of(TestClass.class, MiniAssert.class); + List<Path> files = ImmutableList.of(out1, out2); + GenerateMainDexListRunResult traceResult = traceMainDex(classes, files); + testForD8(parameters.getBackend()) + .addProgramClasses(classes) + .addProgramFiles(files) + .setMinApi(parameters.getApiLevel()) + .addMainDexListClassReferences(traceResult.getMainDexList()) + .setProgramConsumer(mainDexConsumer) + .compile() + .inspect(this::checkExpectedSynthetics) + .run(parameters.getRuntime(), TestClass.class, getRunArgs()) + .assertSuccessWithOutput(EXPECTED); + checkMainDex(mainDexConsumer); } @Test public void testR8() throws Exception { - Set<String> mainDex1 = runR8(); - if (parameters.isDexRuntime()) { - Set<String> mainDex2 = runR8(); - assertEquals(mainDex1, mainDex2); - } - } - - private Set<String> runR8() throws Exception { MainDexConsumer mainDexConsumer = parameters.isDexRuntime() ? new MainDexConsumer() : null; testForR8(parameters.getBackend()) .debug() // Use debug mode to force a minimal main dex. @@ -135,18 +236,17 @@ .addProgramClasses(CLASSES) .addKeepMainRule(TestClass.class) .addKeepClassAndMembersRules(MiniAssert.class) - .addMainDexClassRules(MiniAssert.class, TestClass.class) .addKeepMethodRules( Reference.methodFromMethod(User1.class.getMethod("testBooleanCompare")), Reference.methodFromMethod(User1.class.getMethod("testCharacterCompare"))) + .addMainDexRules(keepMainProguardConfiguration(TestClass.class)) .setMinApi(parameters.getApiLevel()) .run(parameters.getRuntime(), TestClass.class, getRunArgs()) - .assertSuccessWithOutput(EXPECTED); + .assertSuccessWithOutput(EXPECTED) + .inspect(this::checkExpectedSynthetics); if (mainDexConsumer != null) { checkMainDex(mainDexConsumer); - return mainDexConsumer.mainDexDescriptors; } - return null; } private void checkMainDex(MainDexConsumer mainDexConsumer) throws Exception { @@ -160,22 +260,49 @@ assertThat(mainDexInspector.clazz(MiniAssert.class), isPresent()); assertThat(mainDexInspector.clazz(TestClass.class), isPresent()); assertThat(mainDexInspector.clazz(User2.class), isPresent()); + assertEquals(getMainDexExpectedSynthetics(), getSyntheticMethods(mainDexInspector)); + } - // At least one synthetic class placed under User2 must be included in the main-dex file. - assertEquals( - 1, - mainDexInspector.allClasses().stream() - .filter(c -> c.getFinalName().startsWith(SyntheticUnderUser2)) - .count()); + private Set<MethodReference> getSyntheticMethods(CodeInspector inspector) { + Set<ClassReference> nonSyntheticCLasses = + CLASSES.stream().map(Reference::classFromClass).collect(Collectors.toSet()); + Set<MethodReference> methods = new HashSet<>(); + inspector.allClasses().stream() + .filter(c -> !nonSyntheticCLasses.contains(c.getFinalReference())) + .forEach(c -> c.allMethods().forEach(m -> methods.add(m.asMethodReference()))); + return methods; + } - // Minimal main dex should only include one of the User1 synthetics. - assertThat(mainDexInspector.clazz(User1.class), not(isPresent())); - assertEquals( - 1, - mainDexInspector.allClasses().stream() - .filter(c -> c.getFinalName().startsWith(SyntheticUnderUser1)) - .count()); - assertThat(mainDexInspector.clazz(SyntheticUnderUser1), not(isPresent())); + private void checkExpectedSynthetics(CodeInspector inspector) throws Exception { + if (parameters.getApiLevel() == null) { + assertEquals(Collections.emptySet(), getSyntheticMethods(inspector)); + } else { + assertEquals( + Sets.union(getMainDexExpectedSynthetics(), getNonMainDexExpectedSynthetics()), + getSyntheticMethods(inspector)); + } + } + + // Hardcoded set of expected synthetics in a "final" build. This set could change if the + // compiler makes any changes to the naming, sorting or grouping of synthetics. It is hard-coded + // here to + // check that the compiler generates this deterministically for any single run or merge of + // intermediates. + + private ImmutableSet<MethodReference> getNonMainDexExpectedSynthetics() + throws NoSuchMethodException { + return ImmutableSet.of( + SyntheticItemsTestUtils.syntheticMethod( + User1.class, 1, Boolean.class.getMethod("compare", boolean.class, boolean.class))); + } + + private ImmutableSet<MethodReference> getMainDexExpectedSynthetics() + throws NoSuchMethodException { + return ImmutableSet.of( + SyntheticItemsTestUtils.syntheticMethod( + User1.class, 0, Character.class.getMethod("compare", char.class, char.class)), + SyntheticItemsTestUtils.syntheticMethod( + User2.class, 0, Integer.class.getMethod("compare", int.class, int.class))); } static class User1 {
diff --git a/src/test/java/com/android/tools/r8/desugar/backports/DesugarStaticBackportsOnly.java b/src/test/java/com/android/tools/r8/desugar/backports/DesugarStaticBackportsOnly.java deleted file mode 100644 index 77e088c..0000000 --- a/src/test/java/com/android/tools/r8/desugar/backports/DesugarStaticBackportsOnly.java +++ /dev/null
@@ -1,137 +0,0 @@ -// Copyright (c) 2020, 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.desugar.backports; - -import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.core.StringContains.containsString; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -import com.android.tools.r8.CompilationFailedException; -import com.android.tools.r8.D8TestBuilder; -import com.android.tools.r8.Diagnostic; -import com.android.tools.r8.TestBase; -import com.android.tools.r8.TestParameters; -import com.android.tools.r8.TestParametersCollection; -import com.android.tools.r8.synthesis.SyntheticItems; -import com.android.tools.r8.utils.AndroidApiLevel; -import com.android.tools.r8.utils.InternalOptions.DesugarState; -import com.android.tools.r8.utils.StringUtils; -import com.android.tools.r8.utils.codeinspector.ClassSubject; -import com.android.tools.r8.utils.codeinspector.CodeInspector; -import com.android.tools.r8.utils.codeinspector.DexInstructionSubject; -import java.util.Arrays; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; - -@RunWith(Parameterized.class) -public class DesugarStaticBackportsOnly extends TestBase { - - private final TestParameters parameters; - - @Parameterized.Parameters(name = "{0}") - public static TestParametersCollection data() { - return getTestParameters().withDexRuntimes().withAllApiLevels().build(); - } - - public DesugarStaticBackportsOnly(TestParameters parameters) { - this.parameters = parameters; - } - - private void checkLongHashCodeDesugared(CodeInspector inspector) { - ClassSubject classSubject = inspector.clazz(TestClass.class); - assertThat(classSubject, isPresent()); - assertEquals( - parameters.getApiLevel().isLessThan(AndroidApiLevel.N), - classSubject - .uniqueMethodWithName("main") - .streamInstructions() - .anyMatch( - instructionSubject -> - instructionSubject.isInvokeStatic() - && instructionSubject - .toString() - .contains(SyntheticItems.EXTERNAL_SYNTHETIC_CLASS_SEPARATOR))); - assertEquals( - parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.N), - classSubject - .uniqueMethodWithName("main") - .streamInstructions() - .anyMatch( - instructionSubject -> - instructionSubject.isInvokeStatic() - && instructionSubject.toString().contains("java/lang/Long"))); - } - - @Test - public void testBackportDesugared() throws Exception { - String expectedOutput = StringUtils.lines("1234"); - testForD8() - .addProgramClasses(TestClass.class) - .setMinApi(parameters.getApiLevel()) - .addOptionsModification( - options -> options.desugarState = DesugarState.ONLY_BACKPORT_STATICS) - .compile() - .inspect(this::checkLongHashCodeDesugared) - .run(parameters.getRuntime(), TestClass.class) - .assertSuccessWithOutput(expectedOutput); - } - - private void checkLambdaNotDesugared(CodeInspector inspector) { - ClassSubject classSubject = inspector.clazz(TestClassLambda.class); - assertThat(classSubject, isPresent()); - assertTrue( - classSubject - .uniqueMethodWithName("main") - .streamInstructions() - .anyMatch( - instructionSubject -> - ((DexInstructionSubject) instructionSubject).isInvokeCustom())); - } - - @Test - public void testLambdaNotDesugared() throws Exception { - D8TestBuilder builder = - testForD8() - .addProgramClasses(TestClassLambda.class) - .setMinApi(parameters.getApiLevel()) - .addOptionsModification( - options -> options.desugarState = DesugarState.ONLY_BACKPORT_STATICS); - if (parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.O)) { - builder.compile().inspect(this::checkLambdaNotDesugared); - } else { - try { - builder.compileWithExpectedDiagnostics( - diagnostics -> { - diagnostics.assertOnlyErrors(); - diagnostics.assertErrorsCount(1); - Diagnostic diagnostic = diagnostics.getErrors().get(0); - assertThat( - diagnostic.getDiagnosticMessage(), - containsString("Invoke-customs are only supported starting with Android O")); - }); - } catch (CompilationFailedException e) { - // Expected compilation failed. - return; - } - fail("Expected test to fail with CompilationFailedException"); - } - } - - static class TestClass { - public static void main(String[] args) { - System.out.println(Long.hashCode(1234)); - } - } - - static class TestClassLambda { - public static void main(String[] args) { - Arrays.asList(args).forEach(s -> System.out.println(s)); - } - } -}
diff --git a/src/test/java/com/android/tools/r8/enumunboxing/PhiEnumUnboxingTest.java b/src/test/java/com/android/tools/r8/enumunboxing/PhiEnumUnboxingTest.java index 8aa2d5e..7a0c8bb 100644 --- a/src/test/java/com/android/tools/r8/enumunboxing/PhiEnumUnboxingTest.java +++ b/src/test/java/com/android/tools/r8/enumunboxing/PhiEnumUnboxingTest.java
@@ -17,8 +17,6 @@ @RunWith(Parameterized.class) public class PhiEnumUnboxingTest extends EnumUnboxingTestBase { - private static final Class<?> ENUM_CLASS = MyEnum.class; - private final TestParameters parameters; private final boolean enumValueOptimization; private final EnumKeepRules enumKeepRules; @@ -37,11 +35,10 @@ @Test public void testEnumUnboxing() throws Exception { - Class<?> classToTest = Phi.class; R8TestRunResult run = testForR8(parameters.getBackend()) - .addProgramClasses(classToTest, ENUM_CLASS) - .addKeepMainRule(classToTest) + .addProgramClasses(Phi.class, MyEnum.class) + .addKeepMainRule(Phi.class) .addKeepRules(enumKeepRules.getKeepRules()) .enableInliningAnnotations() .enableNeverClassInliningAnnotations() @@ -50,8 +47,8 @@ .setMinApi(parameters.getApiLevel()) .compile() .inspectDiagnosticMessages( - m -> assertEnumIsUnboxed(ENUM_CLASS, classToTest.getSimpleName(), m)) - .run(parameters.getRuntime(), classToTest) + m -> assertEnumIsUnboxed(MyEnum.class, Phi.class.getSimpleName(), m)) + .run(parameters.getRuntime(), Phi.class) .assertSuccess(); assertLines2By2Correct(run.getStdOut()); } @@ -66,23 +63,54 @@ static class Phi { public static void main(String[] args) { + nonNullTest(); + nullTest(); + } + + private static void nonNullTest() { System.out.println(switchOn(1).ordinal()); System.out.println(1); System.out.println(switchOn(2).ordinal()); System.out.println(2); } - // Avoid removing the switch entirely. + private static void nullTest() { + System.out.println(switchOnWithNull(1).ordinal()); + System.out.println(1); + System.out.println(switchOnWithNull(2) == null); + System.out.println(true); + } + @NeverInline static MyEnum switchOn(int i) { + MyEnum returnValue; switch (i) { case 0: - return MyEnum.A; + returnValue = MyEnum.A; + break; case 1: - return MyEnum.B; + returnValue = MyEnum.B; + break; default: - return MyEnum.C; + returnValue = MyEnum.C; } + return returnValue; + } + + @NeverInline + static MyEnum switchOnWithNull(int i) { + MyEnum returnValue; + switch (i) { + case 0: + returnValue = MyEnum.A; + break; + case 1: + returnValue = MyEnum.B; + break; + default: + returnValue = null; + } + return returnValue; } } }
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/SubsumedCatchHandlerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/SubsumedCatchHandlerTest.java index dc4ed0e..87de375 100644 --- a/src/test/java/com/android/tools/r8/ir/optimize/SubsumedCatchHandlerTest.java +++ b/src/test/java/com/android/tools/r8/ir/optimize/SubsumedCatchHandlerTest.java
@@ -104,7 +104,7 @@ TryHandler handler = dexCode.handlers[0]; assertEquals(1, handler.pairs.length); - DexType guard = handler.pairs[0].type; + DexType guard = handler.pairs[0].getType(); assertEquals("java.lang.Exception", guard.toSourceString()); } else { assert code.isCfCode();
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java index d1ecf46..6ef399e 100644 --- a/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java +++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexTracingTest.java
@@ -4,6 +4,7 @@ package com.android.tools.r8.maindexlist; +import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage; import static com.android.tools.r8.utils.FileUtils.JAR_EXTENSION; import static com.android.tools.r8.utils.FileUtils.ZIP_EXTENSION; import static com.android.tools.r8.utils.FileUtils.withNativeFileSeparators; @@ -20,6 +21,7 @@ import com.android.tools.r8.R8FullTestBuilder; import com.android.tools.r8.TestBase; import com.android.tools.r8.TestCompilerBuilder; +import com.android.tools.r8.TestParameters; import com.android.tools.r8.ThrowableConsumer; import com.android.tools.r8.ToolHelper; import com.android.tools.r8.ir.desugar.LambdaRewriter; @@ -46,7 +48,11 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +@RunWith(Parameterized.class) public class MainDexTracingTest extends TestBase { private static final String EXAMPLE_BUILD_DIR = ToolHelper.EXAMPLES_BUILD_DIR; @@ -54,6 +60,31 @@ private static final String EXAMPLE_SRC_DIR = ToolHelper.EXAMPLES_DIR; private static final String EXAMPLE_O_SRC_DIR = ToolHelper.EXAMPLES_ANDROID_O_DIR; + @Parameters(name = "{0}, {1}") + public static List<Object[]> data() { + return buildParameters(getTestParameters().withNoneRuntime().build(), Backend.values()); + } + + private final Backend backend; + + public MainDexTracingTest(TestParameters parameters, Backend backend) { + parameters.assertNoneRuntime(); + this.backend = backend; + } + + private Path getInputJar(Path cfJar) throws Exception { + if (backend == Backend.CF) { + return cfJar; + } + return testForD8() + .setIntermediate(true) + .addProgramFiles(cfJar) + .setMinApi(AndroidApiLevel.K) + .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.K)) + .compile() + .writeToZip(); + } + @Test public void traceMainDexList001_whyareyoukeeping() throws Throwable { PrintStream stdout = System.out; @@ -294,7 +325,7 @@ throws Throwable { Path out = temp.getRoot().toPath().resolve(testName + ZIP_EXTENSION); - Path inputJar = Paths.get(buildDir, packageName + JAR_EXTENSION); + Path inputJar = getInputJar(Paths.get(buildDir, packageName + JAR_EXTENSION)); // Build main-dex list using GenerateMainDexList and test the output from run. GenerateMainDexListCommand.Builder mdlCommandBuilder = GenerateMainDexListCommand.builder(); GenerateMainDexListCommand mdlCommand = mdlCommandBuilder @@ -335,14 +366,22 @@ .addKeepRules("-keepattributes *Annotation*") .addMainDexRuleFiles(mainDexRules) .apply(configuration) - .allowDiagnosticWarningMessages() .assumeAllMethodsMayHaveSideEffects() .setMinApi(minSdk) .noMinification() .noTreeShaking() .setMainDexListConsumer(ToolHelper.consumeString(r8MainDexListOutput::set)) - .compile() - .assertAllWarningMessagesMatch(equalTo("Resource 'META-INF/MANIFEST.MF' already exists.")) + .allowDiagnosticMessages() + .compileWithExpectedDiagnostics( + diagnostics -> { + diagnostics.assertNoInfos().assertNoErrors(); + if (backend == Backend.CF) { + diagnostics.assertWarningsMatch( + diagnosticMessage(equalTo("Resource 'META-INF/MANIFEST.MF' already exists."))); + } else { + diagnostics.assertNoWarnings(); + } + }) .writeToZip(out); List<String> r8MainDexList = @@ -371,9 +410,16 @@ if (mainDexGeneratorMainDexList.size() <= i - nonLambdaOffset) { fail("Main dex list generator is missing '" + reference + "'"); } - checkSameMainDexEntry(reference, mainDexGeneratorMainDexList.get(i - nonLambdaOffset)); - checkSameMainDexEntry( - reference, mainDexGeneratorMainDexListFromConsumer.get(i - nonLambdaOffset)); + String fromList = mainDexGeneratorMainDexList.get(i - nonLambdaOffset); + String fromConsumer = mainDexGeneratorMainDexListFromConsumer.get(i - nonLambdaOffset); + if (isLambda(fromList)) { + assertEquals(Backend.DEX, backend); + assertEquals(fromList, fromConsumer); + nonLambdaOffset--; + } else { + checkSameMainDexEntry(reference, fromList); + checkSameMainDexEntry(reference, fromConsumer); + } } else { nonLambdaOffset++; }
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageWithCatchHandlerTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageWithCatchHandlerTest.java new file mode 100644 index 0000000..3afa4c2 --- /dev/null +++ b/src/test/java/com/android/tools/r8/repackage/RepackageWithCatchHandlerTest.java
@@ -0,0 +1,66 @@ +// Copyright (c) 2020, 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.repackage; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static org.hamcrest.MatcherAssert.assertThat; + +import com.android.tools.r8.NeverInline; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.utils.codeinspector.CodeInspector; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class RepackageWithCatchHandlerTest extends RepackageTestBase { + + public RepackageWithCatchHandlerTest( + String flattenPackageHierarchyOrRepackageClasses, TestParameters parameters) { + super(flattenPackageHierarchyOrRepackageClasses, parameters); + } + + @Test + public void test() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(getClass()) + .addKeepMainRule(TestClass.class) + .apply(this::configureRepackaging) + .enableInliningAnnotations() + .setMinApi(parameters.getApiLevel()) + .compile() + .inspect(this::inspect) + .run(parameters.getRuntime(), TestClass.class) + .assertSuccessWithOutputLines("Hello world!"); + } + + private void inspect(CodeInspector inspector) { + assertThat(inspector.clazz(MyException.class), isPresent()); + } + + public static class TestClass { + + public static void main(String[] args) { + try { + raise(); + } catch (MyException e) { + e.greet(); + } + } + + @NeverInline + static void raise() throws MyException { + throw new MyException(); + } + } + + public static class MyException extends Exception { + + @NeverInline + public static void greet() { + System.out.println("Hello world!"); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageWithDexItemBasedConstStringTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageWithDexItemBasedConstStringTest.java new file mode 100644 index 0000000..8a36090 --- /dev/null +++ b/src/test/java/com/android/tools/r8/repackage/RepackageWithDexItemBasedConstStringTest.java
@@ -0,0 +1,75 @@ +// Copyright (c) 2020, 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.repackage; + +import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; + +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.utils.codeinspector.ClassSubject; +import com.android.tools.r8.utils.codeinspector.CodeInspector; +import com.android.tools.r8.utils.codeinspector.InstructionSubject.JumboStringMode; +import com.android.tools.r8.utils.codeinspector.MethodSubject; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class RepackageWithDexItemBasedConstStringTest extends RepackageTestBase { + + public RepackageWithDexItemBasedConstStringTest( + String flattenPackageHierarchyOrRepackageClasses, TestParameters parameters) { + super(flattenPackageHierarchyOrRepackageClasses, parameters); + } + + @Test + public void test() throws Exception { + testForR8(parameters.getBackend()) + .addInnerClasses(getClass()) + .addKeepMainRule(TestClass.class) + .apply(this::configureRepackaging) + .setMinApi(parameters.getApiLevel()) + .compile() + .inspect(this::inspect) + .run(parameters.getRuntime(), TestClass.class) + .assertSuccessWithOutputLines("Hello world!"); + } + + private void inspect(CodeInspector inspector) { + ClassSubject testClassSubject = inspector.clazz(TestClass.class); + assertThat(testClassSubject, isPresent()); + + ClassSubject aClassSubject = inspector.clazz(A.class); + assertThat(aClassSubject, isPresent()); + + MethodSubject mainMethodSubject = testClassSubject.mainMethod(); + assertThat(mainMethodSubject, isPresent()); + assertTrue( + mainMethodSubject + .streamInstructions() + .anyMatch(x -> x.isConstString(aClassSubject.getFinalName(), JumboStringMode.ALLOW))); + } + + public static class TestClass { + + public static void main(String[] args) throws Exception { + Class.forName("com.android.tools.r8.repackage.RepackageWithDexItemBasedConstStringTest$A") + .getConstructor() + .newInstance(); + } + } + + public static class A { + + static { + System.out.print("Hello"); + } + + public A() { + System.out.println(" world!"); + } + } +}
diff --git a/src/test/java/com/android/tools/r8/shaking/examples/InliningClassVersionTest.java b/src/test/java/com/android/tools/r8/shaking/examples/InliningClassVersionTest.java index e47222b..5615c6d 100644 --- a/src/test/java/com/android/tools/r8/shaking/examples/InliningClassVersionTest.java +++ b/src/test/java/com/android/tools/r8/shaking/examples/InliningClassVersionTest.java
@@ -7,24 +7,37 @@ import static org.junit.Assert.assertNotEquals; import com.android.tools.r8.ArchiveClassFileProvider; -import com.android.tools.r8.ByteDataView; -import com.android.tools.r8.ClassFileConsumer; import com.android.tools.r8.NeverPropagateValue; import com.android.tools.r8.TestBase; -import com.android.tools.r8.ToolHelper; -import com.android.tools.r8.ToolHelper.ProcessResult; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.TestParametersCollection; import com.android.tools.r8.utils.DescriptorUtils; import com.android.tools.r8.utils.InternalOptions; import com.google.common.io.ByteStreams; import java.nio.file.Path; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassVisitor; -import org.objectweb.asm.ClassWriter; import org.objectweb.asm.Opcodes; +@RunWith(Parameterized.class) public class InliningClassVersionTest extends TestBase { + private final TestParameters parameters; + private static final String EXPECTED = "Hello from Inlinee!"; + + @Parameters(name = "{0}") + public static TestParametersCollection data() { + return getTestParameters().withCfRuntimes().build(); + } + + public InliningClassVersionTest(TestParameters parameters) { + this.parameters = parameters; + } + private final int OLD_VERSION = Opcodes.V1_6; private final String BASE_DESCRIPTOR = DescriptorUtils.javaTypeToDescriptor(Base.class.getName()); @@ -43,59 +56,44 @@ } } - private static class DowngradeVisitor extends ClassVisitor { - - private final int version; - - DowngradeVisitor(ClassVisitor cv, int version) { - super(InternalOptions.ASM_VERSION, cv); - this.version = version; - } - - @Override - public void visit( - int version, - int access, - String name, - String signature, - String superName, - String[] interfaces) { - assert version > this.version - : "Going from " + version + " to " + this.version + " is not a downgrade"; - super.visit(this.version, access, name, signature, superName, interfaces); - } - } - - private static byte[] downgradeClass(byte[] classBytes, int version) { - ClassWriter writer = new ClassWriter(0); - new ClassReader(classBytes).accept(new DowngradeVisitor(writer, version), 0); - return writer.toByteArray(); + @Test + public void testRuntime() throws Exception { + testForRuntime(parameters) + .addProgramClasses(Inlinee.class) + .addProgramClassFileData(downgradeBaseClass()) + .run(parameters.getRuntime(), Base.class) + .assertSuccessWithOutputLines(EXPECTED); } @Test public void test() throws Exception { - Path inputJar = writeInput(); - assertEquals(OLD_VERSION, getBaseClassVersion(inputJar)); - ProcessResult runInput = run(inputJar); - assertEquals(0, runInput.exitCode); - Path outputJar = - testForR8(Backend.CF) - .addProgramFiles(inputJar) - .addKeepMainRule(Base.class) - .enableMemberValuePropagationAnnotations() - .compile() - .writeToZip(); - ProcessResult runOutput = run(outputJar); - assertEquals(runInput.toString(), runOutput.toString()); + Path outputJar = temp.newFile("output.jar").toPath(); + testForR8(parameters.getBackend()) + .addProgramClasses(Inlinee.class) + .addProgramClassFileData(downgradeBaseClass()) + .addKeepMainRule(Base.class) + .enableMemberValuePropagationAnnotations() + .compile() + .writeToZip(outputJar) + .run(parameters.getRuntime(), Base.class) + .assertSuccessWithOutputLines(EXPECTED); assertNotEquals( "Inliner did not upgrade classfile version", OLD_VERSION, getBaseClassVersion(outputJar)); } - private int getBaseClassVersion(Path jar) throws Exception { - return getClassVersion(jar, BASE_DESCRIPTOR); + private byte[] downgradeBaseClass() throws Exception { + byte[] transform = transformer(Base.class).setVersion(OLD_VERSION).transform(); + assertEquals(OLD_VERSION, getClassVersion(transform)); + return transform; } - private int getClassVersion(Path jar, String descriptor) throws Exception { + private int getBaseClassVersion(Path jar) throws Exception { + return getClassVersion( + ByteStreams.toByteArray( + new ArchiveClassFileProvider(jar).getProgramResource(BASE_DESCRIPTOR).getByteStream())); + } + + private int getClassVersion(byte[] classFileBytes) { class ClassVersionReader extends ClassVisitor { private int version = -1; @@ -117,32 +115,9 @@ this.version = version; } } - - byte[] bytes = - ByteStreams.toByteArray( - new ArchiveClassFileProvider(jar).getProgramResource(descriptor).getByteStream()); ClassVersionReader reader = new ClassVersionReader(); - new ClassReader(bytes).accept(reader, 0); + new ClassReader(classFileBytes).accept(reader, 0); assert reader.version != -1; return reader.version; } - - private Path writeInput() throws Exception { - Path inputJar = temp.getRoot().toPath().resolve("input.jar"); - ClassFileConsumer consumer = new ClassFileConsumer.ArchiveConsumer(inputJar); - consumer.accept( - ByteDataView.of(downgradeClass(ToolHelper.getClassAsBytes(Base.class), OLD_VERSION)), - BASE_DESCRIPTOR, - null); - consumer.accept( - ByteDataView.of(ToolHelper.getClassAsBytes(Inlinee.class)), - DescriptorUtils.javaTypeToDescriptor(Inlinee.class.getName()), - null); - consumer.finished(null); - return inputJar; - } - - private ProcessResult run(Path jar) throws Exception { - return ToolHelper.runJava(jar, Base.class.getName()); - } }
diff --git a/src/test/java/com/android/tools/r8/utils/SyntheticItemsTestUtils.java b/src/test/java/com/android/tools/r8/utils/SyntheticItemsTestUtils.java new file mode 100644 index 0000000..9e78bac --- /dev/null +++ b/src/test/java/com/android/tools/r8/utils/SyntheticItemsTestUtils.java
@@ -0,0 +1,27 @@ +// Copyright (c) 2020, 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.utils; + +import com.android.tools.r8.references.ClassReference; +import com.android.tools.r8.references.MethodReference; +import com.android.tools.r8.references.Reference; +import com.android.tools.r8.synthesis.SyntheticItems; +import java.lang.reflect.Method; + +public class SyntheticItemsTestUtils { + + public static ClassReference syntheticClass(Class<?> clazz, int id) { + return Reference.classFromTypeName( + clazz.getTypeName() + SyntheticItems.EXTERNAL_SYNTHETIC_CLASS_SEPARATOR + id); + } + + public static MethodReference syntheticMethod(Class<?> clazz, int id, Method method) { + ClassReference syntheticHolder = syntheticClass(clazz, id); + MethodReference originalMethod = Reference.methodFromMethod(method); + return Reference.methodFromDescriptor( + syntheticHolder.getDescriptor(), + SyntheticItems.INTERNAL_SYNTHETIC_METHOD_PREFIX + 0, + originalMethod.getMethodDescriptor()); + } +}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java index fe4d4d9..978db99 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/AbsentClassSubject.java
@@ -111,6 +111,11 @@ } @Override + public ClassReference getFinalReference() { + return null; + } + + @Override public String getFinalName() { return null; }
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CfTryCatchSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CfTryCatchSubject.java index d6bac64..5853fdd 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/CfTryCatchSubject.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CfTryCatchSubject.java
@@ -31,7 +31,7 @@ int index = 0; int startIndex = -1; int endIndex = -1; - for (CfInstruction instruction : cfCode.instructions) { + for (CfInstruction instruction : cfCode.getInstructions()) { if (startIndex < 0 && instruction.equals(tryCatch.start)) { startIndex = index; }
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java index 96b0d0c..6c7368b 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/ClassSubject.java
@@ -183,6 +183,8 @@ public abstract String getOriginalBinaryName(); + public abstract ClassReference getFinalReference(); + public abstract String getFinalName(); public abstract String getFinalDescriptor();
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/DexTryCatchSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/DexTryCatchSubject.java index 4624c33..c1fa59a 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/DexTryCatchSubject.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/DexTryCatchSubject.java
@@ -38,8 +38,8 @@ @Override public boolean isCatching(String exceptionType) { for (TypeAddrPair pair : tryHandler.pairs) { - if (pair.type.toString().equals(exceptionType) - || pair.type.toDescriptorString().equals(exceptionType)) { + if (pair.getType().toString().equals(exceptionType) + || pair.getType().toDescriptorString().equals(exceptionType)) { return true; } } @@ -54,7 +54,7 @@ @Override public Stream<TypeSubject> streamGuards() { return Arrays.stream(tryHandler.pairs) - .map(pair -> pair.type) + .map(pair -> pair.getType()) .map(type -> new TypeSubject(inspector, type)); }
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/FieldSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/FieldSubject.java index d4cd896..e63a9fd 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/FieldSubject.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/FieldSubject.java
@@ -20,6 +20,7 @@ public abstract DexValue getStaticValue(); + @Override public abstract boolean isRenamed(); public abstract String getOriginalSignatureAttribute();
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 dbdff3d..f42bc3e 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
@@ -28,6 +28,7 @@ import com.android.tools.r8.naming.MemberNaming.Signature; import com.android.tools.r8.naming.signature.GenericSignatureParser; import com.android.tools.r8.references.ClassReference; +import com.android.tools.r8.references.Reference; import com.android.tools.r8.utils.DescriptorUtils; import com.android.tools.r8.utils.StringUtils; import com.android.tools.r8.utils.ZipUtils; @@ -294,6 +295,11 @@ } @Override + public ClassReference getFinalReference() { + return Reference.classFromDescriptor(getFinalDescriptor()); + } + + @Override public String getFinalName() { return DescriptorUtils.descriptorToJavaType(getFinalDescriptor()); }
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java index 04f007b..4f4c340 100644 --- a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java +++ b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
@@ -299,6 +299,10 @@ return hasVisibility(Visibility.PRIVATE); } + public static <T extends MemberSubject> Matcher<T> isPackagePrivate() { + return hasVisibility(Visibility.PACKAGE_PRIVATE); + } + public static <T extends MemberSubject> Matcher<T> isPublic() { return hasVisibility(Visibility.PUBLIC); }
diff --git a/tools/r8_release.py b/tools/r8_release.py index dce7dad..7f27da8 100755 --- a/tools/r8_release.py +++ b/tools/r8_release.py
@@ -308,6 +308,12 @@ 'g4 change --desc "Update R8 to version %s\n"' % (version), shell=True) +def get_cl_id(c4_change_output): + startIndex = c4_change_output.find('Change ') + len('Change ') + endIndex = c4_change_output.find(' ', startIndex) + cl = c4_change_output[startIndex:endIndex] + assert cl.isdigit() + return cl def sed(pattern, replace, path): with open(path, "r") as sources: @@ -386,7 +392,11 @@ assert options.version in blaze_result if not options.no_upload: - return g4_change(options.version) + change_result = g4_change(options.version) + change_result += 'Run \'(g4d ' + args.p4_client \ + + ' && tap_presubmit -p all --train -c ' \ + + get_cl_id(change_result) + ')\' for running TAP presubmit.' + return change_result return release_google3