Revert "Remove invalid nest check from isValidVirtualTarget predicate." This reverts commit 8895fb3ca4d639ee9f64a1f39be25d90bcc6e22d. Reason for revert: bots are failing Change-Id: Ib090331dbf7794550c4c6bfbfcfa4cc04ed6285e
diff --git a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java index 2c60a43..3080960 100644 --- a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java +++ b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
@@ -6,6 +6,7 @@ import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull; import com.android.tools.r8.errors.CompilationError; +import com.android.tools.r8.utils.InternalOptions; import com.android.tools.r8.utils.SetUtils; import com.google.common.collect.Sets; import java.util.Collection; @@ -56,12 +57,8 @@ public abstract boolean isAccessibleForVirtualDispatchFrom( DexProgramClass context, AppInfoWithSubtyping appInfo); - // TODO(b/145187573): Remove this and use proper access checks. - @Deprecated - public abstract boolean isValidVirtualTarget(); + public abstract boolean isValidVirtualTarget(InternalOptions options); - // TODO(b/145187573): Remove this and use proper access checks. - @Deprecated public abstract boolean isValidVirtualTargetForDynamicDispatch(); /** Lookup the single target of an invoke-special on this resolution result if possible. */ @@ -90,10 +87,10 @@ private final DexClass resolvedHolder; private final DexEncodedMethod resolvedMethod; - // TODO(b/145187573): Remove this and use proper access checks. - @Deprecated - public static boolean isValidVirtualTarget(DexEncodedMethod target) { - return !target.accessFlags.isStatic() && !target.accessFlags.isConstructor(); + public static boolean isValidVirtualTarget(InternalOptions options, DexEncodedMethod target) { + return options.canUseNestBasedAccess() + ? (!target.accessFlags.isStatic() && !target.accessFlags.isConstructor()) + : target.isVirtualMethod(); } public SingleResolutionResult( @@ -144,8 +141,8 @@ } @Override - public boolean isValidVirtualTarget() { - return isValidVirtualTarget(resolvedMethod); + public boolean isValidVirtualTarget(InternalOptions options) { + return isValidVirtualTarget(options, resolvedMethod); } @Override @@ -304,7 +301,7 @@ @Override // TODO(b/140204899): Leverage refined receiver type if available. public Set<DexEncodedMethod> lookupVirtualTargets(AppInfoWithSubtyping appInfo) { - assert isValidVirtualTarget(); + assert isValidVirtualTarget(appInfo.app().options); // First add the target for receiver type method.type. DexEncodedMethod encodedMethod = getSingleTarget(); Set<DexEncodedMethod> result = SetUtils.newIdentityHashSet(encodedMethod); @@ -328,7 +325,7 @@ @Override // TODO(b/140204899): Leverage refined receiver type if available. public Set<DexEncodedMethod> lookupInterfaceTargets(AppInfoWithSubtyping appInfo) { - assert isValidVirtualTarget(); + assert isValidVirtualTarget(appInfo.app().options); Set<DexEncodedMethod> result = Sets.newIdentityHashSet(); if (isSingleResolution()) { // Add default interface methods to the list of targets. @@ -454,7 +451,7 @@ } @Override - public boolean isValidVirtualTarget() { + public boolean isValidVirtualTarget(InternalOptions options) { return true; } @@ -493,7 +490,7 @@ } @Override - public boolean isValidVirtualTarget() { + public boolean isValidVirtualTarget(InternalOptions options) { return false; }
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java b/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java index 0e6d2a3..c7939b5 100644 --- a/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java +++ b/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilderBase.java
@@ -180,7 +180,7 @@ method -> { ResolutionResult resolution = appView.appInfo().resolveMethod(method.holder, method, isInterface); - if (resolution.isValidVirtualTarget()) { + if (resolution.isValidVirtualTarget(appView.options())) { return resolution.lookupVirtualDispatchTargets(isInterface, appView.appInfo()); } return null;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java b/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java index 133eff3..e83a17a 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/CallSiteOptimizationInfoPropagator.java
@@ -88,7 +88,7 @@ ResolutionResult resolutionResult = appView.appInfo().resolveMethod(invokedMethod.holder, invokedMethod); // For virtual and interface calls, proceed on valid results only (since it's enforced). - if (!resolutionResult.isValidVirtualTarget()) { + if (!resolutionResult.isValidVirtualTarget(appView.options())) { continue; } // If the resolution ended up with a single target, check if it is a library override.
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java index e0219c5..4977d4c 100644 --- a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java +++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
@@ -352,7 +352,7 @@ // resolution result. ResolutionResult resolutionResult = appView.appInfo().resolveMethod(method.holder, method, isInterface); - if (!resolutionResult.isValidVirtualTarget()) { + if (!resolutionResult.isValidVirtualTarget(appView.options())) { return ConstraintWithTarget.NEVER; }
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 4e248d9..25a7b15 100644 --- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java +++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -979,7 +979,7 @@ private DexEncodedMethod validateSingleVirtualTarget( DexEncodedMethod singleTarget, DexEncodedMethod resolutionResult) { - assert SingleResolutionResult.isValidVirtualTarget(resolutionResult); + assert SingleResolutionResult.isValidVirtualTarget(options(), resolutionResult); if (singleTarget == null || singleTarget == DexEncodedMethod.SENTINEL) { return null; @@ -996,7 +996,7 @@ private boolean isInvalidSingleVirtualTarget( DexEncodedMethod singleTarget, DexEncodedMethod resolutionResult) { - assert SingleResolutionResult.isValidVirtualTarget(resolutionResult); + assert SingleResolutionResult.isValidVirtualTarget(options(), resolutionResult); // Art978_virtual_interfaceTest correctly expects an IncompatibleClassChangeError exception // at runtime. return !singleTarget.accessFlags.isAtLeastAsVisibleAs(resolutionResult.accessFlags); @@ -1081,7 +1081,7 @@ // First get the target for the holder type. ResolutionResult topMethod = resolveMethodOnClass(holder, method); // We might hit none or multiple targets. Both make this fail at runtime. - if (!topMethod.isSingleResolution() || !topMethod.isValidVirtualTarget()) { + if (!topMethod.isSingleResolution() || !topMethod.isValidVirtualTarget(options())) { method.setSingleVirtualMethodCache(refinedReceiverType, null); return 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 1fa2300..4b412cf 100644 --- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java +++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1658,7 +1658,7 @@ } private void markResolutionAsLive(DexClass libraryClass, ResolutionResult resolution) { - if (resolution.isValidVirtualTarget()) { + if (resolution.isValidVirtualTarget(options)) { DexEncodedMethod target = resolution.getSingleTarget(); DexProgramClass targetHolder = getProgramClassOrNull(target.method.holder); if (targetHolder != null @@ -2004,7 +2004,7 @@ resolution = findAndMarkResolutionTarget(method, interfaceInvoke, reason); virtualTargetsMarkedAsReachable.put(method, resolution); if (resolution.isUnresolved() - || !SingleResolutionResult.isValidVirtualTarget(resolution.method)) { + || !SingleResolutionResult.isValidVirtualTarget(options, resolution.method)) { // There is no valid resolution, so any call will lead to a runtime exception. return; } @@ -2035,7 +2035,7 @@ MarkedResolutionTarget reason, BiPredicate<DexProgramClass, DexEncodedMethod> possibleTargetsFilter, DexEncodedMethod encodedPossibleTarget) { - assert encodedPossibleTarget.isVirtualMethod() || encodedPossibleTarget.isPrivateMethod(); + assert encodedPossibleTarget.isVirtualMethod() || options.canUseNestBasedAccess(); assert !encodedPossibleTarget.isAbstract(); DexMethod possibleTarget = encodedPossibleTarget.method; DexProgramClass clazz = getProgramClassOrNull(possibleTarget.holder);
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 bd05dc3..b86857a 100644 --- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java +++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -533,7 +533,8 @@ } ResolutionResult resolutionResult = appView.appInfo().resolveMethod(originalClazz, method.method); - if (!resolutionResult.isValidVirtualTarget() || !resolutionResult.isSingleResolution()) { + if (!resolutionResult.isValidVirtualTarget(appView.options()) + || !resolutionResult.isSingleResolution()) { return; } DexEncodedMethod methodToKeep = resolutionResult.getSingleTarget();
diff --git a/src/test/java/com/android/tools/r8/resolution/PrivateInvokeVirtualTest.java b/src/test/java/com/android/tools/r8/resolution/PrivateInvokeVirtualTest.java deleted file mode 100644 index 543ecad..0000000 --- a/src/test/java/com/android/tools/r8/resolution/PrivateInvokeVirtualTest.java +++ /dev/null
@@ -1,95 +0,0 @@ -// 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.resolution; - -import static org.junit.Assert.assertEquals; - -import com.android.tools.r8.CompilationFailedException; -import com.android.tools.r8.TestBase; -import com.android.tools.r8.TestParameters; -import com.android.tools.r8.TestParametersCollection; -import java.io.IOException; -import java.util.concurrent.ExecutionException; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameters; -import org.objectweb.asm.Opcodes; - -@RunWith(Parameterized.class) -public class PrivateInvokeVirtualTest extends TestBase { - - private final TestParameters parameters; - - @Parameters(name = "{0}") - public static TestParametersCollection data() { - return getTestParameters().withAllRuntimesAndApiLevels().build(); - } - - public PrivateInvokeVirtualTest(TestParameters parameters) { - this.parameters = parameters; - } - - public static class UInt implements Comparable<UInt> { - - private final int val; - - public UInt(int val) { - this.val = val; - } - - private int compareToHelper(int i) { - return Integer.compare(val, i); - } - - @Override - public int compareTo(UInt o) { - return compareToHelper(o.val); - } - } - - public static class Main { - - public static void main(String[] args) { - System.out.println(new UInt(3).compareTo(new UInt(args.length))); - } - } - - @Test - public void testRuntime() throws IOException, CompilationFailedException, ExecutionException { - testForRuntime(parameters) - .addProgramClassFileData(getUIntWithTransformedInvoke()) - .addProgramClasses(Main.class) - .run(parameters.getRuntime(), Main.class) - .assertSuccessWithOutputLines("1"); - } - - @Test - public void testPrivateInvokeVirtual() - throws IOException, CompilationFailedException, ExecutionException { - testForR8(parameters.getBackend()) - .addProgramClasses(Main.class) - .addProgramClassFileData(getUIntWithTransformedInvoke()) - .addKeepMainRule(Main.class) - .setMinApi(parameters.getApiLevel()) - .run(parameters.getRuntime(), Main.class) - .assertSuccessWithOutputLines("1"); - } - - private byte[] getUIntWithTransformedInvoke() throws IOException { - return transformer(UInt.class) - .transformMethodInsnInMethod( - "compareTo", - (opcode, owner, name, descriptor, isInterface, continuation) -> { - if (name.contains("compareToHelper")) { - assertEquals(Opcodes.INVOKESPECIAL, opcode); - continuation.apply(Opcodes.INVOKEVIRTUAL, owner, name, descriptor, isInterface); - } else { - continuation.apply(opcode, owner, name, descriptor, isInterface); - } - }) - .transform(); - } -}
diff --git a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java index 27c7724..b1bcb52 100644 --- a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java +++ b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
@@ -228,7 +228,7 @@ Assert.assertNotNull( appInfo.resolveMethod(toType(invokeReceiver, appInfo), method).getSingleTarget()); ResolutionResult resolutionResult = appInfo.resolveMethod(method.holder, method); - if (resolutionResult.isValidVirtualTarget()) { + if (resolutionResult.isValidVirtualTarget(appInfo.app().options)) { Set<DexEncodedMethod> targets = resolutionResult.lookupVirtualTargets(appInfo); Set<DexType> targetHolders = targets.stream().map(m -> m.method.holder).collect(Collectors.toSet());
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java index 82452a8..63aa76f 100644 --- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java +++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java
@@ -122,7 +122,7 @@ appInfo.resolveMethodOnInterface(methodOnB.holder, methodOnB); DexEncodedMethod resolved = resolutionResult.getSingleTarget(); assertEquals(methodOnB, resolved.method); - assertFalse(resolutionResult.isValidVirtualTarget()); + assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options)); DexEncodedMethod singleVirtualTarget = appInfo.lookupSingleInterfaceTarget(methodOnB, methodOnB.holder); Assert.assertNull(singleVirtualTarget); @@ -133,7 +133,7 @@ ResolutionResult resolutionResult = appInfo.resolveMethodOnInterface(methodOnB.holder, methodOnB); DexEncodedMethod resolved = resolutionResult.getSingleTarget(); assertEquals(methodOnB, resolved.method); - assertFalse(resolutionResult.isValidVirtualTarget()); + assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options)); } @Test
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java index 702eca9..22a12f2 100644 --- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java +++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
@@ -168,7 +168,7 @@ ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB); DexEncodedMethod resolved = resolutionResult.getSingleTarget(); assertEquals(methodOnA, resolved.method); - assertFalse(resolutionResult.isValidVirtualTarget()); + assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options)); DexEncodedMethod singleVirtualTarget = appInfo.lookupSingleVirtualTarget(methodOnB, methodOnB.holder); Assert.assertNull(singleVirtualTarget); @@ -179,7 +179,7 @@ ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB); DexEncodedMethod resolved = resolutionResult.getSingleTarget(); assertEquals(methodOnA, resolved.method); - assertFalse(resolutionResult.isValidVirtualTarget()); + assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options)); } @Test