Ensure API safe parameter types in instance initializer merger
Fixes: b/324383881
Fixes: b/331574594
Change-Id: I761c712671d5fe731b079a9f98e1b255d46aff96
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 e096d1e..42e9f77 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -259,15 +259,6 @@
defaultTypeRewriter(appInfo));
}
- public static <T extends AppInfo> AppView<T> createForSimulatingD8InR8(T appInfo) {
- return new AppView<>(
- appInfo,
- ArtProfileCollection.empty(),
- StartupProfile.empty(),
- WholeProgramOptimizations.OFF,
- defaultTypeRewriter(appInfo));
- }
-
public static AppView<AppInfoWithClassHierarchy> createForSimulatingR8InD8(
DirectMappedDexApplication application, MainDexInfo mainDexInfo) {
ClassToFeatureSplitMap classToFeatureSplitMap =
diff --git a/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java b/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java
index 3250adb..30e2f54 100644
--- a/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java
+++ b/src/main/java/com/android/tools/r8/graph/DexTypeUtils.java
@@ -12,14 +12,30 @@
public class DexTypeUtils {
+ public static DexType computeApiSafeLeastUpperBound(
+ AppView<? extends AppInfoWithClassHierarchy> appView, Iterable<DexType> types) {
+ DexType leastUpperBound = computeLeastUpperBound(appView, types);
+ return findApiSafeUpperBound(appView, leastUpperBound);
+ }
+
public static DexType computeLeastUpperBound(
AppView<? extends AppInfoWithClassHierarchy> appView, Iterable<DexType> types) {
TypeElement join =
TypeElement.join(Iterables.transform(types, type -> type.toTypeElement(appView)), appView);
- return findApiSafeUpperBound(appView, toDexType(appView, join));
+ return toDexType(appView, join);
}
- @SuppressWarnings("ReferenceEquality")
+ public static boolean isApiSafe(
+ AppView<? extends AppInfoWithClassHierarchy> appView, DexType type) {
+ DexType apiSafeUpperBound = findApiSafeUpperBound(appView, type);
+ return apiSafeUpperBound.isIdenticalTo(type);
+ }
+
+ public static boolean isLeastUpperBoundApiSafe(
+ AppView<? extends AppInfoWithClassHierarchy> appView, Iterable<DexType> types) {
+ return isApiSafe(appView, computeLeastUpperBound(appView, types));
+ }
+
public static DexType toDexType(
AppView<? extends AppInfoWithClassHierarchy> appView, TypeElement type) {
DexItemFactory dexItemFactory = appView.dexItemFactory();
@@ -33,7 +49,7 @@
}
assert type.isClassType();
ClassTypeElement classType = type.asClassType();
- if (classType.getClassType() != dexItemFactory.objectType) {
+ if (classType.getClassType().isNotIdenticalTo(dexItemFactory.objectType)) {
return classType.getClassType();
}
if (classType.getInterfaces().hasSingleKnownInterface()) {
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
index 778132e..6c98445 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInstanceFieldsMerger.java
@@ -189,7 +189,7 @@
DexEncodedField newField;
if (needsRelaxedType(targetField, sourceFields)) {
DexType newFieldType =
- DexTypeUtils.computeLeastUpperBound(
+ DexTypeUtils.computeApiSafeLeastUpperBound(
appView,
Iterables.transform(
Iterables.concat(IterableUtils.singleton(targetField), sourceFields),
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
index 0db2480..3499c30 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassMerger.java
@@ -77,7 +77,6 @@
private ClassMerger(
AppView<?> appView,
- IRCodeProvider codeProvider,
HorizontalClassMergerGraphLens.Builder lensBuilder,
HorizontalMergeGroup group,
Collection<VirtualMethodMerger> virtualMethodMergers) {
@@ -93,8 +92,7 @@
// Method mergers.
this.classInitializerMerger = ClassInitializerMerger.create(group);
this.instanceInitializerMergers =
- InstanceInitializerMergerCollection.create(
- appView, classIdentifiers, codeProvider, group, lensBuilder);
+ InstanceInitializerMergerCollection.create(appView, classIdentifiers, group, lensBuilder);
this.virtualMethodMergers = virtualMethodMergers;
buildClassIdentifierMap();
@@ -365,14 +363,12 @@
public static class Builder {
private final AppView<?> appView;
- private final IRCodeProvider codeProvider;
private final HorizontalMergeGroup group;
private List<VirtualMethodMerger> virtualMethodMergers;
- public Builder(AppView<?> appView, IRCodeProvider codeProvider, HorizontalMergeGroup group) {
+ public Builder(AppView<?> appView, HorizontalMergeGroup group) {
this.appView = appView;
- this.codeProvider = codeProvider;
this.group = group;
}
@@ -464,7 +460,7 @@
public ClassMerger build(
HorizontalClassMergerGraphLens.Builder lensBuilder) {
- return new ClassMerger(appView, codeProvider, lensBuilder, group, virtualMethodMergers);
+ return new ClassMerger(appView, lensBuilder, group, virtualMethodMergers);
}
}
}
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 9307f89..51c1aca 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/HorizontalClassMerger.java
@@ -20,8 +20,6 @@
import com.android.tools.r8.graph.PrunedItems;
import com.android.tools.r8.horizontalclassmerging.code.SyntheticInitializerConverter;
import com.android.tools.r8.ir.conversion.LirConverter;
-import com.android.tools.r8.ir.conversion.MethodConversionOptions;
-import com.android.tools.r8.ir.conversion.MethodConversionOptions.MutableMethodConversionOptions;
import com.android.tools.r8.naming.IdentifierMinifier;
import com.android.tools.r8.profile.art.ArtProfileCompletenessChecker;
import com.android.tools.r8.profile.rewriting.ProfileCollectionAdditions;
@@ -72,11 +70,7 @@
throws ExecutionException {
timing.begin("HorizontalClassMerger");
if (shouldRun()) {
- IRCodeProvider codeProvider =
- appView.hasClassHierarchy()
- ? IRCodeProvider.create(appView.withClassHierarchy(), this::getConversionOptions)
- : IRCodeProvider.createThrowing();
- run(runtimeTypeCheckInfo, codeProvider, executorService, timing);
+ run(runtimeTypeCheckInfo, executorService, timing);
assert ArtProfileCompletenessChecker.verify(appView);
@@ -95,13 +89,8 @@
&& !appView.hasCfByteCodePassThroughMethods();
}
- private MutableMethodConversionOptions getConversionOptions() {
- return MethodConversionOptions.forLirPhase(appView);
- }
-
private void run(
RuntimeTypeCheckInfo runtimeTypeCheckInfo,
- IRCodeProvider codeProvider,
ExecutorService executorService,
Timing timing)
throws ExecutionException {
@@ -128,7 +117,7 @@
new HorizontalClassMergerGraphLens.Builder();
// Determine which classes need a class id field.
- List<ClassMerger.Builder> classMergerBuilders = createClassMergerBuilders(codeProvider, groups);
+ List<ClassMerger.Builder> classMergerBuilders = createClassMergerBuilders(groups);
initializeClassIdFields(classMergerBuilders);
// Ensure that all allocations of classes that end up needing a class id use a constructor on
@@ -141,7 +130,7 @@
ProfileCollectionAdditions profileCollectionAdditions =
ProfileCollectionAdditions.create(appView);
SyntheticInitializerConverter.Builder syntheticInitializerConverterBuilder =
- SyntheticInitializerConverter.builder(appView, codeProvider);
+ SyntheticInitializerConverter.builder(appView);
List<VirtuallyMergedMethodsKeepInfo> virtuallyMergedMethodsKeepInfos = new ArrayList<>();
PrunedItems prunedItems =
applyClassMergers(
@@ -184,7 +173,6 @@
// Set the new graph lens before finalizing any synthetic code.
appView.setGraphLens(horizontalClassMergerGraphLens);
- codeProvider.setGraphLens(horizontalClassMergerGraphLens);
// Must rewrite AppInfoWithLiveness before pruning the merged classes, to ensure that allocation
// sites, fields accesses, etc. are correctly transferred to the target classes.
@@ -378,13 +366,13 @@
}
private List<ClassMerger.Builder> createClassMergerBuilders(
- IRCodeProvider codeProvider, Collection<HorizontalMergeGroup> groups) {
+ Collection<HorizontalMergeGroup> groups) {
List<ClassMerger.Builder> classMergerBuilders = new ArrayList<>(groups.size());
for (HorizontalMergeGroup group : groups) {
assert group.isNonTrivial();
assert group.hasInstanceFieldMap();
assert group.hasTarget();
- classMergerBuilders.add(new ClassMerger.Builder(appView, codeProvider, group));
+ classMergerBuilders.add(new ClassMerger.Builder(appView, group));
}
return classMergerBuilders;
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/IRCodeProvider.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/IRCodeProvider.java
deleted file mode 100644
index 95c4020..0000000
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/IRCodeProvider.java
+++ /dev/null
@@ -1,77 +0,0 @@
-// Copyright (c) 2021, 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.AppInfo;
-import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
-import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.graph.lens.GraphLens;
-import com.android.tools.r8.ir.code.IRCode;
-import com.android.tools.r8.ir.conversion.MethodConversionOptions.MutableMethodConversionOptions;
-import com.android.tools.r8.synthesis.SyntheticItems.GlobalSyntheticsStrategy;
-import java.util.function.Supplier;
-
-public interface IRCodeProvider {
-
- IRCode buildIR(ProgramMethod method);
-
- void setGraphLens(GraphLens graphLens);
-
- static IRCodeProvider create(
- AppView<? extends AppInfoWithClassHierarchy> appView,
- Supplier<MutableMethodConversionOptions> getConversionOptions) {
- return new IRCodeProviderImpl(appView, getConversionOptions);
- }
-
- static IRCodeProvider createThrowing() {
- return new IRCodeProvider() {
- @Override
- public IRCode buildIR(ProgramMethod method) {
- throw new UnsupportedOperationException("Should never build IR for methods in D8");
- }
-
- @Override
- public void setGraphLens(GraphLens graphLens) {}
- };
- }
-
- class IRCodeProviderImpl implements IRCodeProvider {
-
- private final AppView<AppInfo> appViewForConversion;
- private Supplier<MutableMethodConversionOptions> getConversionOptions;
-
- private IRCodeProviderImpl(
- AppView<? extends AppInfoWithClassHierarchy> appView,
- Supplier<MutableMethodConversionOptions> getConversionOptions) {
- // At this point the code rewritings described by repackaging and synthetic finalization have
- // not been applied to the code objects. These code rewritings will be applied in the
- // application writer. We therefore simulate that we are in D8, to allow building IR for each
- // of the class initializers without applying the unapplied code rewritings, to avoid that we
- // apply the lens more than once to the same piece of code.
- AppView<AppInfo> appViewForConversion =
- AppView.createForSimulatingD8InR8(
- AppInfo.createInitialAppInfo(
- appView.appInfo().app(), GlobalSyntheticsStrategy.forNonSynthesizing()));
- appViewForConversion.setGraphLens(appView.graphLens());
- appViewForConversion.setCodeLens(appView.codeLens());
- this.appViewForConversion = appViewForConversion;
- this.getConversionOptions = getConversionOptions;
- }
-
- @Override
- public IRCode buildIR(ProgramMethod method) {
- return method
- .getDefinition()
- .getCode()
- .buildIR(method, appViewForConversion, getConversionOptions.get());
- }
-
- @Override
- public void setGraphLens(GraphLens graphLens) {
- appViewForConversion.setGraphLens(graphLens);
- }
- }
-}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java
index 96aeb10..c7dfba3 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerAnalysis.java
@@ -29,6 +29,7 @@
import com.android.tools.r8.ir.code.Instruction;
import com.android.tools.r8.ir.code.InvokeDirect;
import com.android.tools.r8.ir.code.Value;
+import com.android.tools.r8.ir.conversion.MethodConversionOptions;
import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfo;
import com.android.tools.r8.utils.WorkList;
import com.google.common.collect.Iterables;
@@ -39,7 +40,6 @@
public static InstanceInitializerDescription analyze(
AppView<? extends AppInfoWithClassHierarchy> appView,
- IRCodeProvider codeProvider,
HorizontalMergeGroup group,
InstanceInitializer instanceInitializer) {
if (instanceInitializer.isAbsent()) {
@@ -61,19 +61,17 @@
builder.addInvokeConstructor(invokedConstructor, invokedConstructorArguments);
return builder.build();
} else {
- return analyze(appView, codeProvider, group, instanceInitializer.asPresent().getMethod());
+ return analyze(appView, group, instanceInitializer.asPresent().getMethod());
}
}
- @SuppressWarnings("ReferenceEquality")
public static InstanceInitializerDescription analyze(
AppView<? extends AppInfoWithClassHierarchy> appView,
- IRCodeProvider codeProvider,
HorizontalMergeGroup group,
ProgramMethod instanceInitializer) {
InstanceInitializerDescription.Builder builder =
InstanceInitializerDescription.builder(appView, instanceInitializer);
- IRCode code = codeProvider.buildIR(instanceInitializer);
+ IRCode code = instanceInitializer.buildIR(appView, MethodConversionOptions.nonConverting());
GraphLens codeLens = instanceInitializer.getDefinition().getCode().getCodeLens(appView);
WorkList<BasicBlock> workList = WorkList.newIdentityWorkList(code.entryBlock());
while (workList.hasNext()) {
@@ -108,8 +106,9 @@
DexField fieldReference = instancePut.getField();
DexField lensRewrittenFieldReference =
appView.graphLens().lookupField(fieldReference, codeLens);
- if (lensRewrittenFieldReference.getHolderType()
- != instanceInitializer.getHolderType()) {
+ if (lensRewrittenFieldReference
+ .getHolderType()
+ .isNotIdenticalTo(instanceInitializer.getHolderType())) {
return invalid();
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
index 256971c..be78b39 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMerger.java
@@ -101,16 +101,12 @@
DexType[] newParameters =
new DexType[representative.getParameters().size() + BooleanUtils.intValue(needsClassId)];
System.arraycopy(oldParameters, 0, newParameters, 0, oldParameters.length);
- for (int i = 0; i < oldParameters.length; i++) {
- final int parameterIndex = i;
- Set<DexType> parameterTypes =
- SetUtils.newIdentityHashSet(
- builder ->
- instanceInitializers.forEach(
- instanceInitializer ->
- builder.accept(instanceInitializer.getParameter(parameterIndex))));
+ for (int parameterIndex = 0; parameterIndex < oldParameters.length; parameterIndex++) {
+ Set<DexType> parameterTypes = getParameterTypes(instanceInitializers, parameterIndex);
if (parameterTypes.size() > 1) {
- newParameters[i] = DexTypeUtils.computeLeastUpperBound(appView, parameterTypes);
+ DexType leastUpperBound = DexTypeUtils.computeLeastUpperBound(appView, parameterTypes);
+ assert DexTypeUtils.isApiSafe(appView, leastUpperBound);
+ newParameters[parameterIndex] = leastUpperBound;
}
}
if (needsClassId) {
@@ -120,6 +116,15 @@
return dexItemFactory.createInstanceInitializer(group.getTarget().getType(), newParameters);
}
+ private static Set<DexType> getParameterTypes(
+ List<ProgramMethod> instanceInitializers, int parameterIndex) {
+ return SetUtils.newIdentityHashSet(
+ builder ->
+ instanceInitializers.forEach(
+ instanceInitializer ->
+ builder.accept(instanceInitializer.getParameter(parameterIndex))));
+ }
+
/**
* Returns a special original method signature for the synthesized constructor that did not exist
* prior to horizontal class merging. Otherwise we might accidentally think that the synthesized
@@ -169,9 +174,11 @@
createNewGroup();
}
- private void createNewGroup() {
+ private List<ProgramMethod> createNewGroup() {
estimatedDexCodeSize = 0;
- instanceInitializerGroups.add(new ArrayList<>());
+ List<ProgramMethod> newGroup = new ArrayList<>();
+ instanceInitializerGroups.add(newGroup);
+ return newGroup;
}
public Builder add(ProgramMethod instanceInitializer) {
@@ -190,10 +197,40 @@
}
public Builder addEquivalent(ProgramMethod instanceInitializer) {
- ListUtils.last(instanceInitializerGroups).add(instanceInitializer);
+ // If adding the given constructor to the current merge group leads to any API unsafe
+ // parameter types, then the constructor should be merged into a new group.
+ List<ProgramMethod> eligibleGroup = null;
+ for (List<ProgramMethod> candidateGroup : instanceInitializerGroups) {
+ if (isMergeApiSafe(candidateGroup, instanceInitializer)) {
+ eligibleGroup = candidateGroup;
+ break;
+ }
+ }
+ if (eligibleGroup == null) {
+ eligibleGroup = createNewGroup();
+ }
+ eligibleGroup.add(instanceInitializer);
return this;
}
+ private boolean isMergeApiSafe(List<ProgramMethod> group, ProgramMethod instanceInitializer) {
+ if (group.isEmpty()) {
+ return true;
+ }
+ for (int parameterIndex = 0;
+ parameterIndex < instanceInitializer.getParameters().size();
+ parameterIndex++) {
+ Set<DexType> parameterTypes = getParameterTypes(group, parameterIndex);
+ // Adding the given instance initializer to the group can only lead to an API unsafe
+ // parameter type if the instance initializer contributes a new parameter type to the group.
+ if (parameterTypes.add(instanceInitializer.getParameter(parameterIndex))
+ && !DexTypeUtils.isLeastUpperBoundApiSafe(appView, parameterTypes)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
public List<InstanceInitializerMerger> build(HorizontalMergeGroup group) {
assert instanceInitializerGroups.stream().noneMatch(List::isEmpty);
return ListUtils.map(
@@ -203,18 +240,19 @@
appView, classIdentifiers, group, instanceInitializers, lensBuilder));
}
- public InstanceInitializerMerger buildSingle(
+ public List<InstanceInitializerMerger> buildEquivalent(
HorizontalMergeGroup group, InstanceInitializerDescription instanceInitializerDescription) {
assert instanceInitializerGroups.stream().noneMatch(List::isEmpty);
- assert instanceInitializerGroups.size() == 1;
- List<ProgramMethod> instanceInitializers = ListUtils.first(instanceInitializerGroups);
- return new InstanceInitializerMerger(
- appView,
- classIdentifiers,
- group,
- instanceInitializers,
- lensBuilder,
- instanceInitializerDescription);
+ return ListUtils.map(
+ instanceInitializerGroups,
+ instanceInitializers ->
+ new InstanceInitializerMerger(
+ appView,
+ classIdentifiers,
+ group,
+ instanceInitializers,
+ lensBuilder,
+ instanceInitializerDescription));
}
}
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
index e0088ec..5b9e401 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/InstanceInitializerMergerCollection.java
@@ -40,7 +40,6 @@
public static InstanceInitializerMergerCollection create(
AppView<?> appView,
Reference2IntMap<DexType> classIdentifiers,
- IRCodeProvider codeProvider,
HorizontalMergeGroup group,
HorizontalClassMergerGraphLens.Builder lensBuilder) {
if (!appView.hasClassHierarchy()) {
@@ -61,7 +60,7 @@
instanceInitializer -> {
InstanceInitializerDescription description =
InstanceInitializerAnalysis.analyze(
- appViewWithClassHierarchy, codeProvider, group, instanceInitializer);
+ appViewWithClassHierarchy, group, instanceInitializer);
if (description != null) {
buildersByDescription
.computeIfAbsent(
@@ -80,14 +79,17 @@
equivalentInstanceInitializerMergers = new LinkedHashMap<>();
buildersByDescription.forEach(
(description, builder) -> {
- InstanceInitializerMerger instanceInitializerMerger =
- builder.buildSingle(group, description);
- if (instanceInitializerMerger.size() == 1) {
- // If there is only one constructor with a specific behavior, then consider it for
- // normal instance initializer merging below.
- buildersWithoutDescription.addAll(instanceInitializerMerger.getInstanceInitializers());
- } else {
- equivalentInstanceInitializerMergers.put(description, instanceInitializerMerger);
+ List<InstanceInitializerMerger> instanceInitializerMergers =
+ builder.buildEquivalent(group, description);
+ for (InstanceInitializerMerger instanceInitializerMerger : instanceInitializerMergers) {
+ if (instanceInitializerMerger.size() == 1) {
+ // If there is only one constructor with a specific behavior, then consider it for
+ // normal instance initializer merging below.
+ buildersWithoutDescription.addAll(
+ instanceInitializerMerger.getInstanceInitializers());
+ } else {
+ equivalentInstanceInitializerMergers.put(description, instanceInitializerMerger);
+ }
}
});
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/code/SyntheticInitializerConverter.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/code/SyntheticInitializerConverter.java
index 55a073a..85ad211 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/code/SyntheticInitializerConverter.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/code/SyntheticInitializerConverter.java
@@ -4,14 +4,12 @@
package com.android.tools.r8.horizontalclassmerging.code;
-import com.android.tools.r8.graph.AppInfo;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.ProgramMethod;
-import com.android.tools.r8.horizontalclassmerging.IRCodeProvider;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.conversion.IRConverter;
+import com.android.tools.r8.ir.conversion.MethodConversionOptions;
import com.android.tools.r8.ir.optimize.info.OptimizationFeedbackIgnore;
-import com.android.tools.r8.synthesis.SyntheticItems.GlobalSyntheticsStrategy;
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
import java.util.ArrayList;
@@ -26,25 +24,22 @@
public class SyntheticInitializerConverter {
private final AppView<?> appView;
- private final IRCodeProvider codeProvider;
private final List<ProgramMethod> classInitializers;
- private SyntheticInitializerConverter(
- AppView<?> appView, IRCodeProvider codeProvider, List<ProgramMethod> classInitializers) {
+ private SyntheticInitializerConverter(AppView<?> appView, List<ProgramMethod> classInitializers) {
this.appView = appView;
- this.codeProvider = codeProvider;
this.classInitializers = classInitializers;
}
- public static Builder builder(AppView<?> appView, IRCodeProvider codeProvider) {
- return new Builder(appView, codeProvider);
+ public static Builder builder(AppView<?> appView) {
+ return new Builder(appView);
}
public void convertClassInitializers(ExecutorService executorService) throws ExecutionException {
if (!classInitializers.isEmpty()) {
assert appView.dexItemFactory().verifyNoCachedTypeElements();
- IRConverter converter = new IRConverter(createAppViewForConversion());
+ IRConverter converter = new IRConverter(appView);
ThreadUtils.processItems(
classInitializers,
method -> processMethod(method, converter),
@@ -54,30 +49,8 @@
}
}
- private AppView<AppInfo> createAppViewForConversion() {
- assert appView.enableWholeProgramOptimizations();
- assert appView.hasClassHierarchy();
-
- // At this point the code rewritings described by repackaging and synthetic finalization have
- // not been applied to the code objects. These code rewritings will be applied in the
- // application writer. We therefore simulate that we are in D8, to allow building IR for each of
- // the class initializers without applying the unapplied code rewritings, to avoid that we apply
- // the lens more than once to the same piece of code.
-
- // Since we are now running in D8 mode clear type elements cache.
- appView.dexItemFactory().clearTypeElementsCache();
-
- AppView<AppInfo> appViewForConversion =
- AppView.createForSimulatingD8InR8(
- AppInfo.createInitialAppInfo(
- appView.appInfo().app(), GlobalSyntheticsStrategy.forNonSynthesizing()));
- appViewForConversion.setGraphLens(appView.graphLens());
- appViewForConversion.setCodeLens(appView.codeLens());
- return appViewForConversion;
- }
-
private void processMethod(ProgramMethod method, IRConverter converter) {
- IRCode code = codeProvider.buildIR(method);
+ IRCode code = method.buildIR(appView, MethodConversionOptions.forLirPhase(appView));
converter.removeDeadCodeAndFinalizeIR(
code, OptimizationFeedbackIgnore.getInstance(), Timing.empty());
}
@@ -89,13 +62,11 @@
public static class Builder {
private final AppView<?> appView;
- private final IRCodeProvider codeProvider;
private final List<ProgramMethod> classInitializers = new ArrayList<>();
- private Builder(AppView<?> appView, IRCodeProvider codeProvider) {
+ private Builder(AppView<?> appView) {
this.appView = appView;
- this.codeProvider = codeProvider;
}
public Builder addClassInitializer(ProgramMethod method) {
@@ -104,7 +75,7 @@
}
public SyntheticInitializerConverter build() {
- return new SyntheticInitializerConverter(appView, codeProvider, classInitializers);
+ return new SyntheticInitializerConverter(appView, classInitializers);
}
}
}
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClInitMergeSuperTypeApiLevelTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClInitMergeSuperTypeApiLevelTest.java
index 164755b..f0de7ae 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/ClInitMergeSuperTypeApiLevelTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/ClInitMergeSuperTypeApiLevelTest.java
@@ -6,10 +6,12 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoVerticalClassMerging;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
@@ -39,11 +41,14 @@
return getTestParameters().withAllRuntimesAndApiLevels().build();
}
+ private boolean canUseExecutable() {
+ return parameters.isDexRuntime()
+ && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.O);
+ }
+
private TypeReference getMergeReferenceForApiLevel() {
- boolean canUseExecutable =
- parameters.isDexRuntime()
- && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.O);
- return Reference.typeFromTypeName(typeName(canUseExecutable ? Executable.class : Object.class));
+ return Reference.typeFromTypeName(
+ typeName(canUseExecutable() ? Executable.class : Object.class));
}
@Test
@@ -59,17 +64,34 @@
inspector.assertIsCompleteMergeGroup(A.class, B.class).assertNoOtherClassesMerged())
.enableInliningAnnotations()
.enableNeverClassInliningAnnotations()
+ .enableNoVerticalClassMergingAnnotations()
.compile()
.inspect(
inspector -> {
ClassSubject clazz = inspector.clazz(A.class);
assertThat(clazz, isPresent());
- TypeReference mergeTypeRef = getMergeReferenceForApiLevel();
- MethodSubject init = clazz.init(mergeTypeRef.getTypeName(), "int");
- assertThat(init, isPresent());
+
assertTrue(
clazz.allFields().stream()
- .anyMatch(f -> mergeTypeRef.equals(f.getFinalReference().getFieldType())));
+ .anyMatch(
+ f ->
+ f.getFinalReference()
+ .getFieldType()
+ .equals(getMergeReferenceForApiLevel())));
+
+ assertEquals(
+ canUseExecutable() ? 1 : 2,
+ clazz.allMethods(MethodSubject::isInstanceInitializer).size());
+ if (canUseExecutable()) {
+ MethodSubject constructorInit = clazz.init(Executable.class.getTypeName(), "int");
+ assertThat(constructorInit, isPresent());
+ } else {
+ MethodSubject constructorInit = clazz.init(Constructor.class.getTypeName());
+ assertThat(constructorInit, isPresent());
+
+ MethodSubject methodInit = clazz.init(Method.class.getTypeName());
+ assertThat(methodInit, isPresent());
+ }
})
.run(parameters.getRuntime(), Main.class)
// The test succeeds for some unknown reason.
@@ -92,6 +114,7 @@
}
}
+ @NoVerticalClassMerging
public abstract static class Factory {
abstract Object newInstance() throws Exception;
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentInstanceInitializerMergingWithApiUnsafeParameterTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentInstanceInitializerMergingWithApiUnsafeParameterTest.java
index e2b7710..dac6de6 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentInstanceInitializerMergingWithApiUnsafeParameterTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/EquivalentInstanceInitializerMergingWithApiUnsafeParameterTest.java
@@ -7,9 +7,7 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import com.android.tools.r8.NeverInline;
import com.android.tools.r8.NoFieldTypeStrengthening;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -43,29 +41,20 @@
.addHorizontallyMergedClassesInspector(
inspector ->
inspector.assertIsCompleteMergeGroup(A.class, B.class).assertNoOtherClassesMerged())
- .enableInliningAnnotations()
.enableNoFieldTypeStrengtheningAnnotations()
.setMinApi(parameters)
.compile()
.inspect(
inspector -> {
- // Verify that the two constructors A.<init> and B.<init> have been merged.
+ // Verify that the two constructors A.<init> and B.<init> have not been merged.
ClassSubject aClassSubject = inspector.clazz(A.class);
assertThat(aClassSubject, isPresent());
assertEquals(
- 1, aClassSubject.allMethods(MethodSubject::isInstanceInitializer).size());
-
- MethodSubject instanceInitializer = aClassSubject.uniqueInstanceInitializer();
- assertThat(instanceInitializer, isPresent());
- assertTrue(
- instanceInitializer
- .streamInstructions()
- .noneMatch(i -> i.isIf() || i.isSwitch()));
+ 2, aClassSubject.allMethods(MethodSubject::isInstanceInitializer).size());
})
.addRunClasspathClasses(LibraryClassBase.class, LibraryClass.class)
.run(parameters.getRuntime(), Main.class)
- // TODO(b/331574594): Should succeed with "LibraryClass", "MyLibraryClass".
- .assertFailureWithErrorThatThrows(VerifyError.class);
+ .assertSuccessWithOutputLines("LibraryClass", "MyLibraryClass");
}
static class LibraryClassBase {}
@@ -101,7 +90,6 @@
@NoFieldTypeStrengthening LibraryClassBase f;
- @NeverInline
A(LibraryClass c) {
super(c);
f = c;
@@ -118,7 +106,6 @@
@NoFieldTypeStrengthening LibraryClassBase f;
- @NeverInline
B(MyLibraryClass d) {
super(d);
f = d;
