Ensure correct replacing of super types when having raw types
Bug: 214509535
Change-Id: I6c809245fb9623c1d8d06fbc21db2a630b979717
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index a7362e5..c607e30 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -1129,7 +1129,7 @@
return false;
}
- // Rewrite generic signatures before we merge fields.
+ // Rewrite generic signatures before we merge a base with a generic signature.
rewriteGenericSignatures(target, source, directMethods.values(), virtualMethods.values());
// Convert out of DefaultInstanceInitializerCode, since this piece of code will require lens
@@ -1333,16 +1333,18 @@
assert false : "Type should be present in generic signature";
return null;
}
+ Map<String, FieldTypeSignature> substitutionMap = new HashMap<>();
List<FormalTypeParameter> formals = source.getClassSignature().getFormalTypeParameters();
if (genericArgumentsToSuperType.size() != formals.size()) {
- // TODO(b/214509535): Correctly rewrite signature when type arguments is empty.
- assert genericArgumentsToSuperType.isEmpty() : "Invalid argument count to formals";
- return null;
- }
- Map<String, FieldTypeSignature> substitutionMap = new HashMap<>();
- for (int i = 0; i < formals.size(); i++) {
- // It is OK to override a generic type variable so we just use put.
- substitutionMap.put(formals.get(i).getName(), genericArgumentsToSuperType.get(i));
+ if (!genericArgumentsToSuperType.isEmpty()) {
+ assert false : "Invalid argument count to formals";
+ return null;
+ }
+ } else {
+ for (int i = 0; i < formals.size(); i++) {
+ // It is OK to override a generic type variable so we just use put.
+ substitutionMap.put(formals.get(i).getName(), genericArgumentsToSuperType.get(i));
+ }
}
return GenericSignaturePartialTypeArgumentApplier.build(
appView,
@@ -1512,6 +1514,7 @@
.setApiLevelForDefinition(method.getApiLevelForDefinition())
.setApiLevelForCode(method.getApiLevelForDefinition())
.setIsLibraryMethodOverride(method.isLibraryMethodOverride())
+ .setGenericSignature(method.getGenericSignature())
.build();
if (method.accessFlags.isPromotedToPublic()) {
// The bridge is now the public method serving the role of the original method, and should
diff --git a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergingWithMissingTypeArgsSubstitutionTest.java b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergingWithMissingTypeArgsSubstitutionTest.java
index 50a658f..27b7093 100644
--- a/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergingWithMissingTypeArgsSubstitutionTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/vertical/VerticalClassMergingWithMissingTypeArgsSubstitutionTest.java
@@ -4,15 +4,18 @@
package com.android.tools.r8.classmerging.vertical;
-import static org.junit.Assert.assertThrows;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
-import com.android.tools.r8.CompilationFailedException;
-import com.android.tools.r8.DiagnosticsMatcher;
+import com.android.tools.r8.KeepConstantArguments;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoMethodStaticizing;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.utils.ExceptionDiagnostic;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
import java.lang.reflect.TypeVariable;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -33,23 +36,41 @@
@Test()
public void test() throws Exception {
- assertThrows(
- CompilationFailedException.class,
- () -> {
- testForR8Compat(parameters.getBackend())
- .addInnerClasses(getClass())
- .addKeepMainRule(Main.class)
- .addKeepClassRules(A.class)
- .addKeepAttributeSignature()
- .addVerticallyMergedClassesInspector(
- inspector -> inspector.assertMergedIntoSubtype(B.class))
- .enableInliningAnnotations()
- .setMinApi(parameters.getApiLevel())
- .compileWithExpectedDiagnostics(
- diagnostics ->
- diagnostics.assertErrorsMatch(
- DiagnosticsMatcher.diagnosticType(ExceptionDiagnostic.class)));
- });
+ testForR8Compat(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(Main.class)
+ .addKeepClassRules(A.class)
+ .addKeepAttributeSignature()
+ .addVerticallyMergedClassesInspector(
+ inspector -> inspector.assertMergedIntoSubtype(B.class))
+ .enableInliningAnnotations()
+ .enableNoMethodStaticizingAnnotations()
+ .enableConstantArgumentAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), Main.class)
+ .assertSuccessWithOutputLines("T", "Hello World")
+ .inspect(
+ inspector -> {
+ ClassSubject classSubject = inspector.clazz(C.class);
+ assertThat(classSubject, isPresentAndRenamed());
+ assertEquals(
+ "<T:Ljava/lang/Object;>L" + binaryName(A.class) + "<Ljava/lang/Object;>;",
+ classSubject.getFinalSignatureAttribute());
+ MethodSubject bar = classSubject.uniqueMethodWithName("bar");
+ assertThat(bar, isPresentAndRenamed());
+ assertEquals("(TT;)V", bar.getFinalSignatureAttribute());
+ // The NeverInline is transferred to the private vertically merged method, making
+ // it hard to lookup.
+ MethodSubject movedFooSubject =
+ classSubject.uniqueMethodThatMatches(
+ method ->
+ method.getMethod().getReference() != bar.getMethod().getReference()
+ && !method.isInstanceInitializer());
+ assertThat(movedFooSubject, isPresentAndRenamed());
+ assertEquals(
+ "(Ljava/lang/Object;)Ljava/lang/Object;",
+ movedFooSubject.getFinalSignatureAttribute());
+ });
}
static class Main {
@@ -60,7 +81,7 @@
stringC.getClass().getTypeParameters()) {
System.out.println(typeParameter.getName());
}
- stringC.m("Hello World");
+ stringC.bar("Hello World");
}
}
@@ -68,15 +89,23 @@
static class B<T> extends A<T> {
+ @KeepConstantArguments
+ @NoMethodStaticizing
@NeverInline
public T foo(T t) {
+ if (System.currentTimeMillis() == 0) {
+ throw new RuntimeException("Foo");
+ }
return t;
}
}
static class C<T> extends B {
- void m(T t) {
+ @NoMethodStaticizing
+ @KeepConstantArguments
+ @NeverInline
+ void bar(T t) {
System.out.println(foo(t));
}
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
index bd9081e..8a0bab6 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/MemberSubject.java
@@ -12,6 +12,7 @@
public abstract Signature getFinalSignature();
+ @Override
public String getOriginalName() {
return getOriginalName(true);
}
