Enable inlining of synchronized methods
Change-Id: I399f5583f940efb9035bb813e4d7ddcabb835e8b
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 f34e2bc..09a2c32 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -464,9 +464,6 @@
Position callerPosition,
Origin origin) {
checkIfObsolete();
- if (accessFlags.isSynchronized()) {
- throw new Unreachable("Invalid attempt to build synchronized method for inlining");
- }
return code.buildInliningIR(
context, this, appView, valueNumberGenerator, callerPosition, origin);
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
index fa9384a..c9c6c86 100644
--- a/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
+++ b/src/main/java/com/android/tools/r8/ir/code/BasicBlock.java
@@ -214,6 +214,13 @@
return normals.build();
}
+ public int numberOfNormalSuccessors() {
+ if (hasCatchHandlers()) {
+ return successors.size() - catchHandlers.getUniqueTargets().size();
+ }
+ return successors.size();
+ }
+
public boolean hasUniquePredecessor() {
return predecessors.size() == 1;
}
@@ -893,11 +900,39 @@
catchHandlers = new CatchHandlers<>(guards, successorIndexes);
}
- public void addCatchHandler(BasicBlock rethrowBlock, DexType guard) {
- assert !hasCatchHandlers();
- getMutableSuccessors().add(0, rethrowBlock);
- rethrowBlock.getMutablePredecessors().add(this);
- catchHandlers = new CatchHandlers<>(ImmutableList.of(guard), ImmutableList.of(0));
+ public void appendCatchHandler(BasicBlock target, DexType guard) {
+ if (!canThrow()) {
+ // Nothing to catch.
+ return;
+ }
+ if (hasCatchHandlers()) {
+ if (catchHandlers.getGuards().contains(guard)) {
+ // Subsumed by an existing catch handler.
+ return;
+ }
+ int targetIndex = successors.indexOf(target);
+ if (targetIndex < 0) {
+ List<BasicBlock> successors = getMutableSuccessors();
+ int numberOfSuccessors = successors.size();
+ int numberOfNormalSuccessors = numberOfNormalSuccessors();
+ if (numberOfNormalSuccessors > 0) {
+ // Increase the size of the successor list by 1, and increase the index of each normal
+ // successor by 1.
+ successors.add(numberOfSuccessors - numberOfNormalSuccessors - 1, target);
+ } else {
+ // If there are no normal successors we can simply add the new catch handler.
+ targetIndex = successors.size();
+ successors.add(target);
+ }
+ target.getMutablePredecessors().add(this);
+ }
+ catchHandlers = catchHandlers.appendGuard(guard, targetIndex);
+ } else {
+ assert instructions.stream().filter(Instruction::instructionTypeCanThrow).count() == 1;
+ getMutableSuccessors().add(0, target);
+ target.getMutablePredecessors().add(this);
+ catchHandlers = new CatchHandlers<>(ImmutableList.of(guard), ImmutableList.of(0));
+ }
}
/**
@@ -1795,8 +1830,7 @@
continue;
}
BasicBlock catchSuccessor = fromBlock.successors.get(prevCatchTarget);
- // We assume that all the catch handlers targets has only one
- // predecessor and, thus, no phis.
+ // We assume that all the catch handlers targets has only one predecessor and, thus, no phis.
assert catchSuccessor.getPredecessors().size() == 1;
assert catchSuccessor.getPhis().isEmpty();
diff --git a/src/main/java/com/android/tools/r8/ir/code/CatchHandlers.java b/src/main/java/com/android/tools/r8/ir/code/CatchHandlers.java
index 237c0cc..95ee7e0 100644
--- a/src/main/java/com/android/tools/r8/ir/code/CatchHandlers.java
+++ b/src/main/java/com/android/tools/r8/ir/code/CatchHandlers.java
@@ -76,6 +76,13 @@
&& getGuards().get(getGuards().size() - 1) == factory.throwableType;
}
+ public CatchHandlers<T> appendGuard(DexType guard, T target) {
+ assert !guards.contains(guard);
+ List<DexType> newGuards = ImmutableList.<DexType>builder().addAll(guards).add(guard).build();
+ List<T> newTargets = ImmutableList.<T>builder().addAll(targets).add(target).build();
+ return new CatchHandlers<>(newGuards, newTargets);
+ }
+
public CatchHandlers<T> removeGuard(DexType guardToBeRemoved) {
List<DexType> newGuards = new ArrayList<>();
List<T> newTargets = new ArrayList<>();
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 fee2c94..1078172 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
@@ -13,12 +13,14 @@
import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.Nullability;
import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
+import com.android.tools.r8.ir.code.Phi.RegisterReadType;
import com.android.tools.r8.ir.conversion.IRBuilder;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.VerticalClassMerger.VerticallyMergedClasses;
import com.android.tools.r8.utils.CfgPrinter;
import com.android.tools.r8.utils.DequeUtils;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.StringUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -133,11 +135,6 @@
return metadata;
}
- public void mergeMetadataFromInlinee(IRCode inlinee) {
- assert !inlinee.metadata.mayHaveMonitorInstruction();
- this.metadata.merge(inlinee.metadata);
- }
-
public BasicBlock entryBlock() {
return blocks.getFirst();
}
@@ -286,6 +283,53 @@
return false;
}
+ /**
+ * Prepares every block for getting catch handlers. This involves splitting blocks until each
+ * block has only one throwing instruction, and all instructions after the throwing instruction
+ * are allowed to follow a throwing instruction.
+ *
+ * <p>It is also a requirement that the entry block does not have any catch handlers, thus the
+ * entry block is split to ensure that it contains no throwing instructions.
+ *
+ * <p>This method also inserts a split block between each block with more than two predecessors
+ * and the predecessors that have a throwing instruction. This is necessary because adding catch
+ * handlers to a predecessor would otherwise lead to critical edges.
+ */
+ public void prepareBlocksForCatchHandlers() {
+ BasicBlock entryBlock = entryBlock();
+ ListIterator<BasicBlock> blockIterator = listIterator();
+ while (blockIterator.hasNext()) {
+ BasicBlock block = blockIterator.next();
+ InstructionListIterator instructionIterator = block.listIterator(this);
+ boolean hasSeenThrowingInstruction = false;
+ while (instructionIterator.hasNext()) {
+ Instruction instruction = instructionIterator.next();
+ boolean instructionTypeCanThrow = instruction.instructionTypeCanThrow();
+ if ((hasSeenThrowingInstruction && !instruction.isAllowedAfterThrowingInstruction())
+ || (instructionTypeCanThrow && block == entryBlock)) {
+ instructionIterator.previous();
+ instructionIterator.split(this, blockIterator);
+ blockIterator.previous();
+ break;
+ }
+ if (instructionTypeCanThrow) {
+ hasSeenThrowingInstruction = true;
+ }
+ }
+ if (hasSeenThrowingInstruction) {
+ List<BasicBlock> successors = block.getSuccessors();
+ if (successors.size() == 1 && ListUtils.first(successors).getPredecessors().size() > 1) {
+ BasicBlock splitBlock = block.createSplitBlock(getHighestBlockNumber() + 1, true);
+ Goto newGoto = new Goto(block);
+ newGoto.setPosition(Position.none());
+ splitBlock.listIterator(this).add(newGoto);
+ blockIterator.add(splitBlock);
+ }
+ }
+ }
+ assert blocks.stream().allMatch(block -> block.numberOfThrowingInstructions() <= 1);
+ }
+
public void splitCriticalEdges() {
List<BasicBlock> newBlocks = new ArrayList<>();
int nextBlockNumber = getHighestBlockNumber() + 1;
@@ -959,6 +1003,10 @@
return createValue(typeLattice, null);
}
+ public Phi createPhi(BasicBlock block, TypeLatticeElement type) {
+ return new Phi(valueNumberGenerator.next(), block, type, null, RegisterReadType.NORMAL);
+ }
+
public ConstNumber createIntConstant(int value) {
Value out = createValue(TypeLatticeElement.INT);
return new ConstNumber(out, value);
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 7c1590c..b7d3dcd 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
@@ -441,7 +441,7 @@
BasicBlock.createRethrowBlock(code, lastSelfRecursiveCall.getPosition(), guard, appView);
code.blocks.add(rethrowBlock);
// Add catch handler to the block containing the last recursive call.
- newBlock.addCatchHandler(rethrowBlock, guard);
+ newBlock.appendCatchHandler(rethrowBlock, guard);
}
}
@@ -1220,16 +1220,18 @@
// and for the constructor. To be eligible method should only be using its
// receiver in the following ways:
//
- // (1) as a receiver of reads/writes of instance fields of the holder class
- // (2) as a return value
+ // (1) as a receiver of reads/writes of instance fields of the holder class,
+ // (2) as a return value,
// (3) as a receiver of a call to the superclass initializer. Note that we don't
// check what is passed to superclass initializer as arguments, only check
- // that it is not the instance being initialized.
+ // that it is not the instance being initialized,
+ // (4) as argument to a monitor instruction.
//
+ // Note that (4) can safely be removed as the receiver is guaranteed not to escape when we class
+ // inline it, and hence any monitor instructions are no-ops.
boolean instanceInitializer = method.isInstanceInitializer();
if (method.accessFlags.isNative()
- || (!method.isNonAbstractVirtualMethod() && !instanceInitializer)
- || method.accessFlags.isSynchronized()) {
+ || (!method.isNonAbstractVirtualMethod() && !instanceInitializer)) {
return;
}
@@ -1248,6 +1250,10 @@
boolean receiverUsedAsReturnValue = false;
boolean seenSuperInitCall = false;
for (Instruction insn : receiver.uniqueUsers()) {
+ if (insn.isMonitor()) {
+ continue;
+ }
+
if (insn.isReturn()) {
receiverUsedAsReturnValue = true;
continue;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
index 263da6c..7306af2 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DefaultInliningOracle.java
@@ -264,12 +264,6 @@
return false;
}
- // Don't inline if target is synchronized.
- if (singleTarget.accessFlags.isSynchronized()) {
- whyAreYouNotInliningReporter.reportSynchronizedMethod();
- return false;
- }
-
if (reason == Reason.DUAL_CALLER) {
if (satisfiesRequirementsForSimpleInlining(invoke, singleTarget)) {
// When we have a method with two call sites, we simply inline the method as we normally do
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
index 44c72c5..ce040ff 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Inliner.java
@@ -3,20 +3,27 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.optimize;
+import static com.android.tools.r8.ir.analysis.type.Nullability.definitelyNotNull;
+import static com.google.common.base.Predicates.not;
+
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AccessFlags;
import com.android.tools.r8.graph.AppInfoWithSubtyping;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexEncodedMethod;
+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.DexType;
import com.android.tools.r8.graph.GraphLense;
import com.android.tools.r8.graph.NestMemberClassAttribute;
import com.android.tools.r8.ir.analysis.ClassInitializationAnalysis;
+import com.android.tools.r8.ir.analysis.type.Nullability;
+import com.android.tools.r8.ir.analysis.type.TypeLatticeElement;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.CatchHandlers.CatchHandler;
+import com.android.tools.r8.ir.code.ConstClass;
import com.android.tools.r8.ir.code.IRCode;
import com.android.tools.r8.ir.code.If;
import com.android.tools.r8.ir.code.Instruction;
@@ -24,6 +31,9 @@
import com.android.tools.r8.ir.code.InstructionListIterator;
import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.code.InvokeMethod;
+import com.android.tools.r8.ir.code.Monitor;
+import com.android.tools.r8.ir.code.MoveException;
+import com.android.tools.r8.ir.code.Phi;
import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Throw;
import com.android.tools.r8.ir.code.Value;
@@ -41,6 +51,7 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.android.tools.r8.shaking.MainDexClasses;
import com.android.tools.r8.utils.InternalOptions;
+import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.ThreadUtils;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
@@ -575,14 +586,26 @@
AppView<? extends AppInfoWithSubtyping> appView,
Position callerPosition,
LensCodeRewriter lensCodeRewriter) {
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
+ InternalOptions options = appView.options();
Origin origin = appView.appInfo().originFor(target.method.holder);
// Build the IR for a yet not processed method, and perform minimal IR processing.
IRCode code = target.buildInliningIR(context, appView, generator, callerPosition, origin);
- // Insert a null check if this is needed to preserve the implicit null check for the
- // receiver.
- if (shouldSynthesizeNullCheckForReceiver) {
+ // Insert a null check if this is needed to preserve the implicit null check for the receiver.
+ // This is only needed if we do not also insert a monitor-enter instruction, since that will
+ // throw a NPE if the receiver is null.
+ //
+ // Note: When generating DEX, we synthesize monitor-enter/exit instructions during IR
+ // building, and therefore, we do not need to do anything here. Upon writing, we will use the
+ // flag "declared synchronized" instead of "synchronized".
+ boolean shouldSynthesizeMonitorEnterExit =
+ target.accessFlags.isSynchronized() && options.isGeneratingClassFiles();
+ boolean isSynthesizingNullCheckForReceiverUsingMonitorEnter =
+ shouldSynthesizeMonitorEnterExit && !target.isStatic();
+ if (shouldSynthesizeNullCheckForReceiver
+ && !isSynthesizingNullCheckForReceiverUsingMonitorEnter) {
List<Value> arguments = code.collectArguments();
if (!arguments.isEmpty()) {
Value receiver = arguments.get(0);
@@ -617,9 +640,119 @@
assert false : "Unable to synthesize a null check for the receiver";
}
}
+
+ // Insert monitor-enter and monitor-exit instructions if the method is synchronized.
+ if (shouldSynthesizeMonitorEnterExit) {
+ TypeLatticeElement throwableType =
+ TypeLatticeElement.fromDexType(
+ dexItemFactory.throwableType, Nullability.definitelyNotNull(), appView);
+
+ code.prepareBlocksForCatchHandlers();
+
+ int nextBlockNumber = code.getHighestBlockNumber() + 1;
+
+ // Create a block for holding the monitor-exit instruction.
+ BasicBlock monitorExitBlock = new BasicBlock();
+ monitorExitBlock.setNumber(nextBlockNumber++);
+
+ // For each block in the code that may throw, add a catch-all handler targeting the
+ // monitor-exit block.
+ List<BasicBlock> moveExceptionBlocks = new ArrayList<>();
+ for (BasicBlock block : code.blocks) {
+ if (!block.canThrow()) {
+ continue;
+ }
+ if (block.hasCatchHandlers()
+ && block.getCatchHandlersWithSuccessorIndexes().hasCatchAll(dexItemFactory)) {
+ continue;
+ }
+ BasicBlock moveExceptionBlock =
+ BasicBlock.createGotoBlock(
+ nextBlockNumber++, Position.none(), code.metadata(), monitorExitBlock);
+ InstructionListIterator moveExceptionBlockIterator =
+ moveExceptionBlock.listIterator(code);
+ moveExceptionBlockIterator.setInsertionPosition(Position.syntheticNone());
+ moveExceptionBlockIterator.add(
+ new MoveException(
+ code.createValue(throwableType), dexItemFactory.throwableType, options));
+ block.appendCatchHandler(moveExceptionBlock, dexItemFactory.throwableType);
+ moveExceptionBlocks.add(moveExceptionBlock);
+ }
+
+ // Create a phi for the exception values such that we can rethrow the exception if needed.
+ Value exceptionValue;
+ if (moveExceptionBlocks.size() == 1) {
+ exceptionValue =
+ ListUtils.first(moveExceptionBlocks).getInstructions().getFirst().outValue();
+ } else {
+ Phi phi = code.createPhi(monitorExitBlock, throwableType);
+ List<Value> operands =
+ ListUtils.map(
+ moveExceptionBlocks, block -> block.getInstructions().getFirst().outValue());
+ phi.addOperands(operands);
+ exceptionValue = phi;
+ }
+
+ InstructionListIterator monitorExitBlockIterator = monitorExitBlock.listIterator(code);
+ monitorExitBlockIterator.setInsertionPosition(Position.syntheticNone());
+ monitorExitBlockIterator.add(new Throw(exceptionValue));
+ monitorExitBlock.getMutablePredecessors().addAll(moveExceptionBlocks);
+
+ // Insert the newly created blocks.
+ code.blocks.addAll(moveExceptionBlocks);
+ code.blocks.add(monitorExitBlock);
+
+ // Create a block for holding the monitor-enter instruction. Note that, since this block
+ // is created after we attach catch-all handlers to the code, this block will not have any
+ // catch handlers.
+ BasicBlock entryBlock = code.entryBlock();
+ InstructionListIterator entryBlockIterator = entryBlock.listIterator(code);
+ entryBlockIterator.nextUntil(not(Instruction::isArgument));
+ entryBlockIterator.previous();
+ BasicBlock monitorEnterBlock = entryBlockIterator.split(code, 0, null);
+ assert !monitorEnterBlock.hasCatchHandlers();
+
+ InstructionListIterator monitorEnterBlockIterator = monitorEnterBlock.listIterator(code);
+ monitorEnterBlockIterator.setInsertionPosition(Position.syntheticNone());
+
+ // If this is a static method, then the class object will act as the lock, so we load it
+ // using a const-class instruction.
+ Value lockValue;
+ if (target.isStatic()) {
+ lockValue =
+ code.createValue(
+ TypeLatticeElement.fromDexType(
+ dexItemFactory.objectType, definitelyNotNull(), appView));
+ monitorEnterBlockIterator.add(new ConstClass(lockValue, target.method.holder));
+ } else {
+ lockValue = entryBlock.getInstructions().getFirst().asArgument().outValue();
+ }
+
+ // Insert the monitor-enter and monitor-exit instructions.
+ monitorEnterBlockIterator.add(new Monitor(Monitor.Type.ENTER, lockValue));
+ monitorExitBlockIterator.previous();
+ monitorExitBlockIterator.add(new Monitor(Monitor.Type.EXIT, lockValue));
+ monitorExitBlock.close(null);
+
+ for (BasicBlock block : code.blocks) {
+ if (block.exit().isReturn()) {
+ // Since return instructions are not allowed after a throwing instruction in a block
+ // with catch handlers, the call to prepareBlocksForCatchHandlers() has already taken
+ // care of ensuring that all return blocks have no throwing instructions.
+ assert !block.canThrow();
+
+ InstructionListIterator instructionIterator =
+ block.listIterator(code, block.getInstructions().size() - 1);
+ instructionIterator.setInsertionPosition(Position.syntheticNone());
+ instructionIterator.add(new Monitor(Monitor.Type.EXIT, lockValue));
+ }
+ }
+ }
+
if (!target.isProcessed()) {
lensCodeRewriter.rewrite(code, target);
}
+ assert code.isConsistentSSA();
return new InlineeWithReason(code, reason);
}
}
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 074160b..f1256d0 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
@@ -208,8 +208,7 @@
}
public ConstraintWithTarget forMonitor() {
- // Conservative choice.
- return ConstraintWithTarget.NEVER;
+ return ConstraintWithTarget.ALWAYS;
}
public ConstraintWithTarget forMove() {
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 f836195..809db6e 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
@@ -494,6 +494,13 @@
continue;
}
+ if (user.isMonitor()) {
+ // Since this instance never escapes and is guaranteed to be non-null, any monitor
+ // instructions are no-ops.
+ removeInstruction(user);
+ continue;
+ }
+
throw new Unreachable(
"Unexpected usage left in method `"
+ method.method.toSourceString()
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/inliner/NopWhyAreYouNotInliningReporter.java b/src/main/java/com/android/tools/r8/ir/optimize/inliner/NopWhyAreYouNotInliningReporter.java
index b5c96d4..815bc20 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/inliner/NopWhyAreYouNotInliningReporter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/inliner/NopWhyAreYouNotInliningReporter.java
@@ -104,9 +104,6 @@
public void reportRecursiveMethod() {}
@Override
- public void reportSynchronizedMethod() {}
-
- @Override
public void reportUnknownTarget() {}
@Override
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporter.java b/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporter.java
index 1e869eb..12f6a5a 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporter.java
@@ -101,8 +101,6 @@
public abstract void reportRecursiveMethod();
- public abstract void reportSynchronizedMethod();
-
abstract void reportUnknownTarget();
public abstract void reportUnsafeConstructorInliningDueToFinalFieldAssignment(
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporterImpl.java b/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporterImpl.java
index dcc122b..f64622c 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporterImpl.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/inliner/WhyAreYouNotInliningReporterImpl.java
@@ -210,11 +210,6 @@
}
@Override
- public void reportSynchronizedMethod() {
- print("synchronized methods are not inlined.");
- }
-
- @Override
public void reportUnknownTarget() {
print("could not find a single target.");
}
diff --git a/src/main/java/com/android/tools/r8/utils/ListUtils.java b/src/main/java/com/android/tools/r8/utils/ListUtils.java
index 370d5e2..79f4f6c 100644
--- a/src/main/java/com/android/tools/r8/utils/ListUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ListUtils.java
@@ -12,6 +12,15 @@
public class ListUtils {
+ public static <T> T first(List<T> list) {
+ return list.get(0);
+ }
+
+ public static <T> T last(List<T> list) {
+ assert list instanceof ArrayList;
+ return list.get(list.size() - 1);
+ }
+
public static <T> int lastIndexMatching(List<T> list, Predicate<T> tester) {
for (int i = list.size() - 1; i >= 0; i--) {
if (tester.test(list.get(i))) {
diff --git a/src/test/examples/getmembers/B.java b/src/test/examples/getmembers/B.java
index d29d0a2..1f41948 100644
--- a/src/test/examples/getmembers/B.java
+++ b/src/test/examples/getmembers/B.java
@@ -15,9 +15,9 @@
return bazResult;
}
- synchronized static String inliner() throws Exception {
+ @NeverInline
+ static String inliner() throws Exception {
B self = new B();
return self.toBeInlined();
}
-
}
diff --git a/src/test/examples/getmembers/NeverInline.java b/src/test/examples/getmembers/NeverInline.java
new file mode 100644
index 0000000..0c5a4ef
--- /dev/null
+++ b/src/test/examples/getmembers/NeverInline.java
@@ -0,0 +1,10 @@
+// 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 getmembers;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+@Target({ElementType.METHOD})
+public @interface NeverInline {}
diff --git a/src/test/examples/getmembers/keep-rules.txt b/src/test/examples/getmembers/keep-rules.txt
index 9319c68..9fbea4d 100644
--- a/src/test/examples/getmembers/keep-rules.txt
+++ b/src/test/examples/getmembers/keep-rules.txt
@@ -19,3 +19,7 @@
java.lang.reflect.Method getMethod(java.lang.String, java.lang.Class[]);
java.lang.reflect.Method getDeclaredMethod(java.lang.String, java.lang.Class[]);
}
+
+-neverinline class * {
+ @getmembers.NeverInline <methods>;
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
index e60900c..fbcc6a1 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
@@ -378,6 +378,7 @@
.addKeepAttributes("LineNumberTable")
.addOptionsModification(this::configure)
.allowAccessModification()
+ .enableInliningAnnotations()
.noMinification()
.run(main)
.assertSuccessWithOutput(javaOutput);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/lambdas/LambdasTestClass.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/lambdas/LambdasTestClass.java
index 130f895..566d3f9 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/lambdas/LambdasTestClass.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/lambdas/LambdasTestClass.java
@@ -4,6 +4,8 @@
package com.android.tools.r8.ir.optimize.classinliner.lambdas;
+import com.android.tools.r8.NeverInline;
+
public class LambdasTestClass {
private static int ID = 0;
@@ -39,14 +41,16 @@
return next();
}
- private synchronized void testStatelessLambda() {
+ @NeverInline
+ private void testStatelessLambda() {
IfaceUtil.act(() -> next());
IfaceUtil.act(LambdasTestClass::next);
IfaceUtil.act(LambdasTestClass::exact);
IfaceUtil.act(LambdasTestClass::almost);
}
- private synchronized void testStatefulLambda(String a, String b) {
+ @NeverInline
+ private void testStatefulLambda(String a, String b) {
IfaceUtil.act(() -> a);
IfaceUtil.act(() -> a + b);
IfaceUtil.act((a + b)::toLowerCase);
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTest.java
index dcfe291..b3a7ce8 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTest.java
@@ -5,20 +5,17 @@
package com.android.tools.r8.ir.optimize.inliner;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
-import com.android.tools.r8.graph.DexString;
+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.InstructionSubject;
-import com.android.tools.r8.utils.codeinspector.InvokeInstructionSubject;
-import com.android.tools.r8.utils.codeinspector.MethodSubject;
-import com.google.common.collect.ImmutableList;
-import java.util.Iterator;
-import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -26,71 +23,89 @@
@RunWith(Parameterized.class)
public class InlineSynchronizedTest extends TestBase {
- private static final List<String> METHOD_NAMES =
- ImmutableList.of(
- "println",
- "normalInlinedSynchronized",
- "classInlinedSynchronized",
- "normalInlinedControl",
- "classInlinedControl");
-
- @Parameterized.Parameters(name = "{1}")
- public static List<Object[]> data() {
- return buildParameters(getTestParameters().withNoneRuntime().build(), Backend.values());
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
}
- private final Backend backend;
private final TestParameters parameters;
- public InlineSynchronizedTest(TestParameters parameters, Backend backend) {
+ public InlineSynchronizedTest(TestParameters parameters) {
this.parameters = parameters;
- this.backend = backend;
}
@Test
public void test() throws Exception {
- CodeInspector codeInspector =
- testForR8(backend)
- .addProgramClasses(InlineSynchronizedTestClass.class)
- .addKeepMainRule(InlineSynchronizedTestClass.class)
- .enableInliningAnnotations()
- .noMinification()
- .compile()
- .inspector();
+ testForR8(parameters.getBackend())
+ .addInnerClasses(InlineSynchronizedTest.class)
+ .addKeepMainRule(TestClass.class)
+ .enableClassInliningAnnotations()
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject testClassSubject = inspector.clazz(TestClass.class);
+ assertThat(testClassSubject, isPresent());
- ClassSubject classSubject = codeInspector.clazz(InlineSynchronizedTestClass.class);
- assertThat(classSubject, isPresent());
- MethodSubject methodSubject = classSubject.mainMethod();
- Iterator<InstructionSubject> it = methodSubject.iterateInstructions();
- int[] counts = new int[METHOD_NAMES.size()];
- while (it.hasNext()) {
- InstructionSubject instruction = it.next();
- if (!instruction.isInvoke()) {
- continue;
- }
- DexString invokedName = ((InvokeInstructionSubject) instruction).invokedMethod().name;
- int idx = METHOD_NAMES.indexOf(invokedName.toString());
- if (idx >= 0) {
- ++counts[idx];
- }
- }
- // Synchronized methods can never be inlined.
- assertCount(counts, "normalInlinedSynchronized", 1);
- assertCount(counts, "classInlinedSynchronized", 1);
- // Only the never-merge method is inlined, classInlining should run on the kept class.
- assertCount(counts, "normalInlinedControl", 0);
- assertCount(counts, "classInlinedControl", 1);
- // Double check the total.
- int total = 0;
- for (int i = 0; i < counts.length; ++i) {
- total += counts[i];
- }
- assertEquals(4, total);
+ // Check that there is a single monitor-enter instruction (for the normal inlining
+ // case).
+ assertEquals(
+ 1,
+ testClassSubject
+ .mainMethod()
+ .streamInstructions()
+ .filter(InstructionSubject::isMonitorEnter)
+ .count());
+
+ // Check that there are two monitor-exit instructions (for the normal inlining case,
+ // one for the normal exit and one for the exceptional exit).
+ assertEquals(
+ 2,
+ testClassSubject
+ .mainMethod()
+ .streamInstructions()
+ .filter(InstructionSubject::isMonitorExit)
+ .count());
+
+ ClassSubject aClassSubject = inspector.clazz(A.class);
+ assertThat(aClassSubject, isPresent());
+ assertThat(
+ aClassSubject.uniqueMethodWithName("normalInlinedSynchronized"),
+ not(isPresent()));
+
+ ClassSubject bClassSubject = inspector.clazz(B.class);
+ assertThat(bClassSubject, not(isPresent()));
+ })
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines(
+ "A::normalInlinedSynchronized", "B::classInlinedSynchronized");
}
- private static void assertCount(int counts[], String methodName, int expectedCount) {
- int idx = METHOD_NAMES.indexOf(methodName);
- assert idx >= 0;
- assertEquals(expectedCount, counts[idx]);
+ static class TestClass {
+
+ public static void main(String[] args) {
+ // Test normal inlining.
+ new A().normalInlinedSynchronized();
+
+ // Test class-inlining.
+ new B().classInlinedSynchronized();
+ }
+ }
+
+ @NeverClassInline
+ static class A {
+
+ synchronized void normalInlinedSynchronized() {
+ System.out.println("A::normalInlinedSynchronized");
+ }
+ }
+
+ static class B {
+
+ @NeverInline
+ synchronized void classInlinedSynchronized() {
+ System.out.println("B::classInlinedSynchronized");
+ }
}
}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTestClass.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTestClass.java
deleted file mode 100644
index 45d3a5c..0000000
--- a/src/test/java/com/android/tools/r8/ir/optimize/inliner/InlineSynchronizedTestClass.java
+++ /dev/null
@@ -1,37 +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.ir.optimize.inliner;
-
-import com.android.tools.r8.NeverInline;
-
-class InlineSynchronizedTestClass {
- private synchronized void normalInlinedSynchronized() {
- System.out.println("InlineSynchronizedTestClass::normalInlinedSynchronized");
- }
-
- public synchronized void classInlinedSynchronized() {
- System.out.println("InlineSynchronizedTestClass::classInlinedSynchronized");
- }
-
- private void normalInlinedControl() {
- System.out.println("InlineSynchronizedTestClass::normalInlinedControl");
- }
-
- @NeverInline
- public void classInlinedControl() {
- System.out.println("InlineSynchronizedTestClass::classInlinedControl");
- }
-
- public static void main(String[] args) {
- // Test normal inlining.
- InlineSynchronizedTestClass testClass = new InlineSynchronizedTestClass();
- testClass.normalInlinedSynchronized();
- testClass.normalInlinedControl();
-
- // Test class-inlining.
- new InlineSynchronizedTestClass().classInlinedSynchronized();
- new InlineSynchronizedTestClass().classInlinedControl();
- }
-}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/inliner/sync/InlineStaticSynchronizedMethodTest.java b/src/test/java/com/android/tools/r8/ir/optimize/inliner/sync/InlineStaticSynchronizedMethodTest.java
new file mode 100644
index 0000000..a8578e3
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/inliner/sync/InlineStaticSynchronizedMethodTest.java
@@ -0,0 +1,133 @@
+// 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.ir.optimize.inliner.sync;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ir.optimize.inliner.sync.InlineStaticSynchronizedMethodTest.TestClass.RunnableImpl;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import java.lang.Thread.State;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+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 InlineStaticSynchronizedMethodTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().build();
+ }
+
+ public InlineStaticSynchronizedMethodTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(InlineStaticSynchronizedMethodTest.class)
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(this::verifySynchronizedMethodsAreInlined)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines(
+ "t1 START", "m1 ENTER", "t2 START", "m1 EXIT", "m2 ENTER", "m2 EXIT");
+ }
+
+ private void verifySynchronizedMethodsAreInlined(CodeInspector inspector) {
+ ClassSubject classSubject = inspector.clazz(RunnableImpl.class);
+ assertThat(classSubject, isPresent());
+ assertThat(classSubject.uniqueMethodWithName("m1"), not(isPresent()));
+ assertThat(classSubject.uniqueMethodWithName("m2"), not(isPresent()));
+ }
+
+ static class TestClass {
+
+ private static List<String> logs = Collections.synchronizedList(new ArrayList<>());
+
+ private static volatile Thread t1 = new Thread(new RunnableImpl(1));
+ private static volatile Thread t2 = new Thread(new RunnableImpl(2));
+
+ public static void main(String[] args) {
+ System.out.println("t1 START");
+ t1.start();
+ waitUntil(() -> logs.contains("m1 ENTER"));
+ System.out.println("t2 START");
+ t2.start();
+ }
+
+ static void log(String message) {
+ logs.add(message);
+ System.out.println(message);
+ }
+
+ static void waitUntil(BooleanSupplier condition) {
+ while (!condition.getAsBoolean()) {
+ try {
+ Thread.sleep(50);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+
+ static class RunnableImpl implements Runnable {
+
+ int which;
+
+ RunnableImpl(int which) {
+ this.which = which;
+ }
+
+ @Override
+ public void run() {
+ if (which == 1) {
+ m1();
+ } else {
+ m2();
+ }
+ }
+
+ static synchronized void m1() {
+ log("m1 ENTER");
+ // Intentionally not using a lambda, since we do not allow inlining of methods with an
+ // invoke-custom instruction. (This only makes a difference for the CF backend.)
+ waitUntil(
+ new BooleanSupplier() {
+
+ @Override
+ public boolean getAsBoolean() {
+ return t2.getState() == State.BLOCKED;
+ }
+ });
+ log("m1 EXIT");
+ }
+
+ static synchronized void m2() {
+ log("m2 ENTER");
+ log("m2 EXIT");
+ }
+ }
+ }
+
+ interface BooleanSupplier {
+
+ boolean getAsBoolean();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/naming/IdentifierMinifierTest.java b/src/test/java/com/android/tools/r8/naming/IdentifierMinifierTest.java
index 8f4aca9..2cd766b 100644
--- a/src/test/java/com/android/tools/r8/naming/IdentifierMinifierTest.java
+++ b/src/test/java/com/android/tools/r8/naming/IdentifierMinifierTest.java
@@ -65,8 +65,11 @@
testForR8(parameters.getBackend())
.addProgramFiles(Paths.get(appFileName))
.addKeepRuleFiles(ListUtils.map(keepRulesFiles, Paths::get))
+ .allowUnusedProguardConfigurationRules()
+ .enableProguardTestOptions()
.setMinApi(parameters.getRuntime())
- .compile().inspector();
+ .compile()
+ .inspector();
inspection.accept(parameters, codeInspector);
}
diff --git a/src/test/java/com/android/tools/r8/shaking/annotations/b137392797/B137392797.java b/src/test/java/com/android/tools/r8/shaking/annotations/b137392797/B137392797.java
index 8585a9c..9e92530 100644
--- a/src/test/java/com/android/tools/r8/shaking/annotations/b137392797/B137392797.java
+++ b/src/test/java/com/android/tools/r8/shaking/annotations/b137392797/B137392797.java
@@ -31,10 +31,10 @@
private final TestParameters parameters;
private final boolean defaultEnumValueInAnnotation;
- @Parameterized.Parameters(name = "Backend: {0}, default value in annotation: {1}")
+ @Parameterized.Parameters(name = "{0}, default value in annotation: {1}")
public static Collection<Object[]> data() {
return buildParameters(
- getTestParameters().withAllRuntimes().withAllApiLevels().build(), BooleanUtils.values());
+ getTestParameters().withAllRuntimesAndApiLevels().build(), BooleanUtils.values());
}
public B137392797(TestParameters parameters, boolean defaultEnumValueInAnnotation) {
diff --git a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatibilityTestBase.java b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatibilityTestBase.java
index ab0fc8b..d8afbb3 100644
--- a/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatibilityTestBase.java
+++ b/src/test/java/com/android/tools/r8/shaking/forceproguardcompatibility/ProguardCompatibilityTestBase.java
@@ -50,6 +50,10 @@
|| this == R8_CF;
}
+ public boolean isProguard() {
+ return this == PROGUARD5 || this == PROGUARD6 || this == PROGUARD6_THEN_D8;
+ }
+
public boolean isFullModeR8() {
return this == R8
|| this == R8_CF;
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 2b6c5db..02c722a 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
@@ -9,11 +9,15 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeFalse;
-import com.android.tools.r8.naming.MemberNaming.MethodSignature;
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestShrinkerBuilder;
+import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.shaking.forceproguardcompatibility.ProguardCompatibilityTestBase;
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 com.google.common.collect.ImmutableList;
import java.util.Collection;
@@ -25,180 +29,231 @@
@RunWith(Parameterized.class)
public class IfOnAccessModifierTest extends ProguardCompatibilityTestBase {
+
private static final List<Class<?>> CLASSES =
- ImmutableList.of(ClassForIf.class, ClassForSubsequent.class, MainForAccessModifierTest.class);
+ ImmutableList.of(
+ ClassForIf.class,
+ ClassForSubsequent.class,
+ MainForAccessModifierTest.class,
+ NeverClassInline.class,
+ NeverInline.class);
+ private final TestParameters parameters;
private final Shrinker shrinker;
- private final MethodSignature nonPublicMethod;
- private final MethodSignature publicMethod;
- public IfOnAccessModifierTest(Shrinker shrinker) {
+ public IfOnAccessModifierTest(TestParameters parameters, Shrinker shrinker) {
+ this.parameters = parameters;
this.shrinker = shrinker;
- nonPublicMethod = new MethodSignature("nonPublicMethod", "void", ImmutableList.of());
- publicMethod = new MethodSignature("publicMethod", "void", ImmutableList.of());
}
- @Parameters(name = "shrinker: {0}")
- public static Collection<Object> data() {
- return ImmutableList.of(Shrinker.R8_CF, Shrinker.PROGUARD6, Shrinker.R8);
+ @Parameters(name = "{0}, shrinker: {1}")
+ public static Collection<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withAllRuntimes().build(),
+ ImmutableList.of(Shrinker.PROGUARD6, Shrinker.R8));
+ }
+
+ private TestShrinkerBuilder<?, ?, ?, ?, ?> getTestBuilder() {
+ switch (shrinker) {
+ case PROGUARD6:
+ assertTrue(parameters.isCfRuntime());
+ return testForProguard();
+ case R8:
+ return testForR8(parameters.getBackend())
+ .allowUnusedProguardConfigurationRules()
+ .enableClassInliningAnnotations()
+ .enableInliningAnnotations();
+ default:
+ throw new Unreachable();
+ }
}
@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());
+ assumeFalse(shrinker.isProguard() && parameters.isDexRuntime());
- classSubject = codeInspector.clazz(ClassForSubsequent.class);
- assertThat(classSubject, not(isPresent()));
+ getTestBuilder()
+ .addProgramClasses(CLASSES)
+ .addKeepRules(
+ "-repackageclasses 'top'",
+ "-keep class **.Main* {",
+ " public static void callIfNonPublic();",
+ "}",
+ "-if public class **.ClassForIf {",
+ " <methods>;",
+ "}",
+ "-keep,allowobfuscation class **.ClassForSubsequent {",
+ " public <methods>;",
+ "}")
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject classSubject = inspector.clazz(ClassForIf.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.uniqueMethodWithName("publicMethod");
+ assertThat(methodSubject, not(isPresent()));
+ methodSubject = classSubject.uniqueMethodWithName("nonPublicMethod");
+ assertThat(methodSubject, isPresent());
+ assertFalse(methodSubject.getMethod().accessFlags.isPublic());
+ classSubject = inspector.clazz(ClassForSubsequent.class);
+ assertThat(classSubject, not(isPresent()));
+ });
}
@Test
public void ifOnNonPublic_keepOnPublic() throws Exception {
- List<String> config = ImmutableList.of(
- "-printmapping",
- "-repackageclasses 'top'",
- "-allowaccessmodification",
- "-keep class **.Main* {",
- " public static void callIfNonPublic();",
- "}",
- "-if class **.ClassForIf {",
- " !public <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());
- assertTrue(methodSubject.getMethod().accessFlags.isPublic());
+ assumeFalse(shrinker.isProguard() && parameters.isDexRuntime());
- classSubject = codeInspector.clazz(ClassForSubsequent.class);
- assertThat(classSubject, isPresent());
- methodSubject = classSubject.method(publicMethod);
- assertThat(methodSubject, isPresent());
- methodSubject = classSubject.method(nonPublicMethod);
- assertThat(methodSubject, not(isPresent()));
+ getTestBuilder()
+ .addProgramClasses(CLASSES)
+ .addKeepRules(
+ "-repackageclasses 'top'",
+ "-allowaccessmodification",
+ "-keep class **.Main* {",
+ " public static void callIfNonPublic();",
+ "}",
+ "-if class **.ClassForIf {",
+ " !public <methods>;",
+ "}",
+ "-keep,allowobfuscation class **.ClassForSubsequent {",
+ " public <methods>;",
+ "}")
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject classSubject = inspector.clazz(ClassForIf.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.uniqueMethodWithName("publicMethod");
+ assertThat(methodSubject, not(isPresent()));
+ methodSubject = classSubject.uniqueMethodWithName("nonPublicMethod");
+ assertThat(methodSubject, isPresent());
+ assertTrue(methodSubject.getMethod().accessFlags.isPublic());
+
+ classSubject = inspector.clazz(ClassForSubsequent.class);
+ assertThat(classSubject, isPresent());
+ methodSubject = classSubject.uniqueMethodWithName("publicMethod");
+ assertThat(methodSubject, isPresent());
+ methodSubject = classSubject.uniqueMethodWithName("nonPublicMethod");
+ assertThat(methodSubject, not(isPresent()));
+ });
}
@Test
public void ifOnNonPublic_keepOnNonPublic() throws Exception {
- List<String> config = ImmutableList.of(
- "-printmapping",
- "-repackageclasses 'top'",
- "-allowaccessmodification",
- "-keep class **.Main* {",
- " public static void callIfNonPublic();",
- "}",
- "-if class **.ClassForIf {",
- " !public <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());
- assertTrue(methodSubject.getMethod().accessFlags.isPublic());
+ assumeFalse(shrinker.isProguard() && parameters.isDexRuntime());
- classSubject = codeInspector.clazz(ClassForSubsequent.class);
- assertThat(classSubject, isPresent());
- methodSubject = classSubject.method(publicMethod);
- assertThat(methodSubject, not(isPresent()));
- methodSubject = classSubject.method(nonPublicMethod);
- assertThat(methodSubject, isPresent());
- assertEquals(shrinker.isR8(), methodSubject.getMethod().accessFlags.isPublic());
+ getTestBuilder()
+ .addProgramClasses(CLASSES)
+ .addKeepRules(
+ "-repackageclasses 'top'",
+ "-allowaccessmodification",
+ "-keep class **.Main* {",
+ " public static void callIfNonPublic();",
+ "}",
+ "-if class **.ClassForIf {",
+ " !public <methods>;",
+ "}",
+ "-keep,allowobfuscation class **.ClassForSubsequent {",
+ " !public <methods>;",
+ "}")
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject classSubject = inspector.clazz(ClassForIf.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.uniqueMethodWithName("publicMethod");
+ assertThat(methodSubject, not(isPresent()));
+ methodSubject = classSubject.uniqueMethodWithName("nonPublicMethod");
+ assertThat(methodSubject, isPresent());
+ assertTrue(methodSubject.getMethod().accessFlags.isPublic());
+
+ classSubject = inspector.clazz(ClassForSubsequent.class);
+ assertThat(classSubject, isPresent());
+ methodSubject = classSubject.uniqueMethodWithName("publicMethod");
+ assertThat(methodSubject, not(isPresent()));
+ methodSubject = classSubject.uniqueMethodWithName("nonPublicMethod");
+ assertThat(methodSubject, isPresent());
+ assertEquals(shrinker.isR8(), methodSubject.getMethod().accessFlags.isPublic());
+ });
}
@Test
public void ifOnPublic_keepOnPublic() throws Exception {
- List<String> config = ImmutableList.of(
- "-printmapping",
- "-repackageclasses 'top'",
- "-allowaccessmodification",
- "-keep class **.Main* {",
- " public static void callIfPublic();",
- "}",
- "-if class **.ClassForIf {",
- " public <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, isPresent());
- methodSubject = classSubject.method(nonPublicMethod);
- assertThat(methodSubject, not(isPresent()));
+ assumeFalse(shrinker.isProguard() && parameters.isDexRuntime());
- classSubject = codeInspector.clazz(ClassForSubsequent.class);
- assertThat(classSubject, isPresent());
- methodSubject = classSubject.method(publicMethod);
- assertThat(methodSubject, isPresent());
- methodSubject = classSubject.method(nonPublicMethod);
- assertThat(methodSubject, not(isPresent()));
+ getTestBuilder()
+ .addProgramClasses(CLASSES)
+ .addKeepRules(
+ "-repackageclasses 'top'",
+ "-allowaccessmodification",
+ "-keep class **.Main* {",
+ " public static void callIfPublic();",
+ "}",
+ "-if class **.ClassForIf {",
+ " public <methods>;",
+ "}",
+ "-keep,allowobfuscation class **.ClassForSubsequent {",
+ " public <methods>;",
+ "}")
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject classSubject = inspector.clazz(ClassForIf.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.uniqueMethodWithName("publicMethod");
+ assertThat(methodSubject, isPresent());
+ methodSubject = classSubject.uniqueMethodWithName("nonPublicMethod");
+ assertThat(methodSubject, not(isPresent()));
+
+ classSubject = inspector.clazz(ClassForSubsequent.class);
+ assertThat(classSubject, isPresent());
+ methodSubject = classSubject.uniqueMethodWithName("publicMethod");
+ assertThat(methodSubject, isPresent());
+ methodSubject = classSubject.uniqueMethodWithName("nonPublicMethod");
+ assertThat(methodSubject, not(isPresent()));
+ });
}
@Test
public void ifOnPublic_keepOnNonPublic() throws Exception {
- List<String> config = ImmutableList.of(
- "-printmapping",
- "-repackageclasses 'top'",
- "-allowaccessmodification",
- "-keep class **.Main* {",
- " public static void callIfPublic();",
- "}",
- "-if class **.ClassForIf {",
- " public <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, isPresent());
- methodSubject = classSubject.method(nonPublicMethod);
- assertThat(methodSubject, not(isPresent()));
+ assumeFalse(shrinker.isProguard() && parameters.isDexRuntime());
- classSubject = codeInspector.clazz(ClassForSubsequent.class);
- assertThat(classSubject, isPresent());
- methodSubject = classSubject.method(publicMethod);
- assertThat(methodSubject, not(isPresent()));
- methodSubject = classSubject.method(nonPublicMethod);
- assertThat(methodSubject, isPresent());
- assertEquals(shrinker.isR8(), methodSubject.getMethod().accessFlags.isPublic());
+ getTestBuilder()
+ .addProgramClasses(CLASSES)
+ .addKeepRules(
+ "-repackageclasses 'top'",
+ "-allowaccessmodification",
+ "-keep class **.Main* {",
+ " public static void callIfPublic();",
+ "}",
+ "-if class **.ClassForIf {",
+ " public <methods>;",
+ "}",
+ "-keep,allowobfuscation class **.ClassForSubsequent {",
+ " !public <methods>;",
+ "}")
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .inspect(
+ inspector -> {
+ ClassSubject classSubject = inspector.clazz(ClassForIf.class);
+ assertThat(classSubject, isPresent());
+ MethodSubject methodSubject = classSubject.uniqueMethodWithName("publicMethod");
+ assertThat(methodSubject, isPresent());
+ methodSubject = classSubject.uniqueMethodWithName("nonPublicMethod");
+ assertThat(methodSubject, not(isPresent()));
+
+ classSubject = inspector.clazz(ClassForSubsequent.class);
+ assertThat(classSubject, isPresent());
+ methodSubject = classSubject.uniqueMethodWithName("publicMethod");
+ assertThat(methodSubject, not(isPresent()));
+ methodSubject = classSubject.uniqueMethodWithName("nonPublicMethod");
+ assertThat(methodSubject, isPresent());
+ assertEquals(shrinker.isR8(), methodSubject.getMethod().accessFlags.isPublic());
+ });
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTestClasses.java b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTestClasses.java
index 1b5bd75..49d75f2 100644
--- a/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTestClasses.java
+++ b/src/test/java/com/android/tools/r8/shaking/ifrule/IfOnAccessModifierTestClasses.java
@@ -3,14 +3,20 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.shaking.ifrule;
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+
+@NeverClassInline
class ClassForIf {
ClassForIf() {
}
+ @NeverInline
synchronized void nonPublicMethod() {
System.out.println("ClassForIf::nonPublicMethod");
}
+ @NeverInline
synchronized public void publicMethod() {
System.out.println("ClassForIf::publicMethod");
}
@@ -20,10 +26,12 @@
ClassForSubsequent() {
}
+ @NeverInline
synchronized void nonPublicMethod() {
System.out.println("ClassForSubsequent::nonPublicMethod");
}
+ @NeverInline
synchronized public void publicMethod() {
System.out.println("ClassForSubsequent::publicMethod");
}
