Ignore -assumenosideeffects rules for Object members
This CL additionally refines the side effect models for Object.equals(), Object.hashCode(), and Object.toString(), since it is no longer possible to specify using -assumenosideeffects that these methods do not have side effects.
Fixes: 171380978
Change-Id: Ieebd750800e50449f0508bc617e1d937da92ff7d
diff --git a/src/main/java/com/android/tools/r8/errors/AssumeNoSideEffectsRuleForObjectMembersDiagnostic.java b/src/main/java/com/android/tools/r8/errors/AssumeNoSideEffectsRuleForObjectMembersDiagnostic.java
new file mode 100644
index 0000000..8910cda
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/errors/AssumeNoSideEffectsRuleForObjectMembersDiagnostic.java
@@ -0,0 +1,92 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.errors;
+
+import com.android.tools.r8.Diagnostic;
+import com.android.tools.r8.Keep;
+import com.android.tools.r8.errors.CheckDiscardDiagnostic.Builder;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.position.Position;
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.utils.MethodReferenceUtils;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+
+@Keep
+public class AssumeNoSideEffectsRuleForObjectMembersDiagnostic implements Diagnostic {
+
+ private final List<MethodReference> methods;
+ private final Origin origin;
+ private final Position position;
+
+ private AssumeNoSideEffectsRuleForObjectMembersDiagnostic(
+ List<MethodReference> methods, Origin origin, Position position) {
+ this.methods = methods;
+ this.origin = origin;
+ this.position = position;
+ }
+
+ @Override
+ public Origin getOrigin() {
+ return origin;
+ }
+
+ @Override
+ public Position getPosition() {
+ return position;
+ }
+
+ @Override
+ public String getDiagnosticMessage() {
+ Iterator<MethodReference> iterator = methods.iterator();
+ StringBuilder message =
+ new StringBuilder("The -assumenosideeffects rule matches the following method(s) ")
+ .append("on java.lang.Object: ")
+ .append(MethodReferenceUtils.toSourceStringWithoutHolderAndReturnType(iterator.next()));
+ while (iterator.hasNext()) {
+ MethodReference method = iterator.next();
+ message
+ .append(iterator.hasNext() ? ", " : " and ")
+ .append(MethodReferenceUtils.toSourceStringWithoutHolderAndReturnType(method));
+ }
+ return message
+ .append(". ")
+ .append("This is most likely not intended. ")
+ .append("Consider specifying the methods more precisely.")
+ .toString();
+ }
+
+ public static class Builder {
+
+ private final List<MethodReference> methods = new ArrayList<>();
+ private Origin origin;
+ private Position position;
+
+ public Builder() {}
+
+ public Builder addMatchedMethods(Set<DexMethod> methods) {
+ for (DexMethod method : methods) {
+ this.methods.add(method.asMethodReference());
+ }
+ return this;
+ }
+
+ public Builder setOrigin(Origin origin) {
+ this.origin = origin;
+ return this;
+ }
+
+ public Builder setPosition(Position position) {
+ this.position = position;
+ return this;
+ }
+
+ public AssumeNoSideEffectsRuleForObjectMembersDiagnostic build() {
+ return new AssumeNoSideEffectsRuleForObjectMembersDiagnostic(methods, origin, position);
+ }
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/graph/AppView.java b/src/main/java/com/android/tools/r8/graph/AppView.java
index 877f2ba..37f45c6 100644
--- a/src/main/java/com/android/tools/r8/graph/AppView.java
+++ b/src/main/java/com/android/tools/r8/graph/AppView.java
@@ -23,6 +23,7 @@
import com.android.tools.r8.ir.optimize.CallSiteOptimizationInfoPropagator;
import com.android.tools.r8.ir.optimize.info.field.InstanceFieldInitializationInfoFactory;
import com.android.tools.r8.ir.optimize.library.LibraryMemberOptimizer;
+import com.android.tools.r8.ir.optimize.library.LibraryMethodSideEffectModelCollection;
import com.android.tools.r8.optimize.MemberRebindingLens;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.KeepInfoCollection;
@@ -69,6 +70,9 @@
// Desugared library prefix rewriter.
public final PrefixRewritingMapper rewritePrefix;
+ // Modeling.
+ private final LibraryMethodSideEffectModelCollection libraryMethodSideEffectModelCollection;
+
// Optimizations.
private final CallSiteOptimizationInfoPropagator callSiteOptimizationInfoPropagator;
private final LibraryMemberOptimizer libraryMemberOptimizer;
@@ -108,6 +112,8 @@
}
this.libraryMemberOptimizer = new LibraryMemberOptimizer(this);
+ this.libraryMethodSideEffectModelCollection =
+ new LibraryMethodSideEffectModelCollection(dexItemFactory());
if (enableWholeProgramOptimizations() && options().protoShrinking().isProtoShrinkingEnabled()) {
this.protoShrinker = new ProtoShrinker(withLiveness());
@@ -287,6 +293,10 @@
return libraryMemberOptimizer;
}
+ public LibraryMethodSideEffectModelCollection getLibraryMethodSideEffectModelCollection() {
+ return libraryMethodSideEffectModelCollection;
+ }
+
public ProtoShrinker protoShrinker() {
return protoShrinker;
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java b/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java
index ecb1b4f..0a17134 100644
--- a/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexClassAndMethod.java
@@ -20,7 +20,7 @@
if (holder.isProgramClass()) {
return new ProgramMethod(holder.asProgramClass(), method);
} else if (holder.isLibraryClass()) {
- return new DexClassAndMethod(holder, method);
+ return new LibraryMethod(holder.asLibraryClass(), method);
} else {
assert holder.isClasspathClass();
return new ClasspathMethod(holder.asClasspathClass(), method);
@@ -50,6 +50,14 @@
return null;
}
+ public boolean isLibraryMethod() {
+ return false;
+ }
+
+ public LibraryMethod asLibraryMethod() {
+ return null;
+ }
+
public boolean isProgramMethod() {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index cf30eed..b192017 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -5,7 +5,6 @@
import static com.android.tools.r8.ir.analysis.type.ClassTypeElement.computeLeastUpperBoundOfInterfaces;
import static com.android.tools.r8.ir.optimize.ServiceLoaderRewriter.SERVICE_LOADER_CLASS_NAME;
-import static com.google.common.base.Predicates.alwaysTrue;
import com.android.tools.r8.dex.Constants;
import com.android.tools.r8.dex.Marker;
@@ -31,7 +30,6 @@
import com.android.tools.r8.kotlin.Kotlin;
import com.android.tools.r8.utils.ArrayUtils;
import com.android.tools.r8.utils.LRUCacheTable;
-import com.android.tools.r8.utils.Pair;
import com.google.common.base.Strings;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
@@ -39,7 +37,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
-import com.google.common.collect.Streams;
import it.unimi.dsi.fastutil.ints.Int2ReferenceArrayMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
import it.unimi.dsi.fastutil.ints.Int2ReferenceOpenHashMap;
@@ -57,7 +54,6 @@
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;
-import java.util.stream.Stream;
public class DexItemFactory {
@@ -109,8 +105,6 @@
public DexItemFactory() {
this.kotlin = new Kotlin(this);
- assert libraryMethodsWithReturnValueDependingOnlyOnArguments.stream()
- .allMatch(libraryMethodsWithoutSideEffects::containsKey);
}
public static boolean isInternalSentinel(DexItem item) {
@@ -604,7 +598,7 @@
}
// Boxed Boxed#valueOf(Primitive), e.g., Boolean Boolean#valueOf(B)
- private Set<DexMethod> boxedValueOfMethods() {
+ public Set<DexMethod> boxedValueOfMethods() {
return primitiveToBoxed.entrySet().stream()
.map(
entry -> {
@@ -686,31 +680,6 @@
objectsMethods.requireNonNullWithMessageSupplier,
stringMembers.valueOf);
- // We assume library methods listed here are `public`, i.e., free from visibility side effects.
- // If not, that library method should not be added here because it literally has side effects.
- public Map<DexMethod, Predicate<InvokeMethod>> libraryMethodsWithoutSideEffects =
- Streams.<Pair<DexMethod, Predicate<InvokeMethod>>>concat(
- Stream.of(new Pair<>(enumMembers.constructor, alwaysTrue())),
- Stream.of(new Pair<>(npeMethods.init, alwaysTrue())),
- Stream.of(new Pair<>(npeMethods.initWithMessage, alwaysTrue())),
- Stream.of(new Pair<>(objectMembers.constructor, alwaysTrue())),
- Stream.of(new Pair<>(objectMembers.getClass, alwaysTrue())),
- Stream.of(new Pair<>(stringMembers.hashCode, alwaysTrue())),
- mapToPredicate(classMethods.getNames, alwaysTrue()),
- mapToPredicate(
- stringBufferMethods.constructorMethods,
- stringBufferMethods::constructorInvokeIsSideEffectFree),
- mapToPredicate(
- stringBuilderMethods.constructorMethods,
- stringBuilderMethods::constructorInvokeIsSideEffectFree),
- mapToPredicate(boxedValueOfMethods(), alwaysTrue()))
- .collect(Collectors.toMap(Pair::getFirst, Pair::getSecond));
-
- private static Stream<Pair<DexMethod, Predicate<InvokeMethod>>> mapToPredicate(
- Set<DexMethod> methods, Predicate<InvokeMethod> predicate) {
- return methods.stream().map(method -> new Pair<>(method, predicate));
- }
-
// TODO(b/119596718): More idempotent methods? Any singleton accessors? E.g.,
// java.util.Calendar#getInstance(...) // 4 variants
// java.util.Locale#getDefault() // returns JVM default locale.
@@ -1130,7 +1099,10 @@
public final DexField clinitField = createField(objectType, intType, "$r8$clinit");
public final DexMethod clone;
+ public final DexMethod equals =
+ createMethod(objectType, createProto(booleanType, objectType), "equals");
public final DexMethod getClass;
+ public final DexMethod hashCode = createMethod(objectType, createProto(intType), "hashCode");
public final DexMethod constructor;
public final DexMethod finalize;
public final DexMethod toString;
@@ -1199,7 +1171,7 @@
public final DexMethod getDeclaredMethod;
public final DexMethod newInstance;
private final Set<DexMethod> getMembers;
- private final Set<DexMethod> getNames;
+ public final Set<DexMethod> getNames;
private ClassMethods() {
desiredAssertionStatus = createMethod(classDescriptor,
@@ -1561,8 +1533,6 @@
public final DexMethod toString;
private final Set<DexMethod> appendMethods;
- private final Set<DexMethod> constructorMethods;
-
private StringBuildingMethods(DexType receiver) {
DexString append = createString("append");
@@ -1611,6 +1581,8 @@
charSequenceConstructor, defaultConstructor, intConstructor, stringConstructor);
}
+ public final Set<DexMethod> constructorMethods;
+
public boolean isAppendMethod(DexMethod method) {
return appendMethods.contains(method);
}
diff --git a/src/main/java/com/android/tools/r8/graph/DexMethod.java b/src/main/java/com/android/tools/r8/graph/DexMethod.java
index 6f77653..74b0fff 100644
--- a/src/main/java/com/android/tools/r8/graph/DexMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexMethod.java
@@ -223,15 +223,22 @@
@Override
public String toSourceString() {
- return toSourceString(true);
+ return toSourceString(true, true);
}
public String toSourceStringWithoutHolder() {
- return toSourceString(false);
+ return toSourceString(false, true);
}
- private String toSourceString(boolean includeHolder) {
- StringBuilder builder = new StringBuilder(proto.returnType.toSourceString()).append(" ");
+ public String toSourceStringWithoutHolderAndReturnType() {
+ return toSourceString(false, false);
+ }
+
+ private String toSourceString(boolean includeHolder, boolean includeReturnType) {
+ StringBuilder builder = new StringBuilder();
+ if (includeReturnType) {
+ builder.append(getReturnType().toSourceString()).append(" ");
+ }
if (includeHolder) {
builder.append(holder.toSourceString()).append(".");
}
diff --git a/src/main/java/com/android/tools/r8/graph/GraphLens.java b/src/main/java/com/android/tools/r8/graph/GraphLens.java
index d00b9d9..a69be5e 100644
--- a/src/main/java/com/android/tools/r8/graph/GraphLens.java
+++ b/src/main/java/com/android/tools/r8/graph/GraphLens.java
@@ -502,15 +502,15 @@
return result;
}
- public DexReference rewriteReference(DexReference reference) {
+ public <T extends DexReference> T rewriteReference(T reference) {
if (reference.isDexField()) {
- return getRenamedFieldSignature(reference.asDexField());
+ return (T) getRenamedFieldSignature(reference.asDexField());
}
if (reference.isDexMethod()) {
- return getRenamedMethodSignature(reference.asDexMethod());
+ return (T) getRenamedMethodSignature(reference.asDexMethod());
}
assert reference.isDexType();
- return lookupType(reference.asDexType());
+ return (T) lookupType(reference.asDexType());
}
public Set<DexReference> rewriteReferences(Set<DexReference> references) {
@@ -521,8 +521,8 @@
return result;
}
- public <T> ImmutableMap<DexReference, T> rewriteReferenceKeys(Map<DexReference, T> map) {
- ImmutableMap.Builder<DexReference, T> builder = ImmutableMap.builder();
+ public <R extends DexReference, T> ImmutableMap<R, T> rewriteReferenceKeys(Map<R, T> map) {
+ ImmutableMap.Builder<R, T> builder = ImmutableMap.builder();
map.forEach((reference, value) -> builder.put(rewriteReference(reference), value));
return builder.build();
}
diff --git a/src/main/java/com/android/tools/r8/graph/LibraryMethod.java b/src/main/java/com/android/tools/r8/graph/LibraryMethod.java
new file mode 100644
index 0000000..c06ebab
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/LibraryMethod.java
@@ -0,0 +1,29 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.graph;
+
+/** Type representing a method definition from the library and its holder. */
+public final class LibraryMethod extends DexClassAndMethod {
+
+ public LibraryMethod(DexLibraryClass holder, DexEncodedMethod method) {
+ super(holder, method);
+ }
+
+ @Override
+ public boolean isLibraryMethod() {
+ return true;
+ }
+
+ @Override
+ public LibraryMethod asLibraryMethod() {
+ return this;
+ }
+
+ @Override
+ public DexLibraryClass getHolder() {
+ DexClass holder = super.getHolder();
+ assert holder.isLibraryClass();
+ return holder.asLibraryClass();
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/ir/analysis/modeling/LibraryMethodReadSetModeling.java b/src/main/java/com/android/tools/r8/ir/analysis/modeling/LibraryMethodReadSetModeling.java
index 34ab168..f9a2e25 100644
--- a/src/main/java/com/android/tools/r8/ir/analysis/modeling/LibraryMethodReadSetModeling.java
+++ b/src/main/java/com/android/tools/r8/ir/analysis/modeling/LibraryMethodReadSetModeling.java
@@ -4,41 +4,39 @@
package com.android.tools.r8.ir.analysis.modeling;
-import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.analysis.fieldvalueanalysis.AbstractFieldSet;
import com.android.tools.r8.ir.analysis.fieldvalueanalysis.EmptyFieldSet;
import com.android.tools.r8.ir.analysis.fieldvalueanalysis.UnknownFieldSet;
import com.android.tools.r8.ir.code.InvokeMethod;
-import java.util.function.Predicate;
/** Models if a given library method may cause a program field to be read. */
public class LibraryMethodReadSetModeling {
public static AbstractFieldSet getModeledReadSetOrUnknown(
- InvokeMethod invoke, DexItemFactory dexItemFactory) {
+ AppView<?> appView, InvokeMethod invoke) {
DexMethod invokedMethod = invoke.getInvokedMethod();
// Check if it is a library method that does not have side effects. In that case it is safe to
// assume that the method does not read any fields, since even if it did, it would not be able
// to do anything with the values it read (since we will remove such invocations without side
// effects).
- Predicate<InvokeMethod> noSideEffectsPredicate =
- dexItemFactory.libraryMethodsWithoutSideEffects.get(invokedMethod);
- if (noSideEffectsPredicate != null && noSideEffectsPredicate.test(invoke)) {
+ if (appView
+ .getLibraryMethodSideEffectModelCollection()
+ .isCallToSideEffectFreeFinalMethod(invoke)) {
return EmptyFieldSet.getInstance();
}
// Already handled above.
- assert !dexItemFactory.classMethods.isReflectiveNameLookup(invokedMethod);
+ assert !appView.dexItemFactory().classMethods.isReflectiveNameLookup(invokedMethod);
// Modeling of other library methods.
DexType holder = invokedMethod.holder;
- if (holder == dexItemFactory.objectType) {
- if (invokedMethod == dexItemFactory.objectMembers.constructor) {
- return EmptyFieldSet.getInstance();
- }
+ if (holder == appView.dexItemFactory().objectType
+ && invokedMethod == appView.dexItemFactory().objectMembers.constructor) {
+ return EmptyFieldSet.getInstance();
}
return UnknownFieldSet.getInstance();
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java b/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
index a6d77f7..c61ca0a 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeDirect.java
@@ -207,6 +207,6 @@
}
}
- return LibraryMethodReadSetModeling.getModeledReadSetOrUnknown(this, appView.dexItemFactory());
+ return LibraryMethodReadSetModeling.getModeledReadSetOrUnknown(appView, this);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
index a78a16b..6daa9a4 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
@@ -202,7 +202,7 @@
@Override
public AbstractFieldSet readSet(AppView<AppInfoWithLiveness> appView, ProgramMethod context) {
- return LibraryMethodReadSetModeling.getModeledReadSetOrUnknown(this, appView.dexItemFactory());
+ return LibraryMethodReadSetModeling.getModeledReadSetOrUnknown(appView, this);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
index 09db288..2aeb9b5 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethodWithReceiver.java
@@ -25,7 +25,6 @@
import com.android.tools.r8.ir.optimize.inliner.WhyAreYouNotInliningReporter;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import java.util.List;
-import java.util.function.Predicate;
public abstract class InvokeMethodWithReceiver extends InvokeMethod {
@@ -195,9 +194,9 @@
}
// Check if it is a call to one of library methods that are known to be side-effect free.
- Predicate<InvokeMethod> noSideEffectsPredicate =
- appView.dexItemFactory().libraryMethodsWithoutSideEffects.get(getInvokedMethod());
- if (noSideEffectsPredicate != null && noSideEffectsPredicate.test(this)) {
+ if (appView
+ .getLibraryMethodSideEffectModelCollection()
+ .isCallToSideEffectFreeFinalMethod(this)) {
return false;
}
@@ -240,6 +239,13 @@
return true;
}
+ if (singleTarget.isLibraryMethod()
+ && appView
+ .getLibraryMethodSideEffectModelCollection()
+ .isSideEffectFree(this, singleTarget.asLibraryMethod())) {
+ return false;
+ }
+
// Verify that the target method does not have side-effects.
if (appViewWithLiveness.appInfo().noSideEffects.containsKey(singleTarget.getReference())) {
return false;
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
index 4e8746d..bf3a599 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeStatic.java
@@ -29,7 +29,6 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.google.common.collect.Sets;
import java.util.List;
-import java.util.function.Predicate;
public class InvokeStatic extends InvokeMethod {
@@ -179,9 +178,9 @@
}
// Check if it is a call to one of library methods that are known to be side-effect free.
- Predicate<InvokeMethod> noSideEffectsPredicate =
- appView.dexItemFactory().libraryMethodsWithoutSideEffects.get(getInvokedMethod());
- if (noSideEffectsPredicate != null && noSideEffectsPredicate.test(this)) {
+ if (appView
+ .getLibraryMethodSideEffectModelCollection()
+ .isCallToSideEffectFreeFinalMethod(this)) {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java b/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
index 72abb27..50443c6 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/IdempotentFunctionCallCanonicalizer.java
@@ -30,7 +30,6 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
-import java.util.function.Predicate;
/**
* Canonicalize idempotent function calls.
@@ -277,12 +276,10 @@
private boolean isIdempotentLibraryMethodInvoke(InvokeMethod invoke) {
DexMethod invokedMethod = invoke.getInvokedMethod();
- Predicate<InvokeMethod> noSideEffectPredicate =
- factory.libraryMethodsWithoutSideEffects.get(invokedMethod);
- if (noSideEffectPredicate == null || !noSideEffectPredicate.test(invoke)) {
- return false;
- }
- return factory.libraryMethodsWithReturnValueDependingOnlyOnArguments.contains(invokedMethod);
+ return appView
+ .getLibraryMethodSideEffectModelCollection()
+ .isCallToSideEffectFreeFinalMethod(invoke)
+ && factory.libraryMethodsWithReturnValueDependingOnlyOnArguments.contains(invokedMethod);
}
private static void insertCanonicalizedInvokeWithInValues(
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
index d0fc9c5..6b0b61d 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/InlineCandidateProcessor.java
@@ -6,7 +6,6 @@
import static com.android.tools.r8.graph.DexEncodedMethod.asProgramMethodOrNull;
import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
-import static com.google.common.base.Predicates.alwaysFalse;
import com.android.tools.r8.errors.InternalCompilerError;
import com.android.tools.r8.errors.Unreachable;
@@ -19,6 +18,7 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.LibraryMethod;
import com.android.tools.r8.graph.ProgramMethod;
import com.android.tools.r8.graph.ResolutionResult;
import com.android.tools.r8.graph.ResolutionResult.SingleResolutionResult;
@@ -302,7 +302,8 @@
return user; // Not eligible.
}
- if (isEligibleLibraryMethodCall(invokeMethod, singleTarget)) {
+ if (singleTarget.isLibraryMethod()
+ && isEligibleLibraryMethodCall(invokeMethod, singleTarget.asLibraryMethod())) {
continue;
}
@@ -648,11 +649,12 @@
}
DexClassAndMethod singleTarget = invoke.lookupSingleTarget(appView, method);
- if (singleTarget != null) {
- Predicate<InvokeMethod> noSideEffectsPredicate =
- dexItemFactory.libraryMethodsWithoutSideEffects.getOrDefault(
- singleTarget.getReference(), alwaysFalse());
- if (noSideEffectsPredicate.test(invoke)) {
+ if (singleTarget != null && singleTarget.isLibraryMethod()) {
+ boolean isSideEffectFree =
+ appView
+ .getLibraryMethodSideEffectModelCollection()
+ .isSideEffectFree(invoke, singleTarget.asLibraryMethod());
+ if (isSideEffectFree) {
if (!invoke.hasOutValue() || !invoke.outValue().hasAnyUsers()) {
removeInstruction(invoke);
continue;
@@ -1170,10 +1172,10 @@
return true;
}
- private boolean isEligibleLibraryMethodCall(InvokeMethod invoke, DexClassAndMethod singleTarget) {
- Predicate<InvokeMethod> noSideEffectsPredicate =
- dexItemFactory.libraryMethodsWithoutSideEffects.get(singleTarget.getReference());
- if (noSideEffectsPredicate != null && noSideEffectsPredicate.test(invoke)) {
+ private boolean isEligibleLibraryMethodCall(InvokeMethod invoke, LibraryMethod singleTarget) {
+ boolean isSideEffectFree =
+ appView.getLibraryMethodSideEffectModelCollection().isSideEffectFree(invoke, singleTarget);
+ if (isSideEffectFree) {
return !invoke.hasOutValue() || !invoke.outValue().hasAnyUsers();
}
if (singleTarget.getReference() == dexItemFactory.objectsMethods.requireNonNull) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/library/LibraryMethodSideEffectModelCollection.java b/src/main/java/com/android/tools/r8/ir/optimize/library/LibraryMethodSideEffectModelCollection.java
new file mode 100644
index 0000000..55d1dc3
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/ir/optimize/library/LibraryMethodSideEffectModelCollection.java
@@ -0,0 +1,82 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.ir.optimize.library;
+
+import static com.google.common.base.Predicates.alwaysFalse;
+import static com.google.common.base.Predicates.alwaysTrue;
+
+import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.LibraryMethod;
+import com.android.tools.r8.ir.code.InvokeMethod;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Predicate;
+
+public class LibraryMethodSideEffectModelCollection {
+
+ private final Map<DexMethod, Predicate<InvokeMethod>> finalMethodsWithoutSideEffects;
+ private final Set<DexMethod> nonFinalMethodsWithoutSideEffects;
+
+ public LibraryMethodSideEffectModelCollection(DexItemFactory dexItemFactory) {
+ finalMethodsWithoutSideEffects = buildFinalMethodsWithoutSideEffects(dexItemFactory);
+ nonFinalMethodsWithoutSideEffects = buildNonFinalMethodsWithoutSideEffects(dexItemFactory);
+ }
+
+ private static Map<DexMethod, Predicate<InvokeMethod>> buildFinalMethodsWithoutSideEffects(
+ DexItemFactory dexItemFactory) {
+ ImmutableMap.Builder<DexMethod, Predicate<InvokeMethod>> builder =
+ ImmutableMap.<DexMethod, Predicate<InvokeMethod>>builder()
+ .put(dexItemFactory.enumMembers.constructor, alwaysTrue())
+ .put(dexItemFactory.npeMethods.init, alwaysTrue())
+ .put(dexItemFactory.npeMethods.initWithMessage, alwaysTrue())
+ .put(dexItemFactory.objectMembers.constructor, alwaysTrue())
+ .put(dexItemFactory.objectMembers.getClass, alwaysTrue())
+ .put(dexItemFactory.stringMembers.hashCode, alwaysTrue());
+ putAll(builder, dexItemFactory.classMethods.getNames, alwaysTrue());
+ putAll(
+ builder,
+ dexItemFactory.stringBufferMethods.constructorMethods,
+ dexItemFactory.stringBufferMethods::constructorInvokeIsSideEffectFree);
+ putAll(
+ builder,
+ dexItemFactory.stringBuilderMethods.constructorMethods,
+ dexItemFactory.stringBuilderMethods::constructorInvokeIsSideEffectFree);
+ putAll(builder, dexItemFactory.boxedValueOfMethods(), alwaysTrue());
+ return builder.build();
+ }
+
+ private static Set<DexMethod> buildNonFinalMethodsWithoutSideEffects(
+ DexItemFactory dexItemFactory) {
+ return ImmutableSet.of(
+ dexItemFactory.objectMembers.equals,
+ dexItemFactory.objectMembers.hashCode,
+ dexItemFactory.objectMembers.toString);
+ }
+
+ private static void putAll(
+ ImmutableMap.Builder<DexMethod, Predicate<InvokeMethod>> builder,
+ Iterable<DexMethod> methods,
+ Predicate<InvokeMethod> predicate) {
+ for (DexMethod method : methods) {
+ builder.put(method, predicate);
+ }
+ }
+
+ public boolean isCallToSideEffectFreeFinalMethod(InvokeMethod invoke) {
+ return finalMethodsWithoutSideEffects
+ .getOrDefault(invoke.getInvokedMethod(), alwaysFalse())
+ .test(invoke);
+ }
+
+ // This intentionally takes the invoke instruction since the determination of whether a library
+ // method has side effects may depend on the arguments.
+ public boolean isSideEffectFree(InvokeMethod invoke, LibraryMethod singleTarget) {
+ return isCallToSideEffectFreeFinalMethod(invoke)
+ || nonFinalMethodsWithoutSideEffects.contains(singleTarget.getReference());
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index 7c6d911..b54a0db 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexField;
+import com.android.tools.r8.graph.DexMember;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexReference;
@@ -136,9 +137,9 @@
/** All items with assumemayhavesideeffects rule. */
public final Map<DexReference, ProguardMemberRule> mayHaveSideEffects;
/** All items with assumenosideeffects rule. */
- public final Map<DexReference, ProguardMemberRule> noSideEffects;
+ public final Map<DexMember<?, ?>, ProguardMemberRule> noSideEffects;
/** All items with assumevalues rule. */
- public final Map<DexReference, ProguardMemberRule> assumedValues;
+ public final Map<DexMember<?, ?>, ProguardMemberRule> assumedValues;
/** All methods that should be inlined if possible due to a configuration directive. */
public final Set<DexMethod> alwaysInline;
/** All methods that *must* be inlined due to a configuration directive (testing only). */
@@ -213,8 +214,8 @@
Map<DexCallSite, ProgramMethodSet> callSites,
KeepInfoCollection keepInfo,
Map<DexReference, ProguardMemberRule> mayHaveSideEffects,
- Map<DexReference, ProguardMemberRule> noSideEffects,
- Map<DexReference, ProguardMemberRule> assumedValues,
+ Map<DexMember<?, ?>, ProguardMemberRule> noSideEffects,
+ Map<DexMember<?, ?>, ProguardMemberRule> assumedValues,
Set<DexMethod> alwaysInline,
Set<DexMethod> forceInline,
Set<DexMethod> neverInline,
@@ -294,8 +295,8 @@
Map<DexCallSite, ProgramMethodSet> callSites,
KeepInfoCollection keepInfo,
Map<DexReference, ProguardMemberRule> mayHaveSideEffects,
- Map<DexReference, ProguardMemberRule> noSideEffects,
- Map<DexReference, ProguardMemberRule> assumedValues,
+ Map<DexMember<?, ?>, ProguardMemberRule> noSideEffects,
+ Map<DexMember<?, ?>, ProguardMemberRule> assumedValues,
Set<DexMethod> alwaysInline,
Set<DexMethod> forceInline,
Set<DexMethod> neverInline,
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 5b3038f..a5ee8b7 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -5,8 +5,8 @@
import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
-import com.android.tools.r8.Diagnostic;
import com.android.tools.r8.dex.Constants;
+import com.android.tools.r8.errors.AssumeNoSideEffectsRuleForObjectMembersDiagnostic;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
import com.android.tools.r8.graph.AppView;
@@ -67,6 +67,7 @@
import java.util.Queue;
import java.util.Set;
import java.util.Stack;
+import java.util.TreeSet;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
@@ -109,8 +110,8 @@
private final Map<DexType, Set<ProguardKeepRuleBase>> dependentKeepClassCompatRule =
new IdentityHashMap<>();
private final Map<DexReference, ProguardMemberRule> mayHaveSideEffects = new IdentityHashMap<>();
- private final Map<DexReference, ProguardMemberRule> noSideEffects = new IdentityHashMap<>();
- private final Map<DexReference, ProguardMemberRule> assumedValues = new IdentityHashMap<>();
+ private final Map<DexMember<?, ?>, ProguardMemberRule> noSideEffects = new IdentityHashMap<>();
+ private final Map<DexMember<?, ?>, ProguardMemberRule> assumedValues = new IdentityHashMap<>();
private final Set<DexReference> identifierNameStrings = Sets.newIdentityHashSet();
private final Queue<DelayedRootSetActionItem> delayedRootSetActionItems =
new ConcurrentLinkedQueue<>();
@@ -119,8 +120,8 @@
private final DexStringCache dexStringCache = new DexStringCache();
private final Set<ProguardIfRule> ifRules = Sets.newIdentityHashSet();
- private final Map<OriginWithPosition, List<DexMethod>> assumeNoSideEffectsWarnings =
- new HashMap<>();
+ private final Map<OriginWithPosition, Set<DexMethod>> assumeNoSideEffectsWarnings =
+ new LinkedHashMap<>();
public RootSetBuilder(
AppView<? extends AppInfoWithClassHierarchy> appView,
@@ -387,7 +388,7 @@
DexType type,
DexMethod reference,
Set<DexType> subTypes,
- Map<DexReference, ProguardMemberRule> assumeRulePool) {
+ Map<DexMember<?, ?>, ProguardMemberRule> assumeRulePool) {
ProguardMemberRule ruleToBePropagated = null;
for (DexType subType : subTypes) {
DexMethod referenceInSubType =
@@ -1170,15 +1171,25 @@
mayHaveSideEffects.put(item.toReference(), rule);
context.markAsUsed();
} else if (context instanceof ProguardAssumeNoSideEffectRule) {
- checkAssumeNoSideEffectsWarnings(item, (ProguardAssumeNoSideEffectRule) context, rule);
- noSideEffects.put(item.toReference(), rule);
- context.markAsUsed();
+ if (item.isDexEncodedMember()) {
+ DexEncodedMember<?, ?> member = item.asDexEncodedMember();
+ if (member.holder() == appView.dexItemFactory().objectType) {
+ assert member.isDexEncodedMethod();
+ reportAssumeNoSideEffectsWarningForJavaLangClassMethod(
+ member.asDexEncodedMethod(), (ProguardAssumeNoSideEffectRule) context);
+ } else {
+ noSideEffects.put(member.toReference(), rule);
+ }
+ context.markAsUsed();
+ }
} else if (context instanceof ProguardWhyAreYouKeepingRule) {
reasonAsked.computeIfAbsent(item.toReference(), i -> i);
context.markAsUsed();
} else if (context instanceof ProguardAssumeValuesRule) {
- assumedValues.put(item.toReference(), rule);
- context.markAsUsed();
+ if (item.isDexEncodedMember()) {
+ assumedValues.put(item.asDexEncodedMember().toReference(), rule);
+ context.markAsUsed();
+ }
} else if (context instanceof ProguardCheckDiscardRule) {
checkDiscarded.computeIfAbsent(item.toReference(), i -> i);
context.markAsUsed();
@@ -1683,18 +1694,13 @@
}
}
- private void checkAssumeNoSideEffectsWarnings(
- DexDefinition item, ProguardAssumeNoSideEffectRule context, ProguardMemberRule rule) {
- if (rule.getRuleType() == ProguardMemberType.METHOD && rule.isSpecific()) {
- return;
- }
- if (item.isDexEncodedMethod()) {
- DexEncodedMethod method = item.asDexEncodedMethod();
- if (method.holder() == options.itemFactory.objectType) {
- OriginWithPosition key = new OriginWithPosition(context.getOrigin(), context.getPosition());
- assumeNoSideEffectsWarnings.computeIfAbsent(key, k -> new ArrayList<>()).add(method.method);
- }
- }
+ private void reportAssumeNoSideEffectsWarningForJavaLangClassMethod(
+ DexEncodedMethod method, ProguardAssumeNoSideEffectRule context) {
+ assert method.getHolderType() == options.dexItemFactory().objectType;
+ OriginWithPosition key = new OriginWithPosition(context.getOrigin(), context.getPosition());
+ assumeNoSideEffectsWarnings
+ .computeIfAbsent(key, ignore -> new TreeSet<>(DexMethod::slowCompareTo))
+ .add(method.getReference());
}
private boolean isWaitOrNotifyMethod(DexMethod method) {
@@ -1708,50 +1714,27 @@
options.getProguardConfiguration() != null
? options.getProguardConfiguration().getDontWarnPatterns()
: ProguardClassFilter.empty();
+ if (dontWarnPatterns.matches(options.itemFactory.objectType)) {
+ // Don't report any warnings since we don't apply -assumenosideeffects rules to notify() or
+ // wait() anyway.
+ return;
+ }
assumeNoSideEffectsWarnings.forEach(
(originWithPosition, methods) -> {
- boolean waitOrNotifyMethods = methods.stream().anyMatch(this::isWaitOrNotifyMethod);
- boolean dontWarnObject = dontWarnPatterns.matches(options.itemFactory.objectType);
- StringBuilder message = new StringBuilder();
- message.append(
- "The -assumenosideeffects rule matches methods on `java.lang.Object` with wildcards");
- message.append(" including the method(s) ");
- for (int i = 0; i < methods.size(); i++) {
- if (i > 0) {
- message.append(i < methods.size() - 1 ? ", " : " and ");
- }
- message.append("`");
- message.append(methods.get(i).toSourceStringWithoutHolder());
- message.append("`.");
+ boolean matchesWaitOrNotifyMethods =
+ methods.stream().anyMatch(this::isWaitOrNotifyMethod);
+ if (!matchesWaitOrNotifyMethods) {
+ // We model the remaining methods on java.lang.Object, and thus there should be no need
+ // to warn in this case.
+ return;
}
- if (waitOrNotifyMethods) {
- message.append(" This will most likely cause problems.");
- } else {
- message.append(" This is most likely not intended.");
- }
- if (waitOrNotifyMethods && !dontWarnObject) {
- message.append(" Specify the methods more precisely.");
- } else {
- message.append(" Consider specifying the methods more precisely.");
- }
- Diagnostic diagnostic =
- new StringDiagnostic(
- message.toString(),
- originWithPosition.getOrigin(),
- originWithPosition.getPosition());
- if (waitOrNotifyMethods) {
- if (!dontWarnObject) {
- options.reporter.error(diagnostic);
- } else {
- options.reporter.warning(diagnostic);
- }
-
- } else {
- if (!dontWarnObject) {
- options.reporter.warning(diagnostic);
- }
- }
+ options.reporter.warning(
+ new AssumeNoSideEffectsRuleForObjectMembersDiagnostic.Builder()
+ .addMatchedMethods(methods)
+ .setOrigin(originWithPosition.getOrigin())
+ .setPosition(originWithPosition.getPosition())
+ .build());
});
}
@@ -1773,8 +1756,8 @@
public final Set<DexType> noStaticClassMerging;
public final Set<DexReference> neverPropagateValue;
public final Map<DexReference, ProguardMemberRule> mayHaveSideEffects;
- public final Map<DexReference, ProguardMemberRule> noSideEffects;
- public final Map<DexReference, ProguardMemberRule> assumedValues;
+ public final Map<DexMember<?, ?>, ProguardMemberRule> noSideEffects;
+ public final Map<DexMember<?, ?>, ProguardMemberRule> assumedValues;
public final Set<DexReference> identifierNameStrings;
public final Set<ProguardIfRule> ifRules;
@@ -1800,8 +1783,8 @@
Set<DexType> noStaticClassMerging,
Set<DexReference> neverPropagateValue,
Map<DexReference, ProguardMemberRule> mayHaveSideEffects,
- Map<DexReference, ProguardMemberRule> noSideEffects,
- Map<DexReference, ProguardMemberRule> assumedValues,
+ Map<DexMember<?, ?>, ProguardMemberRule> noSideEffects,
+ Map<DexMember<?, ?>, ProguardMemberRule> assumedValues,
Map<DexReference, MutableItemsWithRules> dependentNoShrinking,
Map<DexReference, MutableItemsWithRules> dependentSoftPinned,
Map<DexType, Set<ProguardKeepRuleBase>> dependentKeepClassCompatRule,
@@ -1890,11 +1873,15 @@
if (noObfuscation.contains(original)) {
noObfuscation.add(rewritten);
}
- if (noSideEffects.containsKey(original)) {
- noSideEffects.put(rewritten, noSideEffects.get(original));
- }
- if (assumedValues.containsKey(original)) {
- assumedValues.put(rewritten, assumedValues.get(original));
+ if (original.isDexMember()) {
+ assert rewritten.isDexMember();
+ DexMember<?, ?> originalMember = original.asDexMember();
+ if (noSideEffects.containsKey(originalMember)) {
+ noSideEffects.put(rewritten.asDexMember(), noSideEffects.get(originalMember));
+ }
+ if (assumedValues.containsKey(originalMember)) {
+ assumedValues.put(rewritten.asDexMember(), assumedValues.get(originalMember));
+ }
}
}
diff --git a/src/main/java/com/android/tools/r8/utils/MethodReferenceUtils.java b/src/main/java/com/android/tools/r8/utils/MethodReferenceUtils.java
new file mode 100644
index 0000000..7f48f2a
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/utils/MethodReferenceUtils.java
@@ -0,0 +1,36 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.utils;
+
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.references.TypeReference;
+import java.util.Iterator;
+
+public class MethodReferenceUtils {
+
+ public static String toSourceStringWithoutHolderAndReturnType(MethodReference methodReference) {
+ return toSourceString(methodReference, false, false);
+ }
+
+ public static String toSourceString(
+ MethodReference methodReference, boolean includeHolder, boolean includeReturnType) {
+ StringBuilder builder = new StringBuilder();
+ if (includeReturnType) {
+ builder.append(methodReference.getReturnType().getTypeName()).append(" ");
+ }
+ if (includeHolder) {
+ builder.append(methodReference.getHolderClass().getTypeName()).append(".");
+ }
+ builder.append(methodReference.getMethodName()).append("(");
+ Iterator<TypeReference> formalTypesIterator = methodReference.getFormalTypes().iterator();
+ if (formalTypesIterator.hasNext()) {
+ builder.append(formalTypesIterator.next().getTypeName());
+ while (formalTypesIterator.hasNext()) {
+ builder.append(", ").append(formalTypesIterator.next().getTypeName());
+ }
+ }
+ return builder.append(")").toString();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumeNoSideEffectsForJavaLangClassTest.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumeNoSideEffectsForJavaLangClassTest.java
new file mode 100644
index 0000000..eb29f88
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/AssumeNoSideEffectsForJavaLangClassTest.java
@@ -0,0 +1,141 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.shaking.assumenosideeffects;
+
+import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethodWithName;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.onlyIf;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.AssumeMayHaveSideEffects;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class AssumeNoSideEffectsForJavaLangClassTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return TestBase.getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public AssumeNoSideEffectsForJavaLangClassTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(getClass())
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .enableSideEffectAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(this::inspect)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithEmptyOutput();
+ }
+
+ private void inspect(CodeInspector inspector) {
+ ClassSubject testClassSubject = inspector.clazz(TestClass.class);
+ assertThat(testClassSubject, isPresent());
+
+ inspectMethod(testClassSubject.uniqueMethodWithName("testModelingOfSideEffects"), false, false);
+ inspectMethod(
+ testClassSubject.uniqueMethodWithName("testModelingOfSideEffectsMaybeNull"), true, false);
+ inspectMethod(
+ testClassSubject.uniqueMethodWithName("testModelingOfSideEffectsMaybeSubclass"),
+ false,
+ true);
+ }
+
+ private void inspectMethod(
+ MethodSubject methodSubject, boolean maybeNullReceiver, boolean maybeSubtype) {
+ assertThat(methodSubject, isPresent());
+ assertThat(
+ methodSubject, onlyIf(maybeNullReceiver || maybeSubtype, invokesMethodWithName("equals")));
+ assertThat(
+ methodSubject,
+ onlyIf(maybeNullReceiver || maybeSubtype, invokesMethodWithName("hashCode")));
+ assertThat(methodSubject, onlyIf(maybeNullReceiver, invokesMethodWithName("getClass")));
+ assertThat(
+ methodSubject,
+ onlyIf(maybeNullReceiver || maybeSubtype, invokesMethodWithName("toString")));
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ testModelingOfSideEffects();
+ testModelingOfSideEffectsMaybeNull();
+ testModelingOfSideEffectsMaybeSubclass();
+ }
+
+ @AssumeMayHaveSideEffects
+ @NeverInline
+ static void testModelingOfSideEffects() {
+ Object o = new Object();
+ o.equals(new Object());
+ o.hashCode();
+ o.getClass();
+ o.toString();
+ }
+
+ @NeverInline
+ static void testModelingOfSideEffectsMaybeNull() {
+ createNullableObject().equals(new Object());
+ createNullableObject().hashCode();
+ createNullableObject().getClass();
+ createNullableObject().toString();
+ }
+
+ @NeverInline
+ static void testModelingOfSideEffectsMaybeSubclass() {
+ Object o = System.currentTimeMillis() > 0 ? new Object() : new A();
+ o.equals(new Object());
+ o.hashCode();
+ o.getClass();
+ o.toString();
+ }
+
+ static Object createNullableObject() {
+ return System.currentTimeMillis() > 0 ? new Object() : null;
+ }
+ }
+
+ static class A {
+
+ public A() {
+ super();
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ throw new RuntimeException();
+ }
+
+ @Override
+ public int hashCode() {
+ throw new RuntimeException();
+ }
+
+ @Override
+ public String toString() {
+ throw new RuntimeException();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/B152492625.java b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/B152492625.java
index 5b0f344..8da32bc 100644
--- a/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/B152492625.java
+++ b/src/test/java/com/android/tools/r8/shaking/assumenosideeffects/B152492625.java
@@ -7,17 +7,19 @@
import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
import static com.android.tools.r8.DiagnosticsMatcher.diagnosticOrigin;
import static com.android.tools.r8.DiagnosticsMatcher.diagnosticPosition;
+import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethod;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.onlyIf;
+import static java.util.Collections.emptyList;
import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeTrue;
import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.Diagnostic;
+import com.android.tools.r8.ProguardVersion;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestDiagnosticMessages;
import com.android.tools.r8.TestParameters;
@@ -25,14 +27,12 @@
import com.android.tools.r8.position.TextPosition;
import com.android.tools.r8.position.TextRange;
import com.android.tools.r8.utils.BooleanUtils;
-import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
-import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
import com.google.common.collect.ImmutableList;
import java.util.Collection;
import java.util.List;
import org.hamcrest.Matcher;
-import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -54,53 +54,45 @@
this.dontWarnObject = dontWarnObject;
}
- private void noCallToWait(CodeInspector inspector) {
- ClassSubject classSubject = inspector.clazz(TestClass.class);
- assertThat(classSubject, isPresent());
- classSubject.forAllMethods(
- foundMethodSubject ->
- foundMethodSubject
- .instructions(InstructionSubject::isInvokeVirtual)
- .forEach(
- instructionSubject -> {
- Assert.assertNotEquals(
- "wait", instructionSubject.getMethod().name.toString());
- }));
+ private void checkIfWaitIsInvokedFromMain(CodeInspector inspector, boolean isR8) {
+ MethodSubject mainMethodSubject = inspector.clazz(TestClass.class).mainMethod();
+ assertThat(mainMethodSubject, isPresent());
+ assertThat(
+ mainMethodSubject,
+ onlyIf(isR8, invokesMethod("void", "java.lang.Object", "wait", emptyList())));
}
private Matcher<Diagnostic> matchAssumeNoSideEffectsMessage() {
return diagnosticMessage(
containsString(
- "The -assumenosideeffects rule matches methods on `java.lang.Object` with"
- + " wildcards"));
+ "The -assumenosideeffects rule matches the following method(s) on java.lang.Object: "));
}
private Matcher<Diagnostic> matchMessageForAllProblematicMethods() {
return diagnosticMessage(
allOf(
- containsString("void notify()"),
- containsString("void notifyAll()"),
- containsString("void wait()"),
- containsString("void wait(long)"),
- containsString("void wait(long, int)")));
+ containsString("notify()"),
+ containsString("notifyAll()"),
+ containsString("wait()"),
+ containsString("wait(long)"),
+ containsString("wait(long, int)")));
}
private Matcher<Diagnostic> matchMessageForWaitMethods() {
return diagnosticMessage(
allOf(
- containsString("void wait()"),
- containsString("void wait(long)"),
- containsString("void wait(long, int)")));
+ containsString("wait()"),
+ containsString("wait(long)"),
+ containsString("wait(long, int)")));
}
private void assertErrorsOrWarnings(
TestDiagnosticMessages diagnostics, List<Matcher<Diagnostic>> matchers) {
if (dontWarnObject) {
+ diagnostics.assertNoMessages();
+ } else {
diagnostics.assertOnlyWarnings();
diagnostics.assertWarningsMatch(matchers);
- } else {
- diagnostics.assertOnlyErrors();
- diagnostics.assertErrorsMatch(matchers);
}
}
@@ -115,23 +107,18 @@
ImmutableList.of(
allOf(matchAssumeNoSideEffectsMessage(), matchMessageForAllProblematicMethods()));
- try {
- testForR8(parameters.getBackend())
- .addProgramClasses(TestClass.class, B.class)
- .addKeepMainRule(TestClass.class)
- .applyIf(dontWarnObject, tb -> tb.addKeepRules("-dontwarn java.lang.Object"))
- .addKeepRules("-assumenosideeffects class " + B.class.getTypeName() + " { *; }")
- .setMinApi(parameters.getApiLevel())
- .allowDiagnosticWarningMessages()
- .compileWithExpectedDiagnostics(
- diagnostics -> assertErrorsOrWarnings(diagnostics, matchers))
- .inspect(this::noCallToWait)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutputLines("Hello, world");
- assertTrue(dontWarnObject);
- } catch (CompilationFailedException e) {
- assertFalse(dontWarnObject);
- }
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class, B.class)
+ .addKeepMainRule(TestClass.class)
+ .applyIf(dontWarnObject, tb -> tb.addKeepRules("-dontwarn java.lang.Object"))
+ .addKeepRules("-assumenosideeffects class " + B.class.getTypeName() + " { *; }")
+ .setMinApi(parameters.getApiLevel())
+ .allowDiagnosticWarningMessages(!dontWarnObject)
+ .compileWithExpectedDiagnostics(
+ diagnostics -> assertErrorsOrWarnings(diagnostics, matchers))
+ .inspect(inspector -> checkIfWaitIsInvokedFromMain(inspector, true))
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello, world");
}
@Test
@@ -169,30 +156,24 @@
diagnosticOrigin(methodsRuleOrigin),
diagnosticPosition(textRangeForString(methodsRule))));
- try {
- testForR8(parameters.getBackend())
- .addProgramClasses(TestClass.class, B.class)
- .addKeepMainRule(TestClass.class)
- .applyIf(dontWarnObject, tb -> tb.addKeepRules("-dontwarn java.lang.Object"))
- .apply(
- b ->
- b.getBuilder()
- .addProguardConfiguration(ImmutableList.of(starRule), starRuleOrigin))
- .apply(
- b ->
- b.getBuilder()
- .addProguardConfiguration(ImmutableList.of(methodsRule), methodsRuleOrigin))
- .setMinApi(parameters.getApiLevel())
- .allowDiagnosticWarningMessages()
- .compileWithExpectedDiagnostics(
- diagnostics -> assertErrorsOrWarnings(diagnostics, matchers))
- .inspect(this::noCallToWait)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutputLines("Hello, world");
- assertTrue(dontWarnObject);
- } catch (CompilationFailedException e) {
- assertFalse(dontWarnObject);
- }
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class, B.class)
+ .addKeepMainRule(TestClass.class)
+ .applyIf(dontWarnObject, tb -> tb.addKeepRules("-dontwarn java.lang.Object"))
+ .apply(
+ b ->
+ b.getBuilder().addProguardConfiguration(ImmutableList.of(starRule), starRuleOrigin))
+ .apply(
+ b ->
+ b.getBuilder()
+ .addProguardConfiguration(ImmutableList.of(methodsRule), methodsRuleOrigin))
+ .setMinApi(parameters.getApiLevel())
+ .allowDiagnosticWarningMessages(!dontWarnObject)
+ .compileWithExpectedDiagnostics(
+ diagnostics -> assertErrorsOrWarnings(diagnostics, matchers))
+ .inspect(inspector -> checkIfWaitIsInvokedFromMain(inspector, true))
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello, world");
}
@Test
@@ -203,22 +184,9 @@
.applyIf(dontWarnObject, tb -> tb.addKeepRules("-dontwarn java.lang.Object"))
.addKeepRules("-assumenosideeffects class " + B.class.getTypeName() + " { hash*(); }")
.setMinApi(parameters.getApiLevel())
- .allowDiagnosticWarningMessages(!dontWarnObject)
- .compileWithExpectedDiagnostics(
- diagnostics -> {
- if (dontWarnObject) {
- diagnostics.assertNoMessages();
- } else {
- diagnostics.assertOnlyWarnings();
- diagnostics.assertWarningsMatch(
- allOf(
- matchAssumeNoSideEffectsMessage(),
- diagnosticMessage(containsString("int hashCode()"))));
- }
- })
+ .compile()
.run(parameters.getRuntime(), TestClass.class)
- // Code fails with exception if wait call is not removed.
- .assertFailureWithErrorThatThrows(IllegalMonitorStateException.class);
+ .assertSuccessWithOutputLines("Hello, world");
}
@Test
@@ -227,23 +195,18 @@
ImmutableList.of(
allOf(matchAssumeNoSideEffectsMessage(), matchMessageForAllProblematicMethods()));
- try {
- testForR8(parameters.getBackend())
- .addProgramClasses(TestClass.class, B.class)
- .addKeepMainRule(TestClass.class)
- .applyIf(dontWarnObject, tb -> tb.addKeepRules("-dontwarn java.lang.Object"))
- .addKeepRules("-assumenosideeffects class " + B.class.getTypeName() + " { <methods>; }")
- .setMinApi(parameters.getApiLevel())
- .allowDiagnosticWarningMessages()
- .compileWithExpectedDiagnostics(
- diagnostics -> assertErrorsOrWarnings(diagnostics, matchers))
- .inspect(this::noCallToWait)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutputLines("Hello, world");
- assertTrue(dontWarnObject);
- } catch (CompilationFailedException e) {
- assertFalse(dontWarnObject);
- }
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class, B.class)
+ .addKeepMainRule(TestClass.class)
+ .applyIf(dontWarnObject, tb -> tb.addKeepRules("-dontwarn java.lang.Object"))
+ .addKeepRules("-assumenosideeffects class " + B.class.getTypeName() + " { <methods>; }")
+ .setMinApi(parameters.getApiLevel())
+ .allowDiagnosticWarningMessages(!dontWarnObject)
+ .compileWithExpectedDiagnostics(
+ diagnostics -> assertErrorsOrWarnings(diagnostics, matchers))
+ .inspect(inspector -> checkIfWaitIsInvokedFromMain(inspector, true))
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello, world");
}
@Test
@@ -251,36 +214,35 @@
List<Matcher<Diagnostic>> matchers =
ImmutableList.of(allOf(matchAssumeNoSideEffectsMessage(), matchMessageForWaitMethods()));
- try {
- testForR8(parameters.getBackend())
- .addProgramClasses(TestClass.class, B.class)
- .addKeepMainRule(TestClass.class)
- .applyIf(dontWarnObject, tb -> tb.addKeepRules("-dontwarn java.lang.Object"))
- .addKeepRules("-assumenosideeffects class " + B.class.getTypeName() + " { *** w*(...); }")
- .setMinApi(parameters.getApiLevel())
- .allowDiagnosticWarningMessages()
- .compileWithExpectedDiagnostics(
- diagnostics -> assertErrorsOrWarnings(diagnostics, matchers))
- .inspect(this::noCallToWait)
- .run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutputLines("Hello, world");
- assertTrue(dontWarnObject);
- } catch (CompilationFailedException e) {
- assertFalse(dontWarnObject);
- }
+ testForR8(parameters.getBackend())
+ .addProgramClasses(TestClass.class, B.class)
+ .addKeepMainRule(TestClass.class)
+ .applyIf(dontWarnObject, tb -> tb.addKeepRules("-dontwarn java.lang.Object"))
+ .addKeepRules("-assumenosideeffects class " + B.class.getTypeName() + " { *** w*(...); }")
+ .setMinApi(parameters.getApiLevel())
+ .allowDiagnosticWarningMessages(!dontWarnObject)
+ .compileWithExpectedDiagnostics(
+ diagnostics -> assertErrorsOrWarnings(diagnostics, matchers))
+ .inspect(inspector -> checkIfWaitIsInvokedFromMain(inspector, true))
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("Hello, world");
}
@Test
public void testR8WaitSpecificMethodMatch() throws Exception {
assumeTrue("No need to run this with -dontwarn java.lang.Object", !dontWarnObject);
+ List<Matcher<Diagnostic>> matchers = ImmutableList.of(matchAssumeNoSideEffectsMessage());
+
testForR8(parameters.getBackend())
.addProgramClasses(TestClass.class, B.class)
.addKeepMainRule(TestClass.class)
.addKeepRules("-assumenosideeffects class java.lang.Object { void wait(); }")
.setMinApi(parameters.getApiLevel())
- .compile()
- .inspect(this::noCallToWait)
+ .allowDiagnosticWarningMessages()
+ .compileWithExpectedDiagnostics(
+ diagnostics -> assertErrorsOrWarnings(diagnostics, matchers))
+ .inspect(inspector -> checkIfWaitIsInvokedFromMain(inspector, true))
.run(parameters.getRuntime(), TestClass.class)
.assertSuccessWithOutputLines("Hello, world");
}
@@ -313,7 +275,7 @@
assumeTrue("No need to run this with -dontwarn java.lang.Object", !dontWarnObject);
assumeTrue(parameters.isCfRuntime());
- testForProguard()
+ testForProguard(ProguardVersion.getLatest())
.addProgramClasses(TestClass.class, B.class)
.addKeepMainRule(TestClass.class)
.addKeepRules("-assumenosideeffects class " + B.class.getTypeName() + " { *; }")
@@ -321,7 +283,7 @@
.setMinApi(parameters.getApiLevel())
.compile()
.run(parameters.getRuntime(), TestClass.class)
- .assertFailureWithErrorThatThrows(IllegalMonitorStateException.class);
+ .assertSuccessWithOutputLines("Hello, world");
}
@Test
@@ -329,24 +291,28 @@
assumeTrue("No need to run this with -dontwarn java.lang.Object", !dontWarnObject);
assumeTrue(parameters.isCfRuntime());
- testForProguard()
+ testForProguard(ProguardVersion.getLatest())
.addProgramClasses(TestClass.class, B.class)
.addKeepMainRule(TestClass.class)
.addKeepRules("-assumenosideeffects class java.lang.Object { void wait(); }")
.addKeepRules("-dontwarn " + B152492625.class.getTypeName())
.setMinApi(parameters.getApiLevel())
.compile()
- .inspect(this::noCallToWait)
+ .inspect(inspector -> checkIfWaitIsInvokedFromMain(inspector, false))
.run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutputLines("Hello, world");
+ .assertSuccessWithOutput("Hello");
}
static class TestClass {
public void m() throws Exception {
- System.out.println("Hello, world");
- // test fails if wait is not removed.
- wait();
+ System.out.print("Hello");
+ // test fails if wait is removed.
+ try {
+ wait();
+ } catch (IllegalMonitorStateException e) {
+ System.out.println(", world");
+ }
}
public static void main(String[] args) throws Exception {
diff --git a/src/test/java/com/android/tools/r8/smali/OutlineTest.java b/src/test/java/com/android/tools/r8/smali/OutlineTest.java
index 578376e..e85f2d8 100644
--- a/src/test/java/com/android/tools/r8/smali/OutlineTest.java
+++ b/src/test/java/com/android/tools/r8/smali/OutlineTest.java
@@ -21,6 +21,7 @@
import com.android.tools.r8.code.InvokeStaticRange;
import com.android.tools.r8.code.InvokeVirtual;
import com.android.tools.r8.code.MoveResult;
+import com.android.tools.r8.code.MoveResultObject;
import com.android.tools.r8.code.MoveResultWide;
import com.android.tools.r8.code.Return;
import com.android.tools.r8.code.ReturnObject;
@@ -42,7 +43,6 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
-import java.util.IdentityHashMap;
import java.util.List;
import java.util.function.Consumer;
import org.junit.Assert;
@@ -1299,24 +1299,23 @@
SmaliBuilder builder = new SmaliBuilder(DEFAULT_CLASS_NAME);
// The result from neither the div-int is never used.
- MethodSignature signature = builder.addStaticMethod(
- "void",
- DEFAULT_METHOD_NAME,
- ImmutableList.of("int", "int"),
- 1,
- " div-int v0, p0, p1",
- " new-instance v0, Ljava/lang/StringBuilder;",
- " invoke-direct { v0 }, Ljava/lang/StringBuilder;-><init>()V",
- " return-void"
- );
+ MethodSignature signature =
+ builder.addStaticMethod(
+ "java.lang.Object",
+ DEFAULT_METHOD_NAME,
+ ImmutableList.of("int", "int"),
+ 1,
+ " div-int v0, p0, p1",
+ " new-instance v0, Ljava/lang/StringBuilder;",
+ " invoke-direct { v0 }, Ljava/lang/StringBuilder;-><init>()V",
+ " return-object v0");
builder.addMainMethod(
2,
" const v0, 1",
" const v1, 2",
- " invoke-static { v0, v1 }, LTest;->method(II)V",
- " return-void"
- );
+ " invoke-static { v0, v1 }, LTest;->method(II)Ljava/lang/Object;",
+ " return-void");
Consumer<InternalOptions> options =
configureOptions(
@@ -1324,20 +1323,6 @@
opts.outline.threshold = 1;
opts.outline.minSize = 3;
opts.outline.maxSize = 3;
-
- // Do not allow dead code elimination of the new-instance and invoke-direct
- // instructions.
- // This can be achieved by not assuming that StringBuilder is present.
- DexItemFactory dexItemFactory = opts.itemFactory;
- dexItemFactory.libraryTypesAssumedToBePresent =
- new HashSet<>(dexItemFactory.libraryTypesAssumedToBePresent);
- dexItemFactory.libraryTypesAssumedToBePresent.remove(
- dexItemFactory.stringBuilderType);
- // ... and not assuming that StringBuilder.<init>() cannot have side effects.
- dexItemFactory.libraryMethodsWithoutSideEffects =
- new IdentityHashMap<>(dexItemFactory.libraryMethodsWithoutSideEffects);
- dexItemFactory.libraryMethodsWithoutSideEffects.remove(
- dexItemFactory.stringBuilderMethods.defaultConstructor);
});
AndroidApp originalApplication = buildApplicationWithAndroidJar(builder);
@@ -1347,9 +1332,10 @@
// Return the processed method for inspection.
DexEncodedMethod method = getMethod(processedApplication, signature);
DexCode code = method.getCode().asDexCode();
- assertEquals(2, code.instructions.length);
+ assertEquals(3, code.instructions.length);
assertTrue(code.instructions[0] instanceof InvokeStatic);
- assertTrue(code.instructions[1] instanceof ReturnVoid);
+ assertTrue(code.instructions[1] instanceof MoveResultObject);
+ assertTrue(code.instructions[2] instanceof ReturnObject);
InvokeStatic invoke = (InvokeStatic) code.instructions[0];
assertEquals(firstOutlineMethodName(), invoke.getMethod().qualifiedName());
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java
index 86a21c1..ed4d158 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/CodeMatchers.java
@@ -6,7 +6,9 @@
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexMethod;
+import java.util.List;
import java.util.function.Predicate;
+import java.util.stream.Collectors;
import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.hamcrest.TypeSafeMatcher;
@@ -71,10 +73,93 @@
};
}
+ public static Matcher<MethodSubject> invokesMethod(
+ String returnType, String holderType, String methodName, List<String> parameterTypes) {
+ return new TypeSafeMatcher<MethodSubject>() {
+ @Override
+ protected boolean matchesSafely(MethodSubject subject) {
+ if (!subject.isPresent()) {
+ return false;
+ }
+ if (!subject.getMethod().hasCode()) {
+ return false;
+ }
+ return subject
+ .streamInstructions()
+ .anyMatch(isInvokeWithTarget(returnType, holderType, methodName, parameterTypes));
+ }
+
+ @Override
+ public void describeTo(Description description) {
+ StringBuilder text =
+ new StringBuilder("invokes method `")
+ .append(returnType != null ? returnType : "*")
+ .append(" ")
+ .append(holderType != null ? holderType : "*")
+ .append(".")
+ .append(methodName != null ? methodName : "*")
+ .append("(");
+ if (parameterTypes != null) {
+ text.append(
+ parameterTypes.stream()
+ .map(parameterType -> parameterType != null ? parameterType : "*")
+ .collect(Collectors.joining(", ")));
+ } else {
+ text.append("...");
+ }
+ text.append(")`");
+ description.appendText(text.toString());
+ }
+
+ @Override
+ public void describeMismatchSafely(final MethodSubject subject, Description description) {
+ description.appendText("method did not");
+ }
+ };
+ }
+
+ public static Matcher<MethodSubject> invokesMethodWithName(String name) {
+ return invokesMethod(null, null, name, null);
+ }
+
public static Predicate<InstructionSubject> isInvokeWithTarget(DexMethod target) {
return instruction -> instruction.isInvoke() && instruction.getMethod() == target;
}
+ public static Predicate<InstructionSubject> isInvokeWithTarget(
+ String returnType, String holderType, String methodName, List<String> parameterTypes) {
+ return instruction -> {
+ if (!instruction.isInvoke()) {
+ return false;
+ }
+ DexMethod invokedMethod = instruction.getMethod();
+ if (returnType != null
+ && !invokedMethod.getReturnType().toSourceString().equals(returnType)) {
+ return false;
+ }
+ if (holderType != null
+ && !invokedMethod.getHolderType().toSourceString().equals(holderType)) {
+ return false;
+ }
+ if (methodName != null && !invokedMethod.getName().toSourceString().equals(methodName)) {
+ return false;
+ }
+ if (parameterTypes != null) {
+ if (parameterTypes.size() != invokedMethod.getArity()) {
+ return false;
+ }
+ for (int i = 0; i < parameterTypes.size(); i++) {
+ String parameterType = parameterTypes.get(i);
+ if (parameterType != null
+ && !invokedMethod.getParameter(i).toSourceString().equals(parameterType)) {
+ return false;
+ }
+ }
+ }
+ return true;
+ };
+ }
+
public static Predicate<InstructionSubject> isFieldAccessWithTarget(DexField target) {
return instruction -> instruction.isFieldAccess() && instruction.getField() == target;
}
diff --git a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
index 98c8562..29bebe1 100644
--- a/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
+++ b/src/test/java/com/android/tools/r8/utils/codeinspector/Matchers.java
@@ -713,6 +713,10 @@
}
}
+ public static <T> Matcher<T> onlyIf(boolean condition, Matcher<T> matcher) {
+ return notIf(matcher, !condition);
+ }
+
public static <T> Matcher<T> notIf(Matcher<T> matcher, boolean condition) {
if (condition) {
return not(matcher);
