Do not workaround javac bug that leads to wrong parameter annotations.
ASM internally corrects the number of parameter annotations when javac
gets it wrong. It does so by using a non-existing java.lang.Synthetic
annotation. In the MethodWriter it then undoes that again so that the
output will maintain the javac bug. Since we are not using the
MethodWriter, we get only half of the workaround, namely the bit that
adds these fake annotations. We therefore have to filter them out
ourselves to get the right (wrong) behavior. :)
R=zerny@google.com
Bug: 62300145
Change-Id: Ieec7d165d8b2a6eb664f965693de76801e691bb8
diff --git a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
index 1673470..3cc3ac4 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -24,7 +24,6 @@
import com.android.tools.r8.graph.DexValue.DexValueString;
import com.android.tools.r8.graph.DexValue.DexValueType;
import com.android.tools.r8.graph.JarCode.ReparseContext;
-import com.android.tools.r8.utils.InternalResource;
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
@@ -396,6 +395,7 @@
private final int parameterCount;
private List<DexAnnotation> annotations = null;
private DexValue defaultAnnotation = null;
+ private int fakeParameterAnnotations = 0;
private List<List<DexAnnotation>> parameterAnnotations = null;
private List<DexValue> parameterNames = null;
private List<DexValue> parameterFlags = null;
@@ -447,15 +447,28 @@
@Override
public AnnotationVisitor visitParameterAnnotation(int parameter, String desc, boolean visible) {
+ // ASM decided to workaround a javac bug that incorrectly deals with synthesized parameter
+ // annotations. However, that leads us to have different behavior than javac+jvm and
+ // dx+art. The workaround is to use a non-existing descriptor "Ljava/lang/Synthetic;" for
+ // exactly this case. In order to remove the workaround we ignore all annotations
+ // with that descriptor. If javac is fixed, the ASM workaround will not be hit and we will
+ // never see this non-existing annotation descriptor. ASM uses the same check to make
+ // sure to undo their workaround for the javac bug in their MethodWriter.
+ if (desc.equals("Ljava/lang/Synthetic;")) {
+ assert parameterAnnotations == null;
+ fakeParameterAnnotations++;
+ return null;
+ }
if (parameterAnnotations == null) {
- parameterAnnotations = new ArrayList<>(parameterCount);
- for (int i = 0; i < parameterCount; i++) {
+ int adjustedParameterCount = parameterCount - fakeParameterAnnotations;
+ parameterAnnotations = new ArrayList<>(adjustedParameterCount);
+ for (int i = 0; i < adjustedParameterCount; i++) {
parameterAnnotations.add(new ArrayList<>());
}
}
return createAnnotationVisitor(desc, visible,
mv == null ? null : mv.visitParameterAnnotation(parameter, desc, visible),
- parameterAnnotations.get(parameter), parent.application);
+ parameterAnnotations.get(parameter - fakeParameterAnnotations), parent.application);
}
@Override
diff --git a/src/test/examples/regress_62300145/Regress.java b/src/test/examples/regress_62300145/Regress.java
new file mode 100644
index 0000000..77aa6bd
--- /dev/null
+++ b/src/test/examples/regress_62300145/Regress.java
@@ -0,0 +1,31 @@
+// 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 regress_62300145;
+
+import java.lang.annotation.Annotation;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.reflect.Constructor;
+
+public class Regress {
+ @Retention(RetentionPolicy.RUNTIME)
+ public @interface A {
+ }
+
+ public class InnerClass {
+ public InnerClass(@A String p) {}
+ }
+
+ public static void main(String[] args) throws NoSuchMethodException {
+ Constructor<InnerClass> constructor =
+ InnerClass.class.getDeclaredConstructor(Regress.class, String.class);
+ int i = 0;
+ for (Annotation[] annotations : constructor.getParameterAnnotations()) {
+ System.out.print(i++ + ": ");
+ for (Annotation annotation : annotations) {
+ System.out.println(annotation);
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/R8RunExamplesTest.java b/src/test/java/com/android/tools/r8/R8RunExamplesTest.java
index 4a06085..ba4eea0 100644
--- a/src/test/java/com/android/tools/r8/R8RunExamplesTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunExamplesTest.java
@@ -57,58 +57,58 @@
@Parameters(name = "{0}{1}")
public static Collection<String[]> data() {
- String[][] tests = {
- {"arithmetic.Arithmetic", null},
- {"arrayaccess.ArrayAccess", "37=37"},
- {"barray.BArray", "bits 42 and bool true"},
- {"bridge.BridgeMethod", null},
- {"cse.CommonSubexpressionElimination", "1\n1\n2 2\n2\n3\n3\n4 4\n4\nA\nB\n"},
- {"constants.Constants", null},
- {"controlflow.ControlFlow", null},
- {"conversions.Conversions", null},
- {"floating_point_annotations.FloatingPointValuedAnnotationTest", null},
- {"filledarray.FilledArray", null},
- {"hello.Hello", "Hello, world"},
- {"ifstatements.IfStatements", null},
- {"instancevariable.InstanceVariable", "144=144"},
- {"instanceofstring.InstanceofString", "is-string:true"},
- {"invoke.Invoke", null},
- {"jumbostring.JumboString", null},
- {"loadconst.LoadConst", null},
- {"newarray.NewArray", null},
- {"regalloc.RegAlloc", null},
- {"returns.Returns", null},
- {"staticfield.StaticField", "101010\n101010\nABC\nABC\n"},
- {"stringbuilding.StringBuilding",
- "a2c-xyz-abc7xyz\ntrueABCDE1234232.21.101an Xstringbuilder"},
- {"switches.Switches", null},
- {"sync.Sync", null},
- {"throwing.Throwing", "Throwing"},
- {"trivial.Trivial", null},
- {"trycatch.TryCatch", "Success!"},
- {"nestedtrycatches.NestedTryCatches", "EXCEPTION: PRIMARY"},
- {"trycatchmany.TryCatchMany", "Success!"},
- {"invokeempty.InvokeEmpty", "AB"},
- {"regress.Regress", null},
- {"regress2.Regress2", "START\nLOOP\nLOOP\nLOOP\nLOOP\nLOOP\nEND"},
- {"regress_37726195.Regress", null},
- {"regress_37658666.Regress", null},
- {"regress_37875803.Regress", null},
- {"regress_37955340.Regress", null},
- {"memberrebinding2.Test", Integer.toString((8 * 9) / 2)},
- {"memberrebinding3.Test", null},
- {"minification.Minification", null},
- {"enclosingmethod.Main", null},
- {"interfaceinlining.Main", null},
- {"switchmaps.Switches", null},
+ String[] tests = {
+ "arithmetic.Arithmetic",
+ "arrayaccess.ArrayAccess",
+ "barray.BArray",
+ "bridge.BridgeMethod",
+ "cse.CommonSubexpressionElimination",
+ "constants.Constants",
+ "controlflow.ControlFlow",
+ "conversions.Conversions",
+ "floating_point_annotations.FloatingPointValuedAnnotationTest",
+ "filledarray.FilledArray",
+ "hello.Hello",
+ "ifstatements.IfStatements",
+ "instancevariable.InstanceVariable",
+ "instanceofstring.InstanceofString",
+ "invoke.Invoke",
+ "jumbostring.JumboString",
+ "loadconst.LoadConst",
+ "newarray.NewArray",
+ "regalloc.RegAlloc",
+ "returns.Returns",
+ "staticfield.StaticField",
+ "stringbuilding.StringBuilding",
+ "switches.Switches",
+ "sync.Sync",
+ "throwing.Throwing",
+ "trivial.Trivial",
+ "trycatch.TryCatch",
+ "nestedtrycatches.NestedTryCatches",
+ "trycatchmany.TryCatchMany",
+ "invokeempty.InvokeEmpty",
+ "regress.Regress",
+ "regress2.Regress2",
+ "regress_37726195.Regress",
+ "regress_37658666.Regress",
+ "regress_37875803.Regress",
+ "regress_37955340.Regress",
+ "regress_62300145.Regress",
+ "memberrebinding2.Test",
+ "memberrebinding3.Test",
+ "minification.Minification",
+ "enclosingmethod.Main",
+ "interfaceinlining.Main",
+ "switchmaps.Switches",
};
List<String[]> fullTestList = new ArrayList<>(tests.length * 2);
- for (String[] test : tests) {
- String qualified = test[0];
+ for (String test : tests) {
+ String qualified = test;
String pkg = qualified.substring(0, qualified.lastIndexOf('.'));
- fullTestList.add(new String[]{pkg, DEX_EXTENSION, qualified, test[1]});
- fullTestList.add(new String[]{pkg, JAR_EXTENSION, qualified, test[1]});
+ fullTestList.add(new String[]{pkg, DEX_EXTENSION, qualified});
+ fullTestList.add(new String[]{pkg, JAR_EXTENSION, qualified});
}
return fullTestList;
}
@@ -118,7 +118,6 @@
private final String name;
private final String mainClass;
- private final String expectedOutput;
private final String fileType;
private static Map<DexVm, List<String>> failsOn =
ImmutableMap.of(
@@ -150,11 +149,10 @@
)
);
- public R8RunExamplesTest(String name, String fileType, String mainClass, String expectedOutput) {
+ public R8RunExamplesTest(String name, String fileType, String mainClass) {
this.name = name;
this.fileType = fileType;
this.mainClass = mainClass;
- this.expectedOutput = expectedOutput;
}
private Path getInputFile() {
@@ -249,10 +247,6 @@
}
String output =
ToolHelper.checkArtOutputIdentical(original, generated.toString(), mainClass, version);
- if (expectedOutput != null && !expectedToFail) {
- assertTrue("'" + output + "' lacks '" + expectedOutput + "'",
- output.contains(expectedOutput));
- }
}
}
}
