R8 private constructor nest desugaring
- LensCodeRewriter support for constructor
- Lens support for extra null parameter
Bug: 130529390
Change-Id: I35359ef3f288e366c5f0a329e48de5265ad301ab
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLense.java b/src/main/java/com/android/tools/r8/graph/GraphLense.java
index c3583dc..5b6ef08 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLense.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLense.java
@@ -236,15 +236,19 @@
private static final RewrittenPrototypeDescription none = new RewrittenPrototypeDescription();
private final boolean hasBeenChangedToReturnVoid;
+ private final boolean extraNullParameter;
private final RemovedArgumentsInfo removedArgumentsInfo;
private RewrittenPrototypeDescription() {
- this(false, RemovedArgumentsInfo.empty());
+ this(false, false, RemovedArgumentsInfo.empty());
}
public RewrittenPrototypeDescription(
- boolean hasBeenChangedToReturnVoid, RemovedArgumentsInfo removedArgumentsInfo) {
+ boolean hasBeenChangedToReturnVoid,
+ boolean extraNullParameter,
+ RemovedArgumentsInfo removedArgumentsInfo) {
assert removedArgumentsInfo != null;
+ this.extraNullParameter = extraNullParameter;
this.hasBeenChangedToReturnVoid = hasBeenChangedToReturnVoid;
this.removedArgumentsInfo = removedArgumentsInfo;
}
@@ -254,7 +258,13 @@
}
public boolean isEmpty() {
- return !hasBeenChangedToReturnVoid && !getRemovedArgumentsInfo().hasRemovedArguments();
+ return !extraNullParameter
+ && !hasBeenChangedToReturnVoid
+ && !getRemovedArgumentsInfo().hasRemovedArguments();
+ }
+
+ public boolean hasExtraNullParameter() {
+ return extraNullParameter;
}
public boolean hasBeenChangedToReturnVoid() {
@@ -310,13 +320,20 @@
public RewrittenPrototypeDescription withConstantReturn() {
return !hasBeenChangedToReturnVoid
- ? new RewrittenPrototypeDescription(true, removedArgumentsInfo)
+ ? new RewrittenPrototypeDescription(true, extraNullParameter, removedArgumentsInfo)
: this;
}
public RewrittenPrototypeDescription withRemovedArguments(RemovedArgumentsInfo other) {
return new RewrittenPrototypeDescription(
- hasBeenChangedToReturnVoid, removedArgumentsInfo.combine(other));
+ hasBeenChangedToReturnVoid, extraNullParameter, removedArgumentsInfo.combine(other));
+ }
+
+ public RewrittenPrototypeDescription withExtraNullParameter() {
+ return !extraNullParameter
+ ? new RewrittenPrototypeDescription(
+ hasBeenChangedToReturnVoid, true, removedArgumentsInfo)
+ : this;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
index 7650fe7..2203096 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/LensCodeRewriter.java
@@ -240,6 +240,13 @@
newInValues = invoke.inValues();
}
+ if (prototypeChanges.hasExtraNullParameter()) {
+ iterator.previous();
+ Value extraNullValue = iterator.insertConstNullInstruction(code, appView.options());
+ iterator.next();
+ newInValues.add(extraNullValue);
+ }
+
Invoke newInvoke =
Invoke.create(actualInvokeType, actualTarget, null, newOutValue, newInValues);
iterator.replaceCurrentInstruction(newInvoke);
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaring.java b/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaring.java
index a309b6d..3f6695b 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaring.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaring.java
@@ -53,6 +53,13 @@
this.appView = appView;
}
+ protected DexType getConstructorType() {
+ if (nestConstructor == null) {
+ return null;
+ }
+ return nestConstructor.type;
+ }
+
public void analyzeNests() {
// TODO(b/130529338) we don't need to compute a list with all live nests.
// we just need to iterate all live nests.
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaringAnalysis.java b/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaringAnalysis.java
index b009861..de92422 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaringAnalysis.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/NestBasedAccessDesugaringAnalysis.java
@@ -19,13 +19,12 @@
return appView.graphLense();
}
analyzeNests();
- return builder.build(appView.graphLense());
+ return builder.build(appView.graphLense(), getConstructorType());
}
@Override
protected void shouldRewriteInitializers(DexMethod method, DexMethod bridge) {
- // TODO(b/130529338): support initializer in r8
- // initializerMap.put(method, bridge);
+ builder.mapInitializer(method, bridge);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLense.java b/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLense.java
index 02e466f..252bf7c 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLense.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/NestedPrivateMethodLense.java
@@ -4,6 +4,7 @@
import com.android.tools.r8.graph.AppView;
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.GraphLense;
import com.android.tools.r8.graph.GraphLense.NestedGraphLense;
import com.android.tools.r8.ir.code.Invoke;
@@ -20,6 +21,7 @@
private final Map<DexField, DexMethod> staticPutToMethodMap = new IdentityHashMap<>();
private final Map<DexField, DexMethod> instanceGetToMethodMap = new IdentityHashMap<>();
private final Map<DexField, DexMethod> instancePutToMethodMap = new IdentityHashMap<>();
+ private final Map<DexMethod, DexMethod> initializerMap = new IdentityHashMap<>();
protected Builder(AppView<? extends AppInfo> appView) {
this.appView = appView;
@@ -29,6 +31,10 @@
instanceGetToMethodMap.put(field, method);
}
+ public void mapInitializer(DexMethod initializer, DexMethod method) {
+ initializerMap.put(initializer, method);
+ }
+
public void mapInstancePut(DexField field, DexMethod method) {
instancePutToMethodMap.put(field, method);
}
@@ -41,7 +47,7 @@
staticPutToMethodMap.put(field, method);
}
- public GraphLense build(GraphLense previousLense) {
+ public GraphLense build(GraphLense previousLense, DexType nestConstructorType) {
if (methodMap.isEmpty() && fieldMap.isEmpty()) {
return previousLense;
}
@@ -53,6 +59,8 @@
staticPutToMethodMap,
instanceGetToMethodMap,
instancePutToMethodMap,
+ initializerMap,
+ nestConstructorType,
previousLense);
}
}
@@ -63,6 +71,8 @@
private final Map<DexField, DexMethod> staticPutToMethodMap;
private final Map<DexField, DexMethod> instanceGetToMethodMap;
private final Map<DexField, DexMethod> instancePutToMethodMap;
+ private final Map<DexMethod, DexMethod> initializerMap;
+ private final DexType nestConstructorType;
public NestedPrivateMethodLense(
AppView<?> appView,
@@ -72,6 +82,8 @@
Map<DexField, DexMethod> staticPutToMethodMap,
Map<DexField, DexMethod> instanceGetToMethodMap,
Map<DexField, DexMethod> instancePutToMethodMap,
+ Map<DexMethod, DexMethod> initializerMap,
+ DexType nestConstructorType,
GraphLense previousLense) {
super(
ImmutableMap.of(),
@@ -86,6 +98,8 @@
this.staticPutToMethodMap = staticPutToMethodMap;
this.instanceGetToMethodMap = instanceGetToMethodMap;
this.instancePutToMethodMap = instancePutToMethodMap;
+ this.initializerMap = initializerMap;
+ this.nestConstructorType = nestConstructorType;
}
@Override
@@ -114,6 +128,20 @@
}
@Override
+ public RewrittenPrototypeDescription lookupPrototypeChanges(DexMethod method) {
+ RewrittenPrototypeDescription previous = previousLense.lookupPrototypeChanges(method);
+ DexType[] parameters = method.proto.parameters.values;
+ if (parameters.length == 0) {
+ return previous;
+ }
+ DexType lastParameterType = parameters[parameters.length - 1];
+ if (lastParameterType == nestConstructorType) {
+ return previous.withExtraNullParameter();
+ }
+ return previous;
+ }
+
+ @Override
public GraphLenseLookupResult lookupMethod(
DexMethod method, DexMethod context, Invoke.Type type) {
DexMethod previousContext =
@@ -122,8 +150,17 @@
: context;
GraphLenseLookupResult previous = previousLense.lookupMethod(method, previousContext, type);
DexMethod newMethod = methodMap.get(previous.getMethod());
- if (newMethod == null) {
- return previous;
+ Invoke.Type newType;
+ if (newMethod != null) {
+ // All generated non-initializer bridges are static.
+ newType = Invoke.Type.STATIC;
+ } else {
+ newMethod = initializerMap.get(previous.getMethod());
+ if (newMethod == null) {
+ return previous;
+ }
+ // All generated initializer bridges are direct.
+ newType = Invoke.Type.DIRECT;
}
if (newMethod == context) {
// Bridges should not rewrite themselves.
@@ -131,8 +168,7 @@
// A private method, we do not rewrite it.
return previous;
}
- // Use invokeStatic since all generated bridges are always static.
- return new GraphLenseLookupResult(newMethod, Invoke.Type.STATIC);
+ return new GraphLenseLookupResult(newMethod, newType);
}
public static Builder builder(AppView<?> appView) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
index 0dc5eae..ec95f89 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/UninstantiatedTypeOptimization.java
@@ -161,6 +161,7 @@
RewrittenPrototypeDescription prototypeChanges =
new RewrittenPrototypeDescription(
virtualMethod.method.proto.returnType.isAlwaysNull(appView),
+ false,
getRemovedArgumentsInfo(virtualMethod, ALLOW_ARGUMENT_REMOVAL));
if (!prototypeChanges.isEmpty()) {
DexMethod newMethod = getNewMethodSignature(virtualMethod, prototypeChanges);
@@ -296,6 +297,7 @@
}
return new RewrittenPrototypeDescription(
encodedMethod.method.proto.returnType.isAlwaysNull(appView),
+ false,
getRemovedArgumentsInfo(encodedMethod, strategy));
}
diff --git a/src/test/java/com/android/tools/r8/desugar/NestAccessControl/NestAccessControlTest.java b/src/test/java/com/android/tools/r8/desugar/NestAccessControl/NestAccessControlTest.java
index 1fa9dd9..2c05ed5 100644
--- a/src/test/java/com/android/tools/r8/desugar/NestAccessControl/NestAccessControlTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/NestAccessControl/NestAccessControlTest.java
@@ -5,7 +5,6 @@
package com.android.tools.r8.desugar.NestAccessControl;
import static com.android.tools.r8.utils.FileUtils.JAR_EXTENSION;
-import static org.hamcrest.core.StringContains.containsString;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -19,7 +18,6 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.TestRuntime.CfVm;
import com.android.tools.r8.ToolHelper;
-import com.android.tools.r8.ToolHelper.DexVm.Version;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.StringUtils;
@@ -156,35 +154,25 @@
testJavaAndD8("all");
}
- public void testR8(String id, boolean r8Success) throws Exception {
+ public void testR8(String id) throws Exception {
R8TestRunResult result =
r8CompilationResult
.apply(parameters.getBackend(), parameters.getApiLevel())
.run(parameters.getRuntime(), getMainClass(id));
- if (r8Success) {
- result.assertSuccessWithOutput(getExpectedResult(id));
- if (parameters.isCfRuntime()) {
- result.inspect(NestAccessControlTest::checkNestMateAttributes);
- }
- } else {
- if (parameters.isDexRuntime()
- && (parameters.getRuntime().asDex().getVm().getVersion() == Version.V6_0_1
- || parameters.getRuntime().asDex().getVm().getVersion() == Version.V5_1_1)) {
- result.assertFailure(); // different message, same error
- } else {
- result.assertFailureWithErrorThatMatches(containsString("IllegalAccessError"));
- }
+ result.assertSuccessWithOutput(getExpectedResult(id));
+ if (parameters.isCfRuntime()) {
+ result.inspect(NestAccessControlTest::checkNestMateAttributes);
}
}
@Test
public void testMethodsAccessR8() throws Exception {
// TODO(b/130529390): As features are implemented, set success to true in each line.
- testR8("methods", true);
- testR8("fields", true);
- testR8("constructors", parameters.isCfRuntime());
- testR8("anonymous", true);
- testR8("all", parameters.isCfRuntime());
+ testR8("methods");
+ testR8("fields");
+ testR8("constructors");
+ testR8("anonymous");
+ testR8("all");
}
private static void checkNestMateAttributes(CodeInspector inspector) {
