Refactor building of signature contexts to prepare for validation Bug: 172999267 Bug: 182329331 Change-Id: I6661e5648b8fcfab17d49a0b1558d7ee9024f7db
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java index 581d808..8558e18 100644 --- a/src/main/java/com/android/tools/r8/R8.java +++ b/src/main/java/com/android/tools/r8/R8.java
@@ -33,7 +33,7 @@ import com.android.tools.r8.graph.DexReference; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.DirectMappedDexApplication; -import com.android.tools.r8.graph.GenericSignatureTypeVariableRemover; +import com.android.tools.r8.graph.GenericSignatureContextBuilder; import com.android.tools.r8.graph.GraphLens; import com.android.tools.r8.graph.InitClassLens; import com.android.tools.r8.graph.PrunedItems; @@ -377,8 +377,8 @@ shrinker -> shrinker.run(Mode.INITIAL_TREE_SHAKING)); // Build enclosing information and type-paramter information before pruning. - GenericSignatureTypeVariableRemover typeVariableRemover = - GenericSignatureTypeVariableRemover.create(appView, appView.appInfo().classes()); + GenericSignatureContextBuilder typeVariableRemover = + GenericSignatureContextBuilder.create(appView.appInfo().classes()); TreePruner pruner = new TreePruner(appViewWithLiveness); DirectMappedDexApplication prunedApp = pruner.run(executorService); @@ -403,8 +403,7 @@ annotationRemoverBuilder .build(appViewWithLiveness, removedClasses); annotationRemover.ensureValid().run(); - typeVariableRemover.removeDeadGenericSignatureTypeVariables(appView); - new GenericSignatureRewriter(appView, NamingLens.getIdentityLens()) + new GenericSignatureRewriter(appView, NamingLens.getIdentityLens(), typeVariableRemover) .run(appView.appInfo().classes(), executorService); } } finally { @@ -596,8 +595,8 @@ shrinker -> shrinker.run(enqueuer.getMode()), DefaultTreePrunerConfiguration.getInstance()); - GenericSignatureTypeVariableRemover typeVariableRemover = - GenericSignatureTypeVariableRemover.create(appView, appView.appInfo().classes()); + GenericSignatureContextBuilder genericContextBuilder = + GenericSignatureContextBuilder.create(appView.appInfo().classes()); TreePruner pruner = new TreePruner(appViewWithLiveness, treePrunerConfiguration); DirectMappedDexApplication application = pruner.run(executorService); @@ -638,7 +637,10 @@ AnnotationRemover.builder() .build(appView.withLiveness(), removedClasses) .run(); - typeVariableRemover.removeDeadGenericSignatureTypeVariables(appView); + new GenericSignatureRewriter( + appView, NamingLens.getIdentityLens(), genericContextBuilder) + .run(appView.appInfo().classes(), executorService); + // Synthesize fields for triggering class initializers. new ClassInitFieldSynthesizer(appViewWithLiveness).run(executorService); }
diff --git a/src/main/java/com/android/tools/r8/graph/GenericSignatureTypeVariableRemover.java b/src/main/java/com/android/tools/r8/graph/GenericSignatureContextBuilder.java similarity index 65% rename from src/main/java/com/android/tools/r8/graph/GenericSignatureTypeVariableRemover.java rename to src/main/java/com/android/tools/r8/graph/GenericSignatureContextBuilder.java index 7c0ee29..9f3330c 100644 --- a/src/main/java/com/android/tools/r8/graph/GenericSignatureTypeVariableRemover.java +++ b/src/main/java/com/android/tools/r8/graph/GenericSignatureContextBuilder.java
@@ -4,11 +4,11 @@ package com.android.tools.r8.graph; -import static com.android.tools.r8.graph.GenericSignatureTypeVariableRemover.TypeParameterContext.empty; -import static com.google.common.base.Predicates.alwaysFalse; +import static com.android.tools.r8.graph.GenericSignatureContextBuilder.TypeParameterContext.empty; import com.android.tools.r8.graph.GenericSignature.FormalTypeParameter; import com.android.tools.r8.graph.GenericSignature.MethodTypeSignature; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -18,9 +18,8 @@ import java.util.Set; import java.util.function.Predicate; -public class GenericSignatureTypeVariableRemover { +public class GenericSignatureContextBuilder { - private final DexType objectType; private final Map<DexReference, TypeParameterSubstitutions> formalsInfo; private final Map<DexReference, DexReference> enclosingInfo; @@ -51,7 +50,7 @@ } } - static class TypeParameterContext { + public static class TypeParameterContext { private static final TypeParameterContext EMPTY = new TypeParameterContext(Collections.emptyMap(), Collections.emptySet()); @@ -69,37 +68,99 @@ if (information == null) { return this; } - HashMap<String, DexType> newPruned = new HashMap<>(prunedParametersWithBounds); - HashSet<String> newLiveParameters = new HashSet<>(liveParameters); - information.parametersWithBounds.forEach( - (param, type) -> { - if (dead) { - newPruned.put(param, type); - newLiveParameters.remove(param); - } else { - newLiveParameters.add(param); - newPruned.remove(param); - } - }); - return new TypeParameterContext(newPruned, newLiveParameters); + return dead + ? addPrunedSubstitutions(information.parametersWithBounds) + : addLiveParameters(information.parametersWithBounds.keySet()); } public static TypeParameterContext empty() { return EMPTY; } + + public boolean isLiveParameter(String parameterName) { + return liveParameters.contains(parameterName); + } + + public DexType getPrunedSubstitution(String parameterName) { + assert !isLiveParameter(parameterName); + return prunedParametersWithBounds.get(parameterName); + } + + public TypeParameterContext addLiveParameters(Collection<String> typeParameters) { + if (typeParameters.isEmpty()) { + return this; + } + HashSet<String> newLiveParameters = new HashSet<>(); + newLiveParameters.addAll(liveParameters); + newLiveParameters.addAll(typeParameters); + HashMap<String, DexType> newPruned = new HashMap<>(); + prunedParametersWithBounds.forEach( + (name, type) -> { + if (!typeParameters.contains(name)) { + newPruned.put(name, type); + } + }); + return new TypeParameterContext(newPruned, newLiveParameters); + } + + public TypeParameterContext addPrunedSubstitutions(Map<String, DexType> substitutions) { + if (substitutions.isEmpty()) { + return this; + } + HashMap<String, DexType> newPruned = new HashMap<>(); + newPruned.putAll(prunedParametersWithBounds); + newPruned.putAll(substitutions); + HashSet<String> newLiveParameters = new HashSet<>(); + liveParameters.forEach( + name -> { + if (!substitutions.containsKey(name)) { + newLiveParameters.add(name); + } + }); + return new TypeParameterContext(newPruned, newLiveParameters); + } } - private GenericSignatureTypeVariableRemover( + public static class AlwaysLiveTypeParameterContext extends TypeParameterContext { + + private AlwaysLiveTypeParameterContext() { + super(Collections.emptyMap(), Collections.emptySet()); + } + + public static AlwaysLiveTypeParameterContext create() { + return new AlwaysLiveTypeParameterContext(); + } + + @Override + public boolean isLiveParameter(String parameterName) { + return true; + } + + @Override + public DexType getPrunedSubstitution(String parameterName) { + assert false; + return null; + } + + @Override + public TypeParameterContext addLiveParameters(Collection<String> typeParameters) { + return this; + } + + @Override + public TypeParameterContext addPrunedSubstitutions(Map<String, DexType> substitutions) { + return this; + } + } + + private GenericSignatureContextBuilder( Map<DexReference, TypeParameterSubstitutions> formalsInfo, - Map<DexReference, DexReference> enclosingInfo, - DexType objectType) { + Map<DexReference, DexReference> enclosingInfo) { this.formalsInfo = formalsInfo; this.enclosingInfo = enclosingInfo; - this.objectType = objectType; } - public static GenericSignatureTypeVariableRemover create( - AppView<?> appView, List<DexProgramClass> programClasses) { + public static GenericSignatureContextBuilder create(List<DexProgramClass> programClasses) { Map<DexReference, TypeParameterSubstitutions> formalsInfo = new IdentityHashMap<>(); Map<DexReference, DexReference> enclosingInfo = new IdentityHashMap<>(); programClasses.forEach( @@ -137,8 +198,13 @@ : enclosingMethodAttribute.getEnclosingClass()); } }); - return new GenericSignatureTypeVariableRemover( - formalsInfo, enclosingInfo, appView.dexItemFactory().objectType); + return new GenericSignatureContextBuilder(formalsInfo, enclosingInfo); + } + + public TypeParameterContext computeTypeParameterContext( + AppView<?> appView, DexReference reference, Predicate<DexType> wasPruned) { + assert !wasPruned.test(reference.getContextType()); + return computeTypeParameterContext(appView, reference, wasPruned, false); } private TypeParameterContext computeTypeParameterContext( @@ -175,7 +241,7 @@ return typeParameterContext.combine(formalsInfo.get(reference), prunedHere); } - private static boolean hasPrunedRelationship( + public boolean hasPrunedRelationship( AppView<?> appView, DexReference enclosingReference, DexType enclosedClassType, @@ -189,8 +255,11 @@ if (wasPruned.test(enclosingReference.getContextType()) || wasPruned.test(enclosedClassType)) { return true; } - DexClass enclosingClass = appView.definitionFor(enclosingReference.getContextType()); - DexClass enclosedClass = appView.definitionFor(enclosedClassType); + DexClass enclosingClass = + appView.definitionFor( + appView.graphLens().lookupClassType(enclosingReference.getContextType())); + DexClass enclosedClass = + appView.definitionFor(appView.graphLens().lookupClassType(enclosedClassType)); if (enclosingClass == null || enclosedClass == null) { return true; } @@ -207,68 +276,15 @@ } } - private static boolean hasGenericTypeVariables( + public boolean hasGenericTypeVariables( AppView<?> appView, DexType type, Predicate<DexType> wasPruned) { if (wasPruned.test(type)) { return false; } - DexClass clazz = appView.definitionFor(type); + DexClass clazz = appView.definitionFor(appView.graphLens().lookupClassType(type)); if (clazz == null || clazz.isNotProgramClass() || clazz.getClassSignature().isInvalid()) { return true; } return !clazz.getClassSignature().getFormalTypeParameters().isEmpty(); } - - public void removeDeadGenericSignatureTypeVariables(AppView<?> appView) { - Predicate<DexType> wasPruned = - appView.hasLiveness() ? appView.withLiveness().appInfo()::wasPruned : alwaysFalse(); - GenericSignaturePartialTypeArgumentApplier baseArgumentApplier = - GenericSignaturePartialTypeArgumentApplier.build( - objectType, - (enclosing, enclosed) -> hasPrunedRelationship(appView, enclosing, enclosed, wasPruned), - type -> hasGenericTypeVariables(appView, type, wasPruned)); - appView - .appInfo() - .classes() - .forEach( - clazz -> { - if (clazz.getClassSignature().isInvalid()) { - return; - } - TypeParameterContext computedClassFormals = - computeTypeParameterContext(appView, clazz.getType(), wasPruned, false); - GenericSignaturePartialTypeArgumentApplier classArgumentApplier = - baseArgumentApplier.addSubstitutionsAndVariables( - computedClassFormals.prunedParametersWithBounds, - computedClassFormals.liveParameters); - clazz.setClassSignature( - classArgumentApplier.visitClassSignature(clazz.getClassSignature())); - clazz - .methods() - .forEach( - method -> { - MethodTypeSignature methodSignature = method.getGenericSignature(); - if (methodSignature.hasSignature() - && method.getGenericSignature().isValid()) { - // The reflection api do not distinguish static methods context from - // virtual methods. - method.setGenericSignature( - classArgumentApplier - .buildForMethod(methodSignature.getFormalTypeParameters()) - .visitMethodSignature(methodSignature)); - } - }); - clazz - .instanceFields() - .forEach( - field -> { - if (field.getGenericSignature().hasSignature() - && field.getGenericSignature().isValid()) { - field.setGenericSignature( - classArgumentApplier.visitFieldTypeSignature( - field.getGenericSignature())); - } - }); - }); - } }
diff --git a/src/main/java/com/android/tools/r8/graph/GenericSignaturePartialTypeArgumentApplier.java b/src/main/java/com/android/tools/r8/graph/GenericSignaturePartialTypeArgumentApplier.java index c381074..22282ec 100644 --- a/src/main/java/com/android/tools/r8/graph/GenericSignaturePartialTypeArgumentApplier.java +++ b/src/main/java/com/android/tools/r8/graph/GenericSignaturePartialTypeArgumentApplier.java
@@ -14,52 +14,37 @@ import com.android.tools.r8.graph.GenericSignature.ReturnType; import com.android.tools.r8.graph.GenericSignature.TypeSignature; import com.android.tools.r8.graph.GenericSignature.WildcardIndicator; +import com.android.tools.r8.graph.GenericSignatureContextBuilder.TypeParameterContext; import com.android.tools.r8.utils.ListUtils; -import com.google.common.collect.ImmutableSet; -import java.util.Collections; import java.util.List; -import java.util.Map; -import java.util.Set; import java.util.function.BiPredicate; import java.util.function.Predicate; public class GenericSignaturePartialTypeArgumentApplier implements GenericSignatureVisitor { - private final Map<String, DexType> substitutions; - private final Set<String> liveTypeVariables; - private final DexType objectType; + private final TypeParameterContext typeParameterContext; private final BiPredicate<DexType, DexType> enclosingPruned; private final Predicate<DexType> hasGenericTypeParameters; + private final AppView<?> appView; private GenericSignaturePartialTypeArgumentApplier( - Map<String, DexType> substitutions, - Set<String> liveTypeVariables, - DexType objectType, + AppView<?> appView, + TypeParameterContext typeParameterContext, BiPredicate<DexType, DexType> enclosingPruned, Predicate<DexType> hasGenericTypeParameters) { - this.substitutions = substitutions; - this.liveTypeVariables = liveTypeVariables; - this.objectType = objectType; + this.appView = appView; + this.typeParameterContext = typeParameterContext; this.enclosingPruned = enclosingPruned; this.hasGenericTypeParameters = hasGenericTypeParameters; } public static GenericSignaturePartialTypeArgumentApplier build( - DexType objectType, + AppView<?> appView, + TypeParameterContext typeParameterContext, BiPredicate<DexType, DexType> enclosingPruned, Predicate<DexType> hasGenericTypeParameters) { return new GenericSignaturePartialTypeArgumentApplier( - Collections.emptyMap(), - Collections.emptySet(), - objectType, - enclosingPruned, - hasGenericTypeParameters); - } - - public GenericSignaturePartialTypeArgumentApplier addSubstitutionsAndVariables( - Map<String, DexType> substitutions, Set<String> liveTypeVariables) { - return new GenericSignaturePartialTypeArgumentApplier( - substitutions, liveTypeVariables, objectType, enclosingPruned, hasGenericTypeParameters); + appView, typeParameterContext, enclosingPruned, hasGenericTypeParameters); } public GenericSignaturePartialTypeArgumentApplier buildForMethod( @@ -67,23 +52,27 @@ if (formals.isEmpty()) { return this; } - ImmutableSet.Builder<String> liveVariablesBuilder = ImmutableSet.builder(); - liveVariablesBuilder.addAll(liveTypeVariables); - formals.forEach( - formal -> { - liveVariablesBuilder.add(formal.name); - }); return new GenericSignaturePartialTypeArgumentApplier( - substitutions, liveTypeVariables, objectType, enclosingPruned, hasGenericTypeParameters); + appView, + typeParameterContext.addLiveParameters( + ListUtils.map(formals, FormalTypeParameter::getName)), + enclosingPruned, + hasGenericTypeParameters); } @Override public ClassSignature visitClassSignature(ClassSignature classSignature) { + if (classSignature.hasNoSignature() || classSignature.isInvalid()) { + return classSignature; + } return classSignature.visit(this); } @Override public MethodTypeSignature visitMethodSignature(MethodTypeSignature methodSignature) { + if (methodSignature.hasNoSignature() || methodSignature.isInvalid()) { + return methodSignature; + } return methodSignature.visit(this); } @@ -205,6 +194,9 @@ @Override public FieldTypeSignature visitFieldTypeSignature(FieldTypeSignature fieldSignature) { + if (fieldSignature.hasNoSignature() || fieldSignature.isInvalid()) { + return fieldSignature; + } if (fieldSignature.isStar()) { return fieldSignature; } else if (fieldSignature.isClassTypeSignature()) { @@ -214,11 +206,10 @@ } else { assert fieldSignature.isTypeVariableSignature(); String typeVariableName = fieldSignature.asTypeVariableSignature().typeVariable(); - if (substitutions.containsKey(typeVariableName) - && !liveTypeVariables.contains(typeVariableName)) { - DexType substitution = substitutions.get(typeVariableName); + if (!typeParameterContext.isLiveParameter(typeVariableName)) { + DexType substitution = typeParameterContext.getPrunedSubstitution(typeVariableName); if (substitution == null) { - substitution = objectType; + substitution = appView.dexItemFactory().objectType; } return new ClassTypeSignature(substitution).asArgument(WildcardIndicator.NONE); }
diff --git a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java index c44e4e7..a29a001 100644 --- a/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java +++ b/src/main/java/com/android/tools/r8/naming/signature/GenericSignatureRewriter.java
@@ -4,23 +4,37 @@ package com.android.tools.r8.naming.signature; +import static com.google.common.base.Predicates.alwaysFalse; + import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexProgramClass; +import com.android.tools.r8.graph.DexType; +import com.android.tools.r8.graph.GenericSignatureContextBuilder; +import com.android.tools.r8.graph.GenericSignaturePartialTypeArgumentApplier; import com.android.tools.r8.graph.GenericSignatureTypeRewriter; import com.android.tools.r8.naming.NamingLens; import com.android.tools.r8.utils.ThreadUtils; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; +import java.util.function.BiPredicate; +import java.util.function.Predicate; // TODO(b/169516860): We should generalize this to handle rewriting of attributes in general. public class GenericSignatureRewriter { private final AppView<?> appView; private final NamingLens namingLens; + private final GenericSignatureContextBuilder contextBuilder; public GenericSignatureRewriter(AppView<?> appView, NamingLens namingLens) { + this(appView, namingLens, null); + } + + public GenericSignatureRewriter( + AppView<?> appView, NamingLens namingLens, GenericSignatureContextBuilder contextBuilder) { this.appView = appView; this.namingLens = namingLens; + this.contextBuilder = contextBuilder; } public void run(Iterable<? extends DexProgramClass> classes, ExecutorService executorService) @@ -34,20 +48,53 @@ // arguments. If that is the case, the ProguardMapMinifier will pass in all classes that is // either ProgramClass or has a mapping. This is then transitively called inside the // ClassNameMinifier. + Predicate<DexType> wasPruned = + appView.hasLiveness() ? appView.withLiveness().appInfo()::wasPruned : alwaysFalse(); + BiPredicate<DexType, DexType> hasPrunedRelationship = + (enclosing, enclosed) -> + contextBuilder.hasPrunedRelationship(appView, enclosing, enclosed, wasPruned); + Predicate<DexType> hasGenericTypeVariables = + type -> contextBuilder.hasGenericTypeVariables(appView, type, wasPruned); ThreadUtils.processItems( classes, clazz -> { + GenericSignaturePartialTypeArgumentApplier classArgumentApplier = + contextBuilder != null + ? GenericSignaturePartialTypeArgumentApplier.build( + appView, + contextBuilder.computeTypeParameterContext( + appView, clazz.getType(), wasPruned), + hasPrunedRelationship, + hasGenericTypeVariables) + : null; GenericSignatureTypeRewriter genericSignatureTypeRewriter = new GenericSignatureTypeRewriter(appView, clazz); - clazz.setClassSignature(genericSignatureTypeRewriter.rewrite(clazz.getClassSignature())); + clazz.setClassSignature( + genericSignatureTypeRewriter.rewrite( + classArgumentApplier != null + ? classArgumentApplier.visitClassSignature(clazz.getClassSignature()) + : clazz.getClassSignature())); clazz.forEachField( field -> field.setGenericSignature( - genericSignatureTypeRewriter.rewrite(field.getGenericSignature()))); + genericSignatureTypeRewriter.rewrite( + classArgumentApplier != null + ? classArgumentApplier.visitFieldTypeSignature( + field.getGenericSignature()) + : field.getGenericSignature()))); clazz.forEachMethod( method -> + // The reflection api do not distinguish static methods context and + // from virtual methods we therefore always base the context for a method on + // the class context. method.setGenericSignature( - genericSignatureTypeRewriter.rewrite(method.getGenericSignature()))); + genericSignatureTypeRewriter.rewrite( + classArgumentApplier != null + ? classArgumentApplier + .buildForMethod( + method.getGenericSignature().getFormalTypeParameters()) + .visitMethodSignature(method.getGenericSignature()) + : method.getGenericSignature()))); }, executorService); }
diff --git a/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignaturePartialTypeArgumentApplierTest.java b/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignaturePartialTypeArgumentApplierTest.java index 64f2ac3..1fb82ab 100644 --- a/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignaturePartialTypeArgumentApplierTest.java +++ b/src/test/java/com/android/tools/r8/graph/genericsignature/GenericSignaturePartialTypeArgumentApplierTest.java
@@ -13,12 +13,16 @@ import com.android.tools.r8.TestDiagnosticMessagesImpl; import com.android.tools.r8.TestParameters; import com.android.tools.r8.TestParametersCollection; +import com.android.tools.r8.graph.AppInfo; +import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexItemFactory; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.GenericSignature; import com.android.tools.r8.graph.GenericSignature.MethodTypeSignature; +import com.android.tools.r8.graph.GenericSignatureContextBuilder.TypeParameterContext; import com.android.tools.r8.graph.GenericSignaturePartialTypeArgumentApplier; import com.android.tools.r8.origin.Origin; +import com.android.tools.r8.utils.AndroidApp; import com.android.tools.r8.utils.BiPredicateUtils; import com.google.common.collect.ImmutableMap; import java.util.Collections; @@ -48,7 +52,7 @@ } @Test - public void testVariablesInOuterPosition() { + public void testVariablesInOuterPosition() throws Exception { runTest( ImmutableMap.of("T", objectType, "R", objectType), Collections.emptySet(), @@ -60,7 +64,7 @@ } @Test - public void testVariablesInInnerPosition() { + public void testVariablesInInnerPosition() throws Exception { runTest( ImmutableMap.of("T", objectType, "R", objectType), Collections.emptySet(), @@ -72,7 +76,7 @@ } @Test - public void testRemovingPrunedLink() { + public void testRemovingPrunedLink() throws Exception { runTest( Collections.emptyMap(), Collections.emptySet(), @@ -85,7 +89,7 @@ } @Test - public void testRemovedGenericArguments() { + public void testRemovedGenericArguments() throws Exception { runTest( Collections.emptyMap(), Collections.emptySet(), @@ -102,11 +106,17 @@ BiPredicate<DexType, DexType> removedLink, Predicate<DexType> hasFormalTypeParameters, String initialSignature, - String expectedRewrittenSignature) { + String expectedRewrittenSignature) + throws Exception { + AppView<AppInfo> appView = computeAppView(AndroidApp.builder().build()); GenericSignaturePartialTypeArgumentApplier argumentApplier = GenericSignaturePartialTypeArgumentApplier.build( - objectType, removedLink, hasFormalTypeParameters) - .addSubstitutionsAndVariables(substitutions, liveVariables); + appView, + TypeParameterContext.empty() + .addPrunedSubstitutions(substitutions) + .addLiveParameters(liveVariables), + removedLink, + hasFormalTypeParameters); TestDiagnosticMessagesImpl diagnosticsHandler = new TestDiagnosticMessagesImpl(); MethodTypeSignature methodTypeSignature = argumentApplier.visitMethodSignature(