Merge "One step closer to actual JVM field resolution."
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 62c5b7b..fcbd853 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -152,15 +152,15 @@
*/
public DexEncodedField lookupInstanceTarget(DexType type, DexField field) {
assert type.isClassType();
- return lookupTargetAlongSuperChain(type, field, DexClass::findInstanceTarget);
+ return resolveFieldOn(type, field, false);
}
/**
- * Lookup static field starting in type and following the super chain.
+ * Lookup static field starting in type and following the interface and super chain.
*/
public DexEncodedField lookupStaticTarget(DexType type, DexField field) {
assert type.isClassType();
- return lookupTargetAlongInterfaceAndSuperChain(type, field, DexClass::findStaticTarget);
+ return resolveFieldOn(type, field, true);
}
/**
@@ -226,27 +226,37 @@
}
/**
- * Traverse along the super chain and the interface chains until lookup returns non-null value.
+ * Implements resolution of a field descriptor against a type.
+ * <p>
+ * See <a href="https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-5.html#jvms-5.4.3.2">
+ * Section 5.4.3.2 of the JVM Spec</a>.
*/
- private <S extends DexItem, T extends Descriptor<S, T>> S lookupTargetAlongInterfaceAndSuperChain(
- DexType type,
- T desc,
- BiFunction<DexClass, T, S> lookup) {
+ private DexEncodedField resolveFieldOn(DexType type, DexField desc, boolean isStatic) {
DexClass holder = definitionFor(type);
if (holder == null) {
return null;
}
- S result = lookup.apply(holder, desc);
+ // Step 1: Class declares the field.
+ DexEncodedField result =
+ isStatic ? holder.findStaticTarget(desc) : holder.findInstanceTarget(desc);
if (result != null) {
return result;
}
- result = applyToInterfacesAndDisambiguate(holder.interfaces.values, desc, lookup,
- this::lookupTargetAlongSuperAndInterfaceChain);
- if (result != null) {
- return result;
+ // Step 2: Apply recursively to direct superinterfaces. First match succeeds.
+ //
+ // We short-cut here, as we know that interfaces cannot have instance fields.
+ // See https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-4.html#jvms-4.5.
+ if (isStatic) {
+ for (DexType iface : holder.interfaces.values) {
+ result = resolveFieldOn(iface, desc, isStatic);
+ if (result != null) {
+ return result;
+ }
+ }
}
+ // Step 3: Apply recursively to superclass.
if (holder.superType != null) {
- result = lookupTargetAlongInterfaceAndSuperChain(holder.superType, desc, lookup);
+ result = resolveFieldOn(holder.superType, desc, isStatic);
if (result != null) {
return result;
}
diff --git a/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java b/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
index caada82..94f1deb 100644
--- a/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
+++ b/src/test/java/com/android/tools/r8/jasmin/JasminBuilder.java
@@ -29,17 +29,26 @@
public static class ClassBuilder {
public final String name;
public final String superName;
+ public final ImmutableList<String> interfaces;
private final List<String> methods = new ArrayList<>();
private final List<String> fields = new ArrayList<>();
private boolean makeInit = false;
+ private boolean isInterface = false;
public ClassBuilder(String name) {
this(name, "java/lang/Object");
}
- public ClassBuilder(String name, String superName) {
+ private ClassBuilder(String name, String superName) {
this.name = name;
this.superName = superName;
+ this.interfaces = ImmutableList.of();
+ }
+
+ private ClassBuilder(String name, String superName, String... interfaces) {
+ this.name = name;
+ this.superName = superName;
+ this.interfaces = ImmutableList.copyOf(interfaces);
}
public String getSourceFile() {
@@ -121,9 +130,17 @@
@Override
public String toString() {
StringBuilder builder = new StringBuilder();
- builder.append(".source ").append(getSourceFile()).append("\n");
- builder.append(".class public ").append(name).append("\n");
- builder.append(".super ").append(superName).append("\n");
+ builder.append(".source ").append(getSourceFile()).append('\n');
+ if (isInterface) {
+ builder.append(".interface");
+ } else {
+ builder.append(".class");
+ }
+ builder.append(" public ").append(name).append('\n');
+ builder.append(".super ").append(superName).append('\n');
+ for (String iface : interfaces) {
+ builder.append(".implements ").append(iface).append('\n');
+ }
if (makeInit) {
builder
.append(".method public <init>()V\n")
@@ -142,6 +159,19 @@
}
return builder.toString();
}
+
+ void setIsInterface() {
+ isInterface = true;
+ }
+
+ public MethodSignature addDefaultConstructor() {
+ return addMethod("public", "<init>", Collections.emptyList(), "V",
+ ".limit stack 1",
+ ".limit locals 1",
+ " aload_0",
+ " invokenonvirtual " + superName + "/<init>()V",
+ " return");
+ }
}
private final List<ClassBuilder> classes = new ArrayList<>();
@@ -160,6 +190,19 @@
return builder;
}
+ public ClassBuilder addClass(String name, String superName, String... interfaces) {
+ ClassBuilder builder = new ClassBuilder(name, superName, interfaces);
+ classes.add(builder);
+ return builder;
+ }
+
+ public ClassBuilder addInterface(String name, String... interfaces) {
+ ClassBuilder builder = new ClassBuilder(name, "java/lang/Object", interfaces);
+ builder.setIsInterface();
+ classes.add(builder);
+ return builder;
+ }
+
public ImmutableList<ClassBuilder> getClasses() {
return ImmutableList.copyOf(classes);
}
diff --git a/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java b/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java
index 5579380..6d4a501 100644
--- a/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java
+++ b/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java
@@ -87,6 +87,10 @@
return runOnArt(compileWithD8(builder), main);
}
+ protected ProcessResult runOnArtD8Raw(JasminBuilder builder, String main) throws Exception {
+ return runOnArtRaw(compileWithD8(builder), main);
+ }
+
protected AndroidApp compileWithR8(JasminBuilder builder) throws Exception {
return compileWithR8(builder, null);
}
@@ -108,6 +112,13 @@
return runOnArt(result, main);
}
+ protected ProcessResult runOnArtR8Raw(JasminBuilder builder, String main,
+ Consumer<InternalOptions> optionsConsumer)
+ throws Exception {
+ AndroidApp result = compileWithR8(builder, optionsConsumer);
+ return runOnArtRaw(result, main);
+ }
+
private ProcessResult runDx(JasminBuilder builder, File classes, Path dex) throws Exception {
for (ClassBuilder clazz : builder.getClasses()) {
ClassFile file = new ClassFile();
@@ -124,7 +135,7 @@
return ToolHelper.runDX(args.toArray(new String[args.size()]));
}
- protected ProcessResult runOnArtDxRaw(JasminBuilder builder) throws Exception {
+ protected ProcessResult runDX(JasminBuilder builder) throws Exception {
return runDx(builder, temp.newFolder("classes_for_dx"),
temp.getRoot().toPath().resolve("classes.dex"));
}
@@ -142,6 +153,19 @@
return ToolHelper.runArtNoVerificationErrors(dex.toString(), main);
}
+ protected ProcessResult runOnArtDxRaw(JasminBuilder builder, String main) throws Exception {
+ Path dex = temp.getRoot().toPath().resolve("classes.dex");
+ ProcessResult result = runDx(builder, temp.newFolder("classes_for_dx"), dex);
+ if (result.exitCode != 0) {
+ System.out.println("Std out:");
+ System.out.println(result.stdout);
+ System.out.println("Std err:");
+ System.out.println(result.stderr);
+ assertEquals(0, result.exitCode);
+ }
+ return ToolHelper.runArtRaw(dex.toString(), main);
+ }
+
protected ProcessResult runOnArtRaw(AndroidApp app, String main) throws IOException {
Path out = temp.getRoot().toPath().resolve("out.zip");
app.writeToZip(out, OutputMode.Indexed);
@@ -154,14 +178,6 @@
return ToolHelper.runArtNoVerificationErrors(out.toString(), main);
}
- protected AndroidApp buildApplication(JasminBuilder builder) {
- try {
- return AndroidApp.fromClassProgramData(builder.buildClasses());
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- }
-
protected DexEncodedMethod getMethod(AndroidApp application, String clazz,
MethodSignature signature) {
return getMethod(application,
diff --git a/src/test/java/com/android/tools/r8/jasmin/JumpSubroutineTests.java b/src/test/java/com/android/tools/r8/jasmin/JumpSubroutineTests.java
index 779fea0..c07f125 100644
--- a/src/test/java/com/android/tools/r8/jasmin/JumpSubroutineTests.java
+++ b/src/test/java/com/android/tools/r8/jasmin/JumpSubroutineTests.java
@@ -30,7 +30,7 @@
// Uncaught translation error: com.android.dex.util.ExceptionWithContext: returning from
// invalid subroutine
// 1 error; aborting
- ProcessResult result = runOnArtDxRaw(builder);
+ ProcessResult result = runDX(builder);
assertNotEquals(0, result.exitCode);
assertTrue(result.stderr.contains("Uncaught translation error"));
assertTrue(result.stderr.contains("invalid subroutine"));
diff --git a/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java b/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java
new file mode 100644
index 0000000..8f32184
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/jasmin/MemberResolutionTest.java
@@ -0,0 +1,141 @@
+// Copyright (c) 2017, 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.jasmin;
+
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
+import org.junit.Assert;
+import org.junit.Ignore;
+import org.junit.Test;
+
+public class MemberResolutionTest extends JasminTestBase {
+
+ private static final String MAIN_CLASS = "Main";
+
+ @Test
+ public void lookupStaticFieldFromDiamondInterface() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder interfaceA = builder.addInterface("InterfaceA");
+ interfaceA.addStaticFinalField("aField", "I", "42");
+
+ ClassBuilder interfaceB = builder.addInterface("InterfaceB");
+ interfaceB.addStaticFinalField("aField", "I", "123");
+
+ builder.addInterface("SubInterface", "InterfaceB", "InterfaceA");
+
+ ClassBuilder mainClass = builder.addClass(MAIN_CLASS);
+ mainClass.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 1",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " getstatic SubInterface/aField I",
+ " invokevirtual java/io/PrintStream/print(I)V",
+ " return");
+
+ ensureSameOutput(builder);
+ }
+
+ @Test
+ public void lookupStaticFieldFromInterfaceNotSuper() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder superClass = builder.addClass("SuperClass");
+ superClass.addStaticFinalField("aField", "I", "42");
+
+ ClassBuilder iface = builder.addInterface("Interface");
+ iface.addStaticFinalField("aField", "I", "123");
+
+ builder.addClass("SubClass", "SuperClass", "Interface");
+
+ ClassBuilder mainClass = builder.addClass(MAIN_CLASS);
+ mainClass.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 1",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " getstatic SubClass/aField I",
+ " invokevirtual java/io/PrintStream/print(I)V",
+ " return");
+
+ ensureSameOutput(builder);
+ }
+
+ @Test
+ public void lookupStaticFieldFromSupersInterfaceNotSupersSuper() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder superSuperClass = builder.addClass("SuperSuperClass");
+ superSuperClass.addStaticFinalField("aField", "I", "123");
+
+ ClassBuilder iface = builder.addInterface("Interface");
+ iface.addStaticFinalField("aField", "I", "42");
+
+ builder.addClass("SuperClass", "SuperSuperClass", "Interface");
+
+ builder.addClass("SubClass", "SuperClass");
+
+ ClassBuilder mainClass = builder.addClass(MAIN_CLASS);
+ mainClass.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 1",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " getstatic SubClass/aField I",
+ " invokevirtual java/io/PrintStream/print(I)V",
+ " return");
+
+ ensureSameOutput(builder);
+ }
+
+ @Test
+ @Ignore("b/69101406")
+ public void lookupInstanceFieldWithShadowingStatic() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder superClass = builder.addClass("SuperClass");
+ superClass.addField("public", "aField", "I", "42");
+ superClass.addDefaultConstructor();
+
+ ClassBuilder interfaceA = builder.addInterface("Interface");
+ interfaceA.addStaticFinalField("aField", "I", "123");
+
+ ClassBuilder subClass = builder.addClass("SubClass", "SuperClass", "Interface");
+ subClass.addDefaultConstructor();
+
+ ClassBuilder mainClass = builder.addClass(MAIN_CLASS);
+ mainClass.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 1",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " new SubClass",
+ " dup",
+ " invokespecial SubClass/<init>()V",
+ " getfield SubClass/aField I",
+ " invokevirtual java/io/PrintStream/print(I)V",
+ " return");
+
+ ensureICCE(builder);
+ }
+
+
+ private void ensureSameOutput(JasminBuilder app) throws Exception {
+ String dxOutput = runOnArtDx(app, MAIN_CLASS);
+ String d8Output = runOnArtD8(app, MAIN_CLASS);
+ Assert.assertEquals(dxOutput, d8Output);
+ String r8Output = runOnArtR8(app, MAIN_CLASS);
+ Assert.assertEquals(dxOutput, r8Output);
+ String javaOutput = runOnJava(app, MAIN_CLASS);
+ Assert.assertEquals(javaOutput, r8Output);
+ }
+
+ private void ensureICCE(JasminBuilder app) throws Exception {
+ ProcessResult dxOutput = runOnArtDxRaw(app, MAIN_CLASS);
+ Assert.assertTrue(dxOutput.stderr.contains("IncompatibleClassChangeError"));
+ ProcessResult d8Output = runOnArtD8Raw(app, MAIN_CLASS);
+ Assert.assertTrue(d8Output.stderr.contains("IncompatibleClassChangeError"));
+ ProcessResult r8Output = runOnArtR8Raw(app, MAIN_CLASS, null);
+ Assert.assertTrue(r8Output.stderr.contains("IncompatibleClassChangeError"));
+ ProcessResult javaOutput = runOnJavaRaw(app, MAIN_CLASS);
+ Assert.assertTrue(javaOutput.stderr.contains("IncompatibleClassChangeError"));
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/jasmin/Regress65432240.java b/src/test/java/com/android/tools/r8/jasmin/Regress65432240.java
index 4919b9e..df0d6e2 100644
--- a/src/test/java/com/android/tools/r8/jasmin/Regress65432240.java
+++ b/src/test/java/com/android/tools/r8/jasmin/Regress65432240.java
@@ -56,7 +56,7 @@
String expected = runOnJava(builder, clazz.name);
- AndroidApp originalApplication = buildApplication(builder);
+ AndroidApp originalApplication = builder.build();
AndroidApp processedApplication = ToolHelper.runR8(originalApplication);
DexEncodedMethod method = getMethod(processedApplication, clazz.name, signature);
