Merge "Preserve IllegalAccessError during check-cast."
diff --git a/build.gradle b/build.gradle
index 7a44d6d..eed9769 100644
--- a/build.gradle
+++ b/build.gradle
@@ -342,6 +342,7 @@
"youtube/youtube.android_12.10",
"youtube/youtube.android_12.17",
"youtube/youtube.android_12.22",
+ "youtube/youtube.android_13.37",
"proguardsettings",
"proguard/proguard_internal_159423826",
"framework",
diff --git a/src/main/java/com/android/tools/r8/ExtractMarker.java b/src/main/java/com/android/tools/r8/ExtractMarker.java
index 100105f..6fd04b6 100644
--- a/src/main/java/com/android/tools/r8/ExtractMarker.java
+++ b/src/main/java/com/android/tools/r8/ExtractMarker.java
@@ -121,7 +121,7 @@
"Failed to read dex/vdex file `" + programFile + "`: '" + e.getMessage() + "'");
continue;
}
- System.out.print("In file: " + cwd.relativize(programFile));
+ System.out.print("In file: " + cwd.toAbsolutePath().relativize(programFile.toAbsolutePath()));
System.out.println(", " + extractDexSize(programFile) + " bytes:");
for (Marker marker : markers) {
if (marker == null) {
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index dc94041..78a9d3f 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -362,9 +362,8 @@
mainDexKeepRules = parser.getConfig().getRules();
}
- ProguardConfigurationParser parser = new ProguardConfigurationParser(
- factory, reporter,
- !allowPartiallyImplementedProguardOptions, allowTestProguardOptions);
+ ProguardConfigurationParser parser =
+ new ProguardConfigurationParser(factory, reporter, allowTestProguardOptions);
if (!proguardConfigs.isEmpty()) {
parser.parse(proguardConfigs);
}
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 f594c71..1ca8a58 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -112,7 +112,7 @@
public DexEncodedMethod lookupStaticTarget(DexMethod method) {
ResolutionResult resolutionResult = resolveMethod(method.holder, method);
DexEncodedMethod target = resolutionResult.asSingleTarget();
- return target == null || target.isStaticMethod() ? target : null;
+ return target == null || target.isStatic() ? target : null;
}
/**
@@ -145,7 +145,7 @@
}
resolutionResult = resolveMethod(contextClass.superType, method);
DexEncodedMethod target = resolutionResult.asSingleTarget();
- return target == null || !target.isStaticMethod() ? target : null;
+ return target == null || !target.isStatic() ? target : null;
}
/**
diff --git a/src/main/java/com/android/tools/r8/graph/DexClass.java b/src/main/java/com/android/tools/r8/graph/DexClass.java
index b86c4bd..bbc5922 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClass.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClass.java
@@ -350,6 +350,16 @@
return null;
}
+ @Override
+ public boolean isStatic() {
+ return accessFlags.isStatic();
+ }
+
+ @Override
+ public boolean isStaticMember() {
+ return false;
+ }
+
public DexEncodedMethod getClassInitializer() {
return Arrays.stream(directMethods()).filter(DexEncodedMethod::isClassInitializer).findAny()
.orElse(null);
diff --git a/src/main/java/com/android/tools/r8/graph/DexDefinition.java b/src/main/java/com/android/tools/r8/graph/DexDefinition.java
index c2ccfe3..9127b94 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDefinition.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDefinition.java
@@ -74,4 +74,8 @@
public static Stream<DexEncodedMethod> filterDexEncodedMethod(Stream<DexDefinition> stream) {
return filter(stream, DexDefinition::isDexEncodedMethod, DexDefinition::asDexEncodedMethod);
}
+
+ public abstract boolean isStatic();
+
+ public abstract boolean isStaticMember();
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
index 115e88a..42cc0ce 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedField.java
@@ -78,6 +78,16 @@
return this;
}
+ @Override
+ public boolean isStatic() {
+ return accessFlags.isStatic();
+ }
+
+ @Override
+ public boolean isStaticMember() {
+ return isStatic();
+ }
+
public boolean hasAnnotation() {
return !annotations.isEmpty();
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index 5af5739..3d83104 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -213,14 +213,18 @@
return (accessFlags.isPrivate() || accessFlags.isConstructor()) && !accessFlags.isStatic();
}
- /**
- * Returns true if this method can be invoked via invoke-static.
- */
- public boolean isStaticMethod() {
+ @Override
+ public boolean isStatic() {
checkIfObsolete();
return accessFlags.isStatic();
}
+ @Override
+ public boolean isStaticMember() {
+ checkIfObsolete();
+ return isStatic();
+ }
+
/**
* Returns true if this method is synthetic.
*/
diff --git a/src/main/java/com/android/tools/r8/graph/JarCode.java b/src/main/java/com/android/tools/r8/graph/JarCode.java
index f974aca..0a1ee94 100644
--- a/src/main/java/com/android/tools/r8/graph/JarCode.java
+++ b/src/main/java/com/android/tools/r8/graph/JarCode.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
+import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.InvalidDebugInfoException;
import com.android.tools.r8.graph.JarClassFileReader.ReparseContext;
import com.android.tools.r8.ir.code.IRCode;
@@ -241,7 +242,12 @@
private void parseCode(ReparseContext context, boolean useJsrInliner) {
SecondVisitor classVisitor = new SecondVisitor(createCodeLocator(context), useJsrInliner);
- new ClassReader(context.classCache).accept(classVisitor, ClassReader.SKIP_FRAMES);
+ try {
+ new ClassReader(context.classCache).accept(classVisitor, ClassReader.SKIP_FRAMES);
+ } catch (Exception exception) {
+ throw new CompilationError(
+ "Unable to parse method `" + method.toSourceString() + "`", exception);
+ }
}
protected BiFunction<String, String, JarCode> createCodeLocator(ReparseContext context) {
diff --git a/src/main/java/com/android/tools/r8/ir/code/IRCode.java b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
index 19bf655..96cc33a 100644
--- a/src/main/java/com/android/tools/r8/ir/code/IRCode.java
+++ b/src/main/java/com/android/tools/r8/ir/code/IRCode.java
@@ -688,27 +688,17 @@
}
public ConstNumber createIntConstant(int value) {
- return new ConstNumber(createValue(ValueType.INT), value);
- }
-
- public ConstNumber createTrue() {
- return new ConstNumber(createValue(ValueType.INT), 1);
- }
-
- public ConstNumber createFalse() {
- return new ConstNumber(createValue(ValueType.INT), 0);
+ Value out = createValue(ValueType.INT);
+ return new ConstNumber(out, value);
}
public final int getHighestBlockNumber() {
return blocks.stream().max(Comparator.comparingInt(BasicBlock::getNumber)).get().getNumber();
}
- public Instruction createConstNull(Instruction from) {
- return new ConstNumber(createValue(from.outType()), 0);
- }
-
public ConstNumber createConstNull() {
- return new ConstNumber(createValue(ValueType.OBJECT), 0);
+ Value out = createValue(ValueType.OBJECT);
+ return new ConstNumber(out, 0);
}
public boolean doAllThrowingInstructionsHavePositions() {
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaDescriptor.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaDescriptor.java
index e16e2d5..6836ad3 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaDescriptor.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaDescriptor.java
@@ -130,7 +130,7 @@
private boolean isInstanceMethod(DexEncodedMethod encodedMethod) {
assert encodedMethod != null;
- return !encodedMethod.accessFlags.isConstructor() && !encodedMethod.isStaticMethod();
+ return !encodedMethod.accessFlags.isConstructor() && !encodedMethod.isStatic();
}
private boolean isPrivateInstanceMethod(DexEncodedMethod encodedMethod) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index 9541aef..2fddda3 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -768,7 +768,7 @@
public void identifyInvokeSemanticsForInlining(
DexEncodedMethod method, IRCode code, OptimizationFeedback feedback) {
- if (method.isStaticMethod()) {
+ if (method.isStatic()) {
// Identifies if the method preserves class initialization after inlining.
feedback.markTriggerClassInitBeforeAnySideEffect(method,
triggersClassInitializationBeforeSideEffect(code, method.method.getHolder()));
@@ -1335,7 +1335,7 @@
if (current.isInvokeMethod()) {
InvokeMethod invoke = current.asInvokeMethod();
if (invoke.getInvokedMethod() == dexItemFactory.classMethods.desiredAssertionStatus) {
- iterator.replaceCurrentInstruction(code.createFalse());
+ iterator.replaceCurrentInstruction(code.createIntConstant(0));
}
} else if (current.isStaticPut()) {
StaticPut staticPut = current.asStaticPut();
@@ -1345,7 +1345,7 @@
} else if (current.isStaticGet()) {
StaticGet staticGet = current.asStaticGet();
if (staticGet.getField().name == dexItemFactory.assertionsDisabled) {
- iterator.replaceCurrentInstruction(code.createTrue());
+ iterator.replaceCurrentInstruction(code.createIntConstant(1));
}
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/MethodPoolCollection.java b/src/main/java/com/android/tools/r8/ir/optimize/MethodPoolCollection.java
index bd3acb6..49c740c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/MethodPoolCollection.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/MethodPoolCollection.java
@@ -97,7 +97,7 @@
clazz.forEachMethod(
encodedMethod -> {
// We will add private instance methods when we promote them.
- if (!encodedMethod.isPrivateMethod() || encodedMethod.isStaticMethod()) {
+ if (!encodedMethod.isPrivateMethod() || encodedMethod.isStatic()) {
methodPool.seen(equivalence.wrap(encodedMethod.method));
}
});
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaGroupIdFactory.java b/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaGroupIdFactory.java
index 42e0bb1..605fdf9 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaGroupIdFactory.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/lambda/kotlin/KotlinLambdaGroupIdFactory.java
@@ -155,7 +155,7 @@
method.accessFlags, CLASS_INITIALIZER_FLAGS);
checkDirectMethodAnnotations(method);
- } else if (method.isStaticMethod()) {
+ } else if (method.isStatic()) {
throw new LambdaStructureError(
"unexpected static method: " + method.method.toSourceString());
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java
index eaa5037..e33d977 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizer.java
@@ -180,7 +180,7 @@
return true;
}
for (DexEncodedMethod method : clazz.methods()) {
- if (!method.isStaticMethod() && appInfo.isPinned(method.method)) {
+ if (!method.isStatic() && appInfo.isPinned(method.method)) {
return true;
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
index 5b930fe..b7c830f 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/staticizer/StaticizingProcessor.java
@@ -130,7 +130,7 @@
// CHECK: no abstract or native instance methods.
if (Streams.stream(candidateClass.methods()).anyMatch(
- method -> !method.isStaticMethod() && (method.shouldNotHaveCode()))) {
+ method -> !method.isStatic() && (method.shouldNotHaveCode()))) {
it.remove();
continue;
}
@@ -151,7 +151,7 @@
// Collect instance methods to be staticized.
for (DexEncodedMethod method : candidateClass.methods()) {
- if (!method.isStaticMethod()) {
+ if (!method.isStatic()) {
removedInstanceMethods.add(method);
if (!factory().isConstructor(method.method)) {
methodsToBeStaticized.add(method);
@@ -334,7 +334,7 @@
// Move instance methods into static ones.
List<DexEncodedMethod> newDirectMethods = new ArrayList<>();
for (DexEncodedMethod method : candidateClass.methods()) {
- if (method.isStaticMethod()) {
+ if (method.isStatic()) {
newDirectMethods.add(method);
} else if (!factory().isConstructor(method.method)) {
DexEncodedMethod staticizedMethod = method.toStaticMethodWithoutThis();
@@ -374,7 +374,7 @@
}
private boolean classMembersConflict(DexClass a, DexClass b) {
- assert Streams.stream(a.methods()).allMatch(DexEncodedMethod::isStaticMethod);
+ assert Streams.stream(a.methods()).allMatch(DexEncodedMethod::isStatic);
assert a.instanceFields().length == 0;
return Stream.of(a.staticFields()).anyMatch(fld -> b.lookupField(fld.field) != null) ||
Streams.stream(a.methods()).anyMatch(method -> b.lookupMethod(method.method) != null);
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index afccf95..7a0aacb 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -272,6 +272,24 @@
}
}
+ private void enqueueHolderIfDependentNonStaticMember(
+ DexClass holder, Map<DexDefinition, ProguardKeepRule> dependentItems) {
+ // Check if any dependent members are not static, and in that case enqueue the class as well.
+ // Having a dependent rule like -keepclassmembers with non static items indicates that class
+ // instances will be present even if tracing do not find any instantiation. See b/115867670.
+ for (Entry<DexDefinition, ProguardKeepRule> entry : dependentItems.entrySet()) {
+ DexDefinition dependentItem = entry.getKey();
+ if (dependentItem.isDexClass()) {
+ continue;
+ }
+ if (!dependentItem.isStaticMember()) {
+ enqueueRootItem(holder, entry.getValue());
+ // Enough to enqueue the known holder once.
+ break;
+ }
+ }
+ }
+
//
// Things to do with registering events. This is essentially the interface for byte-code
// traversals.
@@ -633,16 +651,6 @@
// Actual actions performed.
//
- static private boolean isStaticMember(DexDefinition definition) {
- if (definition.isDexEncodedMethod()) {
- return (definition.asDexEncodedMethod()).accessFlags.isStatic();
- }
- if (definition.isDexEncodedField()) {
- return (definition.asDexEncodedField()).accessFlags.isStatic();
- }
- return false;
- }
-
private void markTypeAsLive(DexType type) {
assert type.isClassType();
if (liveTypes.add(type)) {
@@ -686,16 +694,8 @@
annotations.forEach(this::handleAnnotationOfLiveType);
}
- // Check if any dependent members are not static, and in that case enqueue the class as well.
- // Having a dependent rule like -keepclassmembers with non static items indicates that class
- // instances will be present even if tracing do not find any instantiation. See b/115867670.
Map<DexDefinition, ProguardKeepRule> dependentItems = rootSet.getDependentItems(holder);
- for (Entry<DexDefinition, ProguardKeepRule> entry : dependentItems.entrySet()) {
- if (!isStaticMember(entry.getKey())) {
- enqueueRootItem(holder, entry.getValue());
- break;
- }
- }
+ enqueueHolderIfDependentNonStaticMember(holder, dependentItems);
// Add all dependent members to the workqueue.
enqueueRootItems(dependentItems);
}
@@ -1298,6 +1298,16 @@
enqueueRootItems(consequentRootSet.noShrinking);
rootSet.noOptimization.addAll(consequentRootSet.noOptimization);
rootSet.noObfuscation.addAll(consequentRootSet.noObfuscation);
+ rootSet.addDependentItems(consequentRootSet.dependentNoShrinking);
+ // Check if any newly dependent members are not static, and in that case find the holder
+ // and enqueue it as well. This is -if version of workaround for b/115867670.
+ consequentRootSet.dependentNoShrinking.forEach((precondition, dependentItems) -> {
+ if (precondition.isDexClass()) {
+ enqueueHolderIfDependentNonStaticMember(precondition.asDexClass(), dependentItems);
+ }
+ // Add all dependent members to the workqueue.
+ enqueueRootItems(dependentItems);
+ });
if (!workList.isEmpty()) {
continue;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
index a49c15c..faf5317 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -49,7 +49,6 @@
private final DexItemFactory dexItemFactory;
private final Reporter reporter;
- private final boolean failOnPartiallyImplementedOptions;
private final boolean allowTestOptions;
private static final List<String> IGNORED_SINGLE_ARG_OPTIONS = ImmutableList.of(
@@ -102,17 +101,15 @@
public ProguardConfigurationParser(
DexItemFactory dexItemFactory, Reporter reporter) {
- this(dexItemFactory, reporter, true, false);
+ this(dexItemFactory, reporter, false);
}
public ProguardConfigurationParser(
- DexItemFactory dexItemFactory, Reporter reporter, boolean failOnPartiallyImplementedOptions,
- boolean allowTestOptions) {
+ DexItemFactory dexItemFactory, Reporter reporter, boolean allowTestOptions) {
this.dexItemFactory = dexItemFactory;
configurationBuilder = ProguardConfiguration.builder(dexItemFactory, reporter);
this.reporter = reporter;
- this.failOnPartiallyImplementedOptions = failOnPartiallyImplementedOptions;
this.allowTestOptions = allowTestOptions;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java b/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
index 54a3f2f..6d6d943 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
@@ -72,7 +72,7 @@
Origin.unknown(),
Position.UNKNOWN,
null,
- getClassAnnotation(),
+ getClassAnnotation() == null ? null : getClassAnnotation().materialize(),
getClassAccessFlags(),
getNegatedClassAccessFlags(),
getClassTypeNegated(),
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardKeepRule.java b/src/main/java/com/android/tools/r8/shaking/ProguardKeepRule.java
index 3c1fb2d..dd2bd33 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardKeepRule.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardKeepRule.java
@@ -63,7 +63,7 @@
Origin.unknown(),
Position.UNKNOWN,
null,
- getClassAnnotation(),
+ getClassAnnotation() == null ? null : getClassAnnotation().materialize(),
getClassAccessFlags(),
getNegatedClassAccessFlags(),
getClassTypeNegated(),
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
index 43227d7..30def9e 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -27,6 +27,7 @@
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.ThreadUtils;
import com.google.common.base.Equivalence.Wrapper;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Sets;
import java.io.PrintStream;
import java.util.ArrayList;
@@ -34,10 +35,12 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
@@ -148,16 +151,13 @@
DexClass clazz,
ProguardConfigurationRule rule,
ProguardIfRule ifRule) {
- if (rule.getClassType().matches(clazz) == rule.getClassTypeNegated()) {
+ if (!satisfyClassType(rule, clazz)) {
return;
}
- if (!rule.getClassAccessFlags().containsAll(clazz.accessFlags)) {
+ if (!satisfyAccessFlag(rule, clazz)) {
return;
}
- if (!rule.getNegatedClassAccessFlags().containsNone(clazz.accessFlags)) {
- return;
- }
- if (!containsAnnotation(rule.getClassAnnotation(), clazz.annotations)) {
+ if (!satisfyAnnotation(rule, clazz)) {
return;
}
// In principle it should make a difference whether the user specified in a class
@@ -202,13 +202,14 @@
if (rule.getClassNames().matches(clazz.type)) {
Collection<ProguardMemberRule> memberKeepRules = rule.getMemberRules();
+ Map<Predicate<DexDefinition>, DexDefinition> preconditionSupplier;
if (rule instanceof ProguardKeepRule) {
switch (((ProguardKeepRule) rule).getType()) {
case KEEP_CLASS_MEMBERS: {
- // If we're handling -if consequent part, that means precondition already met.
- DexClass precondition = ifRule != null ? null : clazz;
- markMatchingVisibleMethods(clazz, memberKeepRules, rule, precondition);
- markMatchingFields(clazz, memberKeepRules, rule, precondition);
+ // Members mentioned at -keepclassmembers always depend on their holder.
+ preconditionSupplier = ImmutableMap.of((definition -> true), clazz);
+ markMatchingVisibleMethods(clazz, memberKeepRules, rule, preconditionSupplier);
+ markMatchingFields(clazz, memberKeepRules, rule, preconditionSupplier);
break;
}
case KEEP_CLASSES_WITH_MEMBERS: {
@@ -219,20 +220,33 @@
}
case KEEP: {
markClass(clazz, rule);
- markMatchingVisibleMethods(clazz, memberKeepRules, rule, null);
- markMatchingFields(clazz, memberKeepRules, rule, null);
+ preconditionSupplier = new HashMap<>();
+ if (ifRule != null) {
+ // Static members in -keep are pinned no matter what.
+ preconditionSupplier.put(DexDefinition::isStaticMember, null);
+ // Instance members may need to be kept even though the holder is not instantiated.
+ preconditionSupplier.put(definition -> !definition.isStaticMember(), clazz);
+ } else {
+ // Members mentioned at -keep should always be pinned as long as that -keep rule is
+ // not triggered conditionally.
+ preconditionSupplier.put((definition -> true), null);
+ }
+ markMatchingVisibleMethods(clazz, memberKeepRules, rule, preconditionSupplier);
+ markMatchingFields(clazz, memberKeepRules, rule, preconditionSupplier);
break;
}
case CONDITIONAL:
- assert rule instanceof ProguardIfRule;
throw new Unreachable("-if rule will be evaluated separately, not here.");
}
+ } else if (rule instanceof ProguardIfRule) {
+ throw new Unreachable("-if rule will be evaluated separately, not here.");
} else if (rule instanceof ProguardCheckDiscardRule) {
if (memberKeepRules.isEmpty()) {
markClass(clazz, rule);
} else {
- markMatchingFields(clazz, memberKeepRules, rule, clazz);
- markMatchingMethods(clazz, memberKeepRules, rule, clazz);
+ preconditionSupplier = ImmutableMap.of((definition -> true), clazz);
+ markMatchingFields(clazz, memberKeepRules, rule, preconditionSupplier);
+ markMatchingMethods(clazz, memberKeepRules, rule, preconditionSupplier);
}
} else if (rule instanceof ProguardWhyAreYouKeepingRule
|| rule instanceof ProguardKeepPackageNamesRule) {
@@ -344,6 +358,19 @@
// -keep rule may vary (due to back references). So, we need to try all pairs of -if rule
// and live types.
for (DexType currentLiveType : liveTypes) {
+ DexClass currentLiveClass = appInfo.definitionFor(currentLiveType);
+ if (currentLiveClass == null) {
+ continue;
+ }
+ if (!satisfyClassType(rule, currentLiveClass)) {
+ continue;
+ }
+ if (!satisfyAccessFlag(rule, currentLiveClass)) {
+ continue;
+ }
+ if (!satisfyAnnotation(rule, currentLiveClass)) {
+ continue;
+ }
if (ifRule.hasInheritanceClassName()) {
if (!satisfyInheritanceRule(currentLiveType, definitionForWithLiveTypes, ifRule)) {
// Try another live type since the current one doesn't satisfy the inheritance rule.
@@ -406,20 +433,44 @@
} finally {
application.timing.end();
}
- return new ConsequentRootSet(noShrinking, noOptimization, noObfuscation);
+ return new ConsequentRootSet(noShrinking, noOptimization, noObfuscation, dependentNoShrinking);
+ }
+
+ private static DexDefinition testAndGetPrecondition(
+ DexDefinition definition, Map<Predicate<DexDefinition>, DexDefinition> preconditionSupplier) {
+ if (preconditionSupplier == null) {
+ return null;
+ }
+ DexDefinition precondition = null;
+ boolean conditionEverMatched = false;
+ for (Entry<Predicate<DexDefinition>, DexDefinition> entry : preconditionSupplier.entrySet()) {
+ if (entry.getKey().test(definition)) {
+ precondition = entry.getValue();
+ conditionEverMatched = true;
+ break;
+ }
+ }
+ // If precondition-supplier is given, there should be at least one predicate that holds.
+ // Actually, there should be only one predicate as we break the loop when it is found.
+ assert conditionEverMatched;
+ return precondition;
}
private void markMatchingVisibleMethods(
DexClass clazz,
Collection<ProguardMemberRule> memberKeepRules,
ProguardConfigurationRule rule,
- DexClass onlyIfClassKept) {
+ Map<Predicate<DexDefinition>, DexDefinition> preconditionSupplier) {
Set<Wrapper<DexMethod>> methodsMarked = new HashSet<>();
- Arrays.stream(clazz.directMethods()).forEach(method ->
- markMethod(method, memberKeepRules, methodsMarked, rule, onlyIfClassKept));
+ Arrays.stream(clazz.directMethods()).forEach(method -> {
+ DexDefinition precondition = testAndGetPrecondition(method, preconditionSupplier);
+ markMethod(method, memberKeepRules, methodsMarked, rule, precondition);
+ });
while (clazz != null) {
- Arrays.stream(clazz.virtualMethods()).forEach(method ->
- markMethod(method, memberKeepRules, methodsMarked, rule, onlyIfClassKept));
+ Arrays.stream(clazz.virtualMethods()).forEach(method -> {
+ DexDefinition precondition = testAndGetPrecondition(method, preconditionSupplier);
+ markMethod(method, memberKeepRules, methodsMarked, rule, precondition);
+ });
clazz = clazz.superType == null ? null : application.definitionFor(clazz.superType);
}
}
@@ -428,19 +479,22 @@
DexClass clazz,
Collection<ProguardMemberRule> memberKeepRules,
ProguardConfigurationRule rule,
- DexClass onlyIfClassKept) {
- Arrays.stream(clazz.directMethods()).forEach(method ->
- markMethod(method, memberKeepRules, null, rule, onlyIfClassKept));
- Arrays.stream(clazz.virtualMethods()).forEach(method ->
- markMethod(method, memberKeepRules, null, rule, onlyIfClassKept));
+ Map<Predicate<DexDefinition>, DexDefinition> preconditionSupplier) {
+ clazz.forEachMethod(method -> {
+ DexDefinition precondition = testAndGetPrecondition(method, preconditionSupplier);
+ markMethod(method, memberKeepRules, null, rule, precondition);
+ });
}
private void markMatchingFields(
DexClass clazz,
Collection<ProguardMemberRule> memberKeepRules,
ProguardConfigurationRule rule,
- DexClass onlyIfClassKept) {
- clazz.forEachField(field -> markField(field, memberKeepRules, rule, onlyIfClassKept));
+ Map<Predicate<DexDefinition>, DexDefinition> preconditionSupplier) {
+ clazz.forEachField(field -> {
+ DexDefinition precondition = testAndGetPrecondition(field, preconditionSupplier);
+ markField(field, memberKeepRules, rule, precondition);
+ });
}
// TODO(67934426): Test this code.
@@ -496,6 +550,19 @@
out.close();
}
+ private static boolean satisfyClassType(ProguardConfigurationRule rule, DexClass clazz) {
+ return rule.getClassType().matches(clazz) != rule.getClassTypeNegated();
+ }
+
+ private static boolean satisfyAccessFlag(ProguardConfigurationRule rule, DexClass clazz) {
+ return rule.getClassAccessFlags().containsAll(clazz.accessFlags)
+ && rule.getNegatedClassAccessFlags().containsNone(clazz.accessFlags);
+ }
+
+ private static boolean satisfyAnnotation(ProguardConfigurationRule rule, DexClass clazz) {
+ return containsAnnotation(rule.getClassAnnotation(), clazz.annotations);
+ }
+
private boolean satisfyInheritanceRule(
DexType type,
Function<DexType, DexClass> definitionFor,
@@ -794,6 +861,15 @@
this.ifRules = Collections.unmodifiableSet(ifRules);
}
+ // Add dependent items that depend on -if rules.
+ void addDependentItems(
+ Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentItems) {
+ dependentItems.forEach((def, dependence) -> {
+ dependentNoShrinking.computeIfAbsent(def, x -> new IdentityHashMap<>())
+ .putAll(dependence);
+ });
+ }
+
Map<DexDefinition, ProguardKeepRule> getDependentItems(DexDefinition item) {
return Collections
.unmodifiableMap(dependentNoShrinking.getOrDefault(item, Collections.emptyMap()));
@@ -831,14 +907,17 @@
final Map<DexDefinition, ProguardKeepRule> noShrinking;
final Set<DexDefinition> noOptimization;
final Set<DexDefinition> noObfuscation;
+ final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking;
private ConsequentRootSet(
Map<DexDefinition, ProguardKeepRule> noShrinking,
Set<DexDefinition> noOptimization,
- Set<DexDefinition> noObfuscation) {
+ Set<DexDefinition> noObfuscation,
+ Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking) {
this.noShrinking = Collections.unmodifiableMap(noShrinking);
this.noOptimization = Collections.unmodifiableSet(noOptimization);
this.noObfuscation = Collections.unmodifiableSet(noObfuscation);
+ this.dependentNoShrinking = Collections.unmodifiableMap(dependentNoShrinking);
}
}
}
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 43f8d20..5e03071 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -811,7 +811,7 @@
deferredRenamings.map(directMethod.method, resultingDirectMethod.method);
deferredRenamings.recordMove(directMethod.method, resultingDirectMethod.method);
- if (!directMethod.isStaticMethod()) {
+ if (!directMethod.isStatic()) {
blockRedirectionOfSuperCalls(resultingDirectMethod.method);
}
}
@@ -1069,7 +1069,7 @@
accessFlags.setSynthetic();
accessFlags.unsetAbstract();
- assert invocationTarget.isPrivateMethod() == !invocationTarget.isStaticMethod();
+ assert invocationTarget.isPrivateMethod() == !invocationTarget.isStatic();
SynthesizedBridgeCode code =
new SynthesizedBridgeCode(
newMethod,
diff --git a/src/test/java/com/android/tools/r8/debug/LocalsLiveAtBlockEntryDebugTest.java b/src/test/java/com/android/tools/r8/debug/LocalsLiveAtBlockEntryDebugTest.java
index 09a46e5..85749ab 100644
--- a/src/test/java/com/android/tools/r8/debug/LocalsLiveAtBlockEntryDebugTest.java
+++ b/src/test/java/com/android/tools/r8/debug/LocalsLiveAtBlockEntryDebugTest.java
@@ -6,6 +6,9 @@
import static org.junit.Assert.assertTrue;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
+import com.android.tools.r8.VmTestRunner;
+import com.android.tools.r8.VmTestRunner.IgnoreIfVmOlderThan;
import com.android.tools.r8.jasmin.JasminBuilder;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
@@ -14,10 +17,10 @@
import java.nio.file.Path;
import java.util.Collections;
import java.util.List;
-import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
/**
* Test to check that locals that are introduced in a block that is not hit, still start in the
@@ -25,6 +28,7 @@
*
* <p>See b/75251251 or b/78617758
*/
+@RunWith(VmTestRunner.class)
public class LocalsLiveAtBlockEntryDebugTest extends DebugTestBase {
final String className = "LocalsLiveAtEntry";
@@ -45,7 +49,7 @@
}
@Test
- @Ignore("b/78617758")
+ @IgnoreIfVmOlderThan(Version.V7_0_0)
public void testD8() throws Throwable {
JasminBuilder builder = getBuilderForTest(className, methodName);
List<Path> outputs = builder.writeClassFiles(temp.newFolder().toPath());
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java
index 9be118c..c184e6e 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/staticizer/ClassStaticizerTest.java
@@ -185,7 +185,7 @@
assertNotNull(clazz);
assertTrue(clazz.isPresent());
return Streams.stream(clazz.getDexClass().methods())
- .filter(method -> !method.isStaticMethod())
+ .filter(method -> !method.isStatic())
.map(method -> method.method.toSourceString())
.sorted()
.collect(Collectors.toList());
diff --git a/src/test/java/com/android/tools/r8/shaking/NonVirtualOverrideTest.java b/src/test/java/com/android/tools/r8/shaking/NonVirtualOverrideTest.java
index 53c9f83..2490d41 100644
--- a/src/test/java/com/android/tools/r8/shaking/NonVirtualOverrideTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/NonVirtualOverrideTest.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.ClassFileConsumer.ArchiveConsumer;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.android.tools.r8.jasmin.JasminBuilder;
@@ -87,22 +88,47 @@
.build();
// Run the program using java.
- Path referenceJar = temp.getRoot().toPath().resolve("input.jar");
- ArchiveConsumer inputConsumer = new ArchiveConsumer(referenceJar);
- for (Class<?> clazz : ImmutableList.of(main, A, C)) {
+ String referenceResult;
+ if (backend == Backend.DEX
+ && ToolHelper.getDexVm().getVersion().isOlderThanOrEqual(Version.V7_0_0)
+ && ToolHelper.getDexVm().getVersion().isAtLeast(Version.V5_1_1)) {
+ referenceResult =
+ String.join(
+ System.lineSeparator(),
+ "In A.m1()",
+ "In A.m2()",
+ "In A.m3()",
+ "In A.m4()",
+ "In C.m1()",
+ "In A.m2()",
+ "In C.m3()",
+ "In A.m4()",
+ "In A.m1()", // With Java: Caught IllegalAccessError when calling B.m1()
+ "In A.m3()", // With Java: Caught IncompatibleClassChangeError when calling B.m3()
+ "In C.m1()", // With Java: Caught IllegalAccessError when calling B.m1()
+ "In C.m3()", // With Java: Caught IncompatibleClassChangeError when calling B.m3()
+ "In C.m1()",
+ "In C.m3()",
+ "");
+ } else {
+ Path referenceJar = temp.getRoot().toPath().resolve("input.jar");
+ ArchiveConsumer inputConsumer = new ArchiveConsumer(referenceJar);
+ for (Class<?> clazz : ImmutableList.of(main, A, C)) {
+ inputConsumer.accept(
+ ByteDataView.of(ToolHelper.getClassAsBytes(clazz)),
+ DescriptorUtils.javaTypeToDescriptor(clazz.getName()),
+ null);
+ }
inputConsumer.accept(
- ByteDataView.of(ToolHelper.getClassAsBytes(clazz)),
- DescriptorUtils.javaTypeToDescriptor(clazz.getName()),
+ ByteDataView.of(jasminBuilder.buildClasses().get(0)),
+ DescriptorUtils.javaTypeToDescriptor(B.getName()),
null);
- }
- inputConsumer.accept(
- ByteDataView.of(jasminBuilder.buildClasses().get(0)),
- DescriptorUtils.javaTypeToDescriptor(B.getName()),
- null);
- inputConsumer.finished(null);
+ inputConsumer.finished(null);
- ProcessResult referenceResult = ToolHelper.runJava(referenceJar, main.getName());
- assertEquals(referenceResult.exitCode, 0);
+ ProcessResult javaResult = ToolHelper.runJava(referenceJar, main.getName());
+ assertEquals(javaResult.exitCode, 0);
+ referenceResult = javaResult.stdout;
+ }
// Run the program on Art after is has been compiled with R8.
AndroidApp compiled =
@@ -115,7 +141,7 @@
options.testing.validInliningReasons = ImmutableSet.of(Reason.FORCE);
},
backend);
- assertEquals(referenceResult.stdout, runOnVM(compiled, main, backend));
+ assertEquals(referenceResult, runOnVM(compiled, main, backend));
// Check that B is present and that it doesn't contain the unused private method m2.
if (!enableClassInlining && !enableVerticalClassMerging) {
diff --git a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
index 3f66921..f8907d6 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -154,17 +154,10 @@
}
@Before
- public void resetAllowPartiallyImplementedOptions() {
- handler = new KeepingDiagnosticHandler();
- reporter = new Reporter(handler);
- parser = new ProguardConfigurationParser(new DexItemFactory(), reporter, false, false);
- }
-
- @Before
public void resetAllowTestOptions() {
handler = new KeepingDiagnosticHandler();
reporter = new Reporter(handler);
- parser = new ProguardConfigurationParser(new DexItemFactory(), reporter, true, true);
+ parser = new ProguardConfigurationParser(new DexItemFactory(), reporter, true);
}
@Test
@@ -901,7 +894,7 @@
@Test
public void parseKeepdirectories() throws Exception {
ProguardConfigurationParser parser =
- new ProguardConfigurationParser(new DexItemFactory(), reporter, false, false);
+ new ProguardConfigurationParser(new DexItemFactory(), reporter, false);
parser.parse(Paths.get(KEEPDIRECTORIES));
verifyParserEndsCleanly();
}
@@ -1317,7 +1310,6 @@
@Test
public void parse_adaptresourcexxx_keepdirectories_noArguments1() {
- resetAllowPartiallyImplementedOptions();
ProguardConfiguration config = parseAndVerifyParserEndsCleanly(ImmutableList.of(
"-adaptresourcefilenames",
"-adaptresourcefilecontents",
@@ -1330,7 +1322,6 @@
@Test
public void parse_adaptresourcexxx_keepdirectories_noArguments2() {
- resetAllowPartiallyImplementedOptions();
ProguardConfiguration config = parseAndVerifyParserEndsCleanly(ImmutableList.of(
"-keepdirectories",
"-adaptresourcefilenames",
@@ -1343,7 +1334,6 @@
@Test
public void parse_adaptresourcexxx_keepdirectories_noArguments3() {
- resetAllowPartiallyImplementedOptions();
ProguardConfiguration config = parseAndVerifyParserEndsCleanly(ImmutableList.of(
"-adaptresourcefilecontents",
"-keepdirectories",
@@ -1365,7 +1355,6 @@
@Test
public void parse_adaptresourcexxx_keepdirectories_singleArgument() {
- resetAllowPartiallyImplementedOptions();
ProguardConfiguration config = parseAndVerifyParserEndsCleanly(ImmutableList.of(
"-adaptresourcefilenames " + FILE_FILTER_SINGLE,
"-adaptresourcefilecontents " + FILE_FILTER_SINGLE,
@@ -1398,7 +1387,6 @@
@Test
public void parse_adaptresourcexxx_keepdirectories_multipleArgument() {
- resetAllowPartiallyImplementedOptions();
ProguardConfiguration config = parseAndVerifyParserEndsCleanly(ImmutableList.of(
"-adaptresourcefilenames " + FILE_FILTER_MULTIPLE,
"-adaptresourcefilecontents " + FILE_FILTER_MULTIPLE,
@@ -1415,7 +1403,7 @@
"-adaptresourcefilenames", "-adaptresourcefilecontents", "-keepdirectories");
for (String option : options) {
try {
- resetAllowPartiallyImplementedOptions();
+ reset();
parser.parse(createConfigurationForTesting(ImmutableList.of(option + " ,")));
fail("Expect to fail due to the lack of path filter.");
} catch (AbortException e) {
@@ -1430,7 +1418,7 @@
"-adaptresourcefilenames", "-adaptresourcefilecontents", "-keepdirectories");
for (String option : options) {
try {
- resetAllowPartiallyImplementedOptions();
+ reset();
parser.parse(createConfigurationForTesting(ImmutableList.of(option + " xxx,,yyy")));
fail("Expect to fail due to the lack of path filter.");
} catch (AbortException e) {
@@ -1445,7 +1433,7 @@
"-adaptresourcefilenames", "-adaptresourcefilecontents", "-keepdirectories");
for (String option : options) {
try {
- resetAllowPartiallyImplementedOptions();
+ reset();
parser.parse(createConfigurationForTesting(ImmutableList.of(option + " xxx,")));
fail("Expect to fail due to the lack of path filter.");
} catch (AbortException e) {
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
index af9cfdf..e6de7cc 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTest.java
@@ -52,6 +52,33 @@
}
@Test
+ public void ifOnPublic_noPublicClassForIfRule() throws Exception {
+ List<String> config = ImmutableList.of(
+ "-repackageclasses 'top'",
+ "-keep class **.Main* {",
+ " public static void callIfNonPublic();",
+ "}",
+ "-if public class **.ClassForIf {",
+ " <methods>;",
+ "}",
+ "-keep,allowobfuscation class **.ClassForSubsequent {",
+ " public <methods>;",
+ "}"
+ );
+ CodeInspector codeInspector = inspectAfterShrinking(shrinker, CLASSES, config);
+ ClassSubject classSubject = codeInspector.clazz(ClassForIf.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.method(publicMethod);
+ assertThat(methodSubject, not(isPresent()));
+ methodSubject = classSubject.method(nonPublicMethod);
+ assertThat(methodSubject, isPresent());
+ assertFalse(methodSubject.getMethod().accessFlags.isPublic());
+
+ classSubject = codeInspector.clazz(ClassForSubsequent.class);
+ assertThat(classSubject, not(isPresent()));
+ }
+
+ @Test
public void ifOnNonPublic_keepOnPublic() throws Exception {
List<String> config = ImmutableList.of(
"-printmapping",
@@ -191,12 +218,12 @@
methodSubject = classSubject.method(publicMethod);
assertThat(methodSubject, not(isPresent()));
methodSubject = classSubject.method(nonPublicMethod);
+ assertThat(methodSubject, isPresent());
if (isR8(shrinker)) {
- // TODO(b/72109068): if kept in the 1st tree shaking, should be kept after publicizing.
- assertThat(methodSubject, not(isPresent()));
+ // TODO(b/72109068): if kept in the 1st tree shaking, should not be publicized.
+ assertTrue(methodSubject.getMethod().accessFlags.isPublic());
return;
}
- assertThat(methodSubject, isPresent());
assertFalse(methodSubject.getMethod().accessFlags.isPublic());
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAnnotationTest.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAnnotationTest.java
index ce2992a..00cb62e 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAnnotationTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAnnotationTest.java
@@ -8,7 +8,6 @@
import com.google.common.collect.ImmutableList;
import java.util.Collection;
import java.util.List;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -84,7 +83,6 @@
UsedAnnotation.class, UsedAnnotationDependent.class);
}
- @Ignore("b/116092333")
@Test
public void ifOnAnnotation_onDependentClass_withNthWildcard() throws Exception {
List<String> config = ImmutableList.of(
diff --git a/src/test/java/com/android/tools/r8/shaking/keepclassmembers/b115867670/B115867670.java b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/b115867670/B115867670.java
index 30a9de9..bdd8e1a 100644
--- a/src/test/java/com/android/tools/r8/shaking/keepclassmembers/b115867670/B115867670.java
+++ b/src/test/java/com/android/tools/r8/shaking/keepclassmembers/b115867670/B115867670.java
@@ -18,7 +18,6 @@
import com.google.common.collect.ImmutableList;
import java.util.Collection;
import java.util.List;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -127,6 +126,20 @@
}
@Test
+ public void testDependentWithKeepClass() throws Exception {
+ runTest(
+ "-keep @" + pkg + ".JsonClass class ** { <fields>; }",
+ this::checkKeepClassMembers);
+ }
+
+ @Test
+ public void testDependentWithKeepClassAllowObfuscation() throws Exception {
+ runTest(
+ "-keep,allowobfuscation @" + pkg + ".JsonClass class ** { <fields>; }",
+ this::checkKeepClassMembersRenamed);
+ }
+
+ @Test
public void testDependentWithKeepClassMembers() throws Exception {
runTest(
"-keepclassmembers @" + pkg + ".JsonClass class ** { <fields>; }",
@@ -141,7 +154,6 @@
}
@Test
- @Ignore("b/116092333")
public void testDependentWithIfKeepClassMembers() throws Exception {
runTest(
"-if @" + pkg + ".JsonClass class * -keepclassmembers class <1> { <fields>; }",
@@ -149,7 +161,6 @@
}
@Test
- @Ignore("b/116092333")
public void testDependentWithIfKeepClassMembersAllowObfuscation() throws Exception {
runTest(
"-if @"
diff --git a/third_party/youtube/youtube.android_13.37.tar.gz.sha1 b/third_party/youtube/youtube.android_13.37.tar.gz.sha1
new file mode 100644
index 0000000..ffc92a8
--- /dev/null
+++ b/third_party/youtube/youtube.android_13.37.tar.gz.sha1
@@ -0,0 +1 @@
+533819c8e988bfbcdd7bc3dbd17e0d374c4ba9ab
\ No newline at end of file
diff --git a/tools/internal_test.py b/tools/internal_test.py
index 0cb9019..e040f80 100755
--- a/tools/internal_test.py
+++ b/tools/internal_test.py
@@ -4,6 +4,30 @@
# BSD-style license that can be found in the LICENSE file.
# Run all internal tests, archive result to cloud storage.
+# In the continuous operation flow we have a tester continuously checking
+# a specific cloud storage location for a file with a git hash.
+# If the file is there, the tester will remove the file, and add another
+# file stating that this is now being run. After successfully running,
+# the tester will add yet another file, and remove the last one.
+# Complete flow with states:
+# 1:
+# BOT:
+# Add file READY_FOR_TESTING (contains git hash)
+# Wait until file TESTING_COMPLETE exists (contains git hash)
+# Timeout if no progress for RUN_TIMEOUT
+# Cleanup READY_FOR_TESTING and TESTING
+# 2:
+# TESTER:
+# Replace file READY_FOR_TESTING by TESTING (contains git hash)
+# Run tests for git hash
+# Upload commit specific logs if failures
+# Upload git specific overall status file (failed or succeeded)
+# Replace file TESTING by TESTING_COMPLETE (contains git hash)
+# 3:
+# BOT:
+# Read overall status
+# Delete TESTING_COMPLETE
+# Exit based on status
import optparse
import os
@@ -12,18 +36,36 @@
import time
import utils
-# How often to pull the git repo, in seconds.
-PULL_DELAY = 25
+# How often the bot/tester should check state
+PULL_DELAY = 30
# Command timeout, in seconds.
RUN_TIMEOUT = 3600
+# Add some extra time for the bot, since the tester might not start immediately.
+BOT_RUN_TIMEOUT = 4000
BUCKET = 'r8-test-results'
TEST_RESULT_DIR = 'internal'
+# Magic files
+READY_FOR_TESTING = 'READY_FOR_TESTING'
+TESTING = 'TESTING'
+TESTING_COMPLETE = 'TESTING_COMPLETE'
+
+ALL_MAGIC = [READY_FOR_TESTING, TESTING, TESTING_COMPLETE]
+
+# Log file names
+STDERR = 'stderr'
+STDOUT = 'stdout'
+EXITCODE = 'exitcode'
+TIMED_OUT = 'timed_out'
+
def ParseOptions():
result = optparse.OptionParser()
result.add_option('--continuous',
help='Continuously run internal tests and post results to GCS.',
default=False, action='store_true')
+ result.add_option('--bot',
+ help='Run in bot mode, i.e., scheduling runs.',
+ default=False, action='store_true')
result.add_option('--archive',
help='Post result to GCS, implied by --continuous',
default=False, action='store_true')
@@ -39,22 +81,40 @@
print('Restarting tools/internal_test.py, content changed')
os.execv(sys.argv[0], sys.argv)
-def git_pull():
+def ensure_git_clean():
# Ensure clean git repo.
diff = subprocess.check_output(['git', 'diff'])
if len(diff) > 0:
print('Local modifications to the git repo, exiting')
sys.exit(1)
+
+def git_pull():
+ ensure_git_clean()
+ subprocess.check_call(['git', 'checkout', 'master'])
subprocess.check_call(['git', 'pull'])
return utils.get_HEAD_sha1()
+def git_checkout(git_hash):
+ ensure_git_clean()
+ # Ensure that we are up to date to get the commit.
+ git_pull()
+ subprocess.check_call(['git', 'checkout', git_hash])
+ return utils.get_HEAD_sha1()
+
+def get_test_result_dir():
+ return os.path.join(BUCKET, TEST_RESULT_DIR)
+
def get_sha_destination(sha):
- return os.path.join(BUCKET, TEST_RESULT_DIR, sha)
+ return os.path.join(get_test_result_dir(), sha)
def archive_status(failed):
gs_destination = 'gs://%s' % get_sha_destination(utils.get_HEAD_sha1())
archive_value('status', gs_destination, failed)
+def get_status(sha):
+ gs_destination = 'gs://%s/status' % get_sha_destination(sha)
+ return utils.cat_file_on_cloud_storage(gs_destination)
+
def archive_file(name, gs_dir, src_file):
gs_file = '%s/%s' % (gs_dir, name)
utils.upload_file_to_cloud_storage(src_file, gs_file, public_read=False)
@@ -68,30 +128,97 @@
def archive_log(stdout, stderr, exitcode, timed_out, cmd):
sha = utils.get_HEAD_sha1()
- cmd_dir = cmd.replace(' ', '_')
+ cmd_dir = cmd.replace(' ', '_').replace('/', '_')
destination = os.path.join(get_sha_destination(sha), cmd_dir)
gs_destination = 'gs://%s' % destination
url = 'https://storage.cloud.google.com/%s' % destination
print('Archiving logs to: %s' % gs_destination)
- archive_value('exitcode', gs_destination, exitcode)
- archive_value('timed_out', gs_destination, timed_out)
- archive_file('stdout', gs_destination, stdout)
- archive_file('stderr', gs_destination, stderr)
+ archive_value(EXITCODE, gs_destination, exitcode)
+ archive_value(TIMED_OUT, gs_destination, timed_out)
+ archive_file(STDOUT, gs_destination, stdout)
+ archive_file(STDERR, gs_destination, stderr)
print('Logs available at: %s' % url)
+def get_magic_file_base_path():
+ return 'gs://%s/magic' % get_test_result_dir()
+
+def get_magic_file_gs_path(name):
+ return '%s/%s' % (get_magic_file_base_path(), name)
+
+def get_magic_file_exists(name):
+ return utils.file_exists_on_cloud_storage(get_magic_file_gs_path(name))
+
+def delete_magic_file(name):
+ utils.delete_file_from_cloud_storage(get_magic_file_gs_path(name))
+
+def put_magic_file(name, sha):
+ archive_value(name, get_magic_file_base_path(), sha)
+
+def get_magic_file_content(name, ignore_errors=False):
+ return utils.cat_file_on_cloud_storage(get_magic_file_gs_path(name),
+ ignore_errors=ignore_errors)
+
+def print_magic_file_state():
+ print('Magic file status:')
+ for magic in ALL_MAGIC:
+ if get_magic_file_exists(magic):
+ content = get_magic_file_content(magic, ignore_errors=True)
+ print('%s content: %s' % (magic, content))
+
+def run_bot():
+ print_magic_file_state()
+ # Ensure that there is nothing currently scheduled (broken/stopped run)
+ for magic in ALL_MAGIC:
+ if get_magic_file_exists(magic):
+ print('ERROR: Synchronizing file %s exists, cleaning up' % magic)
+ delete_magic_file(magic)
+ print_magic_file_state()
+ assert not get_magic_file_exists(READY_FOR_TESTING)
+ git_hash = utils.get_HEAD_sha1()
+ put_magic_file(READY_FOR_TESTING, git_hash)
+ begin = time.time()
+ while True:
+ if time.time() - begin > BOT_RUN_TIMEOUT:
+ print('Timeout exceeded')
+ raise Exception('Bot timeout')
+ if get_magic_file_exists(TESTING_COMPLETE):
+ if get_magic_file_content(TESTING_COMPLETE) == git_hash:
+ break
+ else:
+ raise Exception('Non matching git hashes %s and %s' % (
+ get_magic_file_content(TESTING_COMPLETE), git_hash))
+ print('Still waiting for test result')
+ print_magic_file_state()
+ time.sleep(PULL_DELAY)
+ total_time = time.time()-begin
+ print('Done running test for %s in %ss' % (git_hash, total_time))
+ test_status = get_status(git_hash)
+ delete_magic_file(TESTING_COMPLETE)
+ print('Test status is: %s' % test_status)
+ if test_status != '0':
+ return 1
+
def run_continuously():
# If this script changes, we will restart ourselves
own_content = get_own_file_content()
- git_hash = utils.get_HEAD_sha1()
while True:
restart_if_new_version(own_content)
- print('Running with hash: %s' % git_hash)
- exitcode = run_once(archive=True)
- git_pull()
- while git_pull() == git_hash:
- print('Still on same git hash: %s' % git_hash)
- time.sleep(PULL_DELAY)
- git_hash = utils.get_HEAD_sha1()
+ print_magic_file_state()
+ if get_magic_file_exists(READY_FOR_TESTING):
+ git_hash = get_magic_file_content(READY_FOR_TESTING)
+ checked_out = git_checkout(git_hash)
+ # Sanity check, if this does not succeed stop.
+ if checked_out != git_hash:
+ print('Inconsistent state: %s %s' % (git_hash, checked_out))
+ sys.exit(1)
+ put_magic_file(TESTING, git_hash)
+ delete_magic_file(READY_FOR_TESTING)
+ print('Running with hash: %s' % git_hash)
+ exitcode = run_once(archive=True)
+ print('Running finished with exit code %s' % exitcode)
+ put_magic_file(TESTING_COMPLETE, git_hash)
+ delete_magic_file(TESTING)
+ time.sleep(PULL_DELAY)
def handle_output(archive, stderr, stdout, exitcode, timed_out, cmd):
if archive:
@@ -147,17 +274,21 @@
if execute(cmd, archive):
failed = True
# Ensure that all internal apps compile.
- cmd = ['tools/run_on_app.py', '--run-all', '--out=out']
+ cmd = ['tools/run_on_app.py', '--ignore-java-version','--run-all',
+ '--out=out']
if execute(cmd, archive):
failed = True
archive_status(1 if failed else 0)
+ return failed
def Main():
(options, args) = ParseOptions()
if options.continuous:
run_continuously()
+ elif options.bot:
+ return run_bot()
else:
- run_once(options.archive)
+ return run_once(options.archive)
if __name__ == '__main__':
sys.exit(Main())
diff --git a/tools/run_on_app.py b/tools/run_on_app.py
index 0667abe..bab9b32 100755
--- a/tools/run_on_app.py
+++ b/tools/run_on_app.py
@@ -49,6 +49,10 @@
help='Running on golem, do not build or download',
default=False,
action='store_true')
+ result.add_option('--ignore-java-version',
+ help='Do not check java version',
+ default=False,
+ action='store_true')
result.add_option('--no-libraries',
help='Do not pass in libraries, even if they exist in conf',
default=False,
@@ -115,7 +119,8 @@
# do Bug: #BUG in the commit message of disabling to ensure re-enabling
DISABLED_PERMUTATIONS = [
('youtube', '12.10', 'dex'), # b/116089492
- ('youtube', '12.22', 'deploy') # b/116093710
+ ('youtube', '12.10', 'proguarded'), # b/116089492
+ ('gmscore', 'latest', 'deploy') # b/116575775
]
def get_permutations():
@@ -150,8 +155,10 @@
exit(exit_code)
def main(argv):
- utils.check_java_version()
(options, args) = ParseOptions(argv)
+ if not options.ignore_java_version:
+ utils.check_java_version()
+
if options.run_all:
return run_all(options, args)
return run_with_options(options, args)
diff --git a/tools/utils.py b/tools/utils.py
index d96151d..20ba093 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -106,6 +106,27 @@
PrintCmd(cmd)
subprocess.check_call(cmd)
+def delete_file_from_cloud_storage(destination):
+ cmd = ['gsutil.py', 'rm', destination]
+ PrintCmd(cmd)
+ subprocess.check_call(cmd)
+
+def cat_file_on_cloud_storage(destination, ignore_errors=False):
+ cmd = ['gsutil.py', 'cat', destination]
+ PrintCmd(cmd)
+ try:
+ return subprocess.check_output(cmd)
+ except subprocess.CalledProcessError as e:
+ if ignore_errors:
+ return ''
+ else:
+ raise e
+
+def file_exists_on_cloud_storage(destination):
+ cmd = ['gsutil.py', 'ls', destination]
+ PrintCmd(cmd)
+ return subprocess.call(cmd) == 0
+
def download_file_from_cloud_storage(source, destination):
cmd = ['gsutil.py', 'cp', source, destination]
PrintCmd(cmd)
diff --git a/tools/youtube_data.py b/tools/youtube_data.py
index b7f1ee7..6fdf63a 100644
--- a/tools/youtube_data.py
+++ b/tools/youtube_data.py
@@ -19,6 +19,9 @@
V12_22_BASE = os.path.join(BASE, 'youtube.android_12.22')
V12_22_PREFIX = os.path.join(V12_22_BASE, 'YouTubeRelease')
+V13_37_BASE = os.path.join(BASE, 'youtube.android_13.37')
+V13_37_PREFIX = os.path.join(V13_37_BASE, 'YouTubeRelease')
+
# NOTE: we always use android.jar for SDK v25, later we might want to revise it
# to use proper android.jar version for each of youtube version separately.
ANDROID_JAR = os.path.join(THIRD_PARTY, 'android_jar', 'lib-v25', 'android.jar')
@@ -85,4 +88,30 @@
'min-api' : ANDROID_L_API,
}
},
+ '13.37': {
+ 'dex' : {
+ 'inputs': [os.path.join(V13_37_BASE, 'YouTubeRelease_unsigned.apk')],
+ 'pgmap': '%s_proguard.map' % V13_37_PREFIX,
+ 'libraries' : [ANDROID_JAR],
+ 'min-api' : ANDROID_L_API,
+ },
+ 'deploy' : {
+ 'inputs': ['%s_deploy.jar' % V13_37_PREFIX],
+ 'pgconf': [
+ '%s_proguard.config' % V13_37_PREFIX,
+ '%s/proguardsettings/YouTubeRelease_proguard.config' % THIRD_PARTY],
+ # Build for native multi dex, as Currently R8 cannot meet the main-dex
+ # constraints.
+ #'maindexrules' : [
+ # os.path.join(V13_37_BASE, 'mainDexClasses.rules'),
+ # os.path.join(V13_37_BASE, 'main-dex-classes-release-optimized.cfg'),
+ # os.path.join(V13_37_BASE, 'main_dex_YouTubeRelease_proguard.cfg')],
+ 'min-api' : ANDROID_L_API,
+ },
+ 'proguarded' : {
+ 'inputs': ['%s_proguard.jar' % V13_37_PREFIX],
+ 'pgmap': '%s_proguard.map' % V13_37_PREFIX,
+ 'min-api' : ANDROID_L_API,
+ }
+ },
}
