Merge "CF backend: Omit writing code for native methods"
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
index d709f49..ff37507 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfBuilder.java
@@ -196,10 +196,11 @@
labels = new HashMap<>(code.blocks.size());
emittedLabels = new HashSet<>(code.blocks.size());
instructions = new ArrayList<>();
- Iterator<BasicBlock> blockIterator = code.listIterator();
+ ListIterator<BasicBlock> blockIterator = code.listIterator();
BasicBlock block = blockIterator.next();
CfLabel tryCatchStart = null;
CatchHandlers<BasicBlock> tryCatchHandlers = CatchHandlers.EMPTY_BASIC_BLOCK;
+ BasicBlock pendingFrame = null;
do {
CatchHandlers<BasicBlock> handlers = block.getCatchHandlers();
if (!tryCatchHandlers.equals(handlers)) {
@@ -229,10 +230,26 @@
pendingLocalChanges = true;
}
buildCfInstructions(block, fallthrough, stack);
- if (nextBlock != null && (!fallthrough || nextBlock.getPredecessors().size() > 1)) {
- assert stack.isEmpty();
- emitLabel(getLabel(nextBlock));
- addFrame(nextBlock, Collections.emptyList());
+ if (nextBlock != null) {
+ if (!fallthrough || nextBlock.getPredecessors().size() > 1) {
+ assert stack.isEmpty();
+ pendingFrame = nextBlock;
+ emitLabel(getLabel(nextBlock));
+ }
+ if (pendingFrame != null) {
+ BasicBlock nextNextBlock = null;
+ if (blockIterator.hasNext()) {
+ nextNextBlock = blockIterator.next();
+ blockIterator.previous();
+ }
+ boolean advancesPC = hasMaterializingInstructions(nextBlock, nextNextBlock);
+ // If nextBlock has no materializing instructions, then nextNextBlock must be non-null
+ // (or we would fall off the edge of the method).
+ assert advancesPC || nextNextBlock != null;
+ if (advancesPC) {
+ addFrame(pendingFrame, Collections.emptyList());
+ }
+ }
}
block = nextBlock;
} while (block != null);
@@ -251,6 +268,25 @@
localVariablesTable);
}
+ private static boolean isNopInstruction(Instruction instruction, BasicBlock nextBlock) {
+ // From DexBuilder
+ return instruction.isArgument()
+ || instruction.isDebugLocalsChange()
+ || (instruction.isGoto() && instruction.asGoto().getTarget() == nextBlock);
+ }
+
+ private boolean hasMaterializingInstructions(BasicBlock block, BasicBlock nextBlock) {
+ if (block == null) {
+ return false;
+ }
+ for (Instruction instruction : block.getInstructions()) {
+ if (!isNopInstruction(instruction, nextBlock)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private void buildCfInstructions(BasicBlock block, boolean fallthrough, Stack stack) {
InstructionIterator it = block.iterator();
while (it.hasNext()) {
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
index ca62583..b526973 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -352,21 +352,9 @@
} else if (acceptString("identifiernamestring")) {
configurationBuilder.addRule(parseIdentifierNameStringRule());
} else if (acceptString("if")) {
- // TODO(b/73708139): add support -if <class_spec>
- ProguardKeepRule.Builder keepRuleBuilder = ProguardKeepRule.builder();
- parseClassSpec(keepRuleBuilder, true);
- ProguardKeepRule ifRule = keepRuleBuilder.build();
- assert ifRule.getClassType() == ProguardClassType.CLASS;
-
- // Required a subsequent keep rule.
- skipWhitespace();
- if (acceptString("-keep")) {
- ProguardKeepRule subsequentRule = parseKeepRule();
- warnIgnoringOptions("if", optionStart);
- } else {
- throw reporter.fatalError(new StringDiagnostic(
- "Option -if without a subsequent keep rule.", origin, getPosition(optionStart)));
- }
+ configurationBuilder.addRule(parseIfRule(optionStart));
+ // TODO(b/73708139): remove warning once we add support -if <class_spec>
+ warnIgnoringOptions("if", optionStart);
} else {
String unknownOption = acceptString();
reporter.error(new StringDiagnostic(
@@ -544,6 +532,22 @@
return keepRuleBuilder.build();
}
+ private ProguardIfRule parseIfRule(TextPosition optionStart)
+ throws ProguardRuleParserException {
+ ProguardIfRule.Builder ifRuleBuilder = ProguardIfRule.builder();
+ parseClassSpec(ifRuleBuilder, true);
+
+ // Required a subsequent keep rule.
+ skipWhitespace();
+ if (acceptString("-keep")) {
+ ProguardKeepRule subsequentRule = parseKeepRule();
+ ifRuleBuilder.setSubsequentRule(subsequentRule);
+ return ifRuleBuilder.build();
+ }
+ throw reporter.fatalError(new StringDiagnostic(
+ "Option -if without a subsequent keep rule.", origin, getPosition(optionStart)));
+ }
+
private void parseClassSpec(
ProguardConfigurationRule.Builder builder, boolean allowValueSpecification)
throws ProguardRuleParserException {
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java b/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
new file mode 100644
index 0000000..6facec1
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardIfRule.java
@@ -0,0 +1,52 @@
+// Copyright (c) 2018, 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;
+
+import java.util.Set;
+
+public class ProguardIfRule extends ProguardConfigurationRule {
+
+ final ProguardKeepRule subsequentRule;
+
+ public static class Builder extends ProguardConfigurationRule.Builder {
+
+ ProguardKeepRule subsequentRule = null;
+
+ private Builder() {
+ }
+
+ public void setSubsequentRule(ProguardKeepRule rule) {
+ subsequentRule = rule;
+ }
+
+ public ProguardIfRule build() {
+ assert subsequentRule != null : "Option -if without a subsequent rule.";
+ return new ProguardIfRule(classAnnotation, classAccessFlags,
+ negatedClassAccessFlags, classTypeNegated, classType, classNames, inheritanceAnnotation,
+ inheritanceClassName, inheritanceIsExtends, memberRules, subsequentRule);
+ }
+ }
+
+ private ProguardIfRule(ProguardTypeMatcher classAnnotation,
+ ProguardAccessFlags classAccessFlags,
+ ProguardAccessFlags negatedClassAccessFlags, boolean classTypeNegated,
+ ProguardClassType classType, ProguardClassNameList classNames,
+ ProguardTypeMatcher inheritanceAnnotation,
+ ProguardTypeMatcher inheritanceClassName, boolean inheritanceIsExtends,
+ Set<ProguardMemberRule> memberRules,
+ ProguardKeepRule subsequentRule) {
+ super(classAnnotation, classAccessFlags, negatedClassAccessFlags, classTypeNegated, classType,
+ classNames, inheritanceAnnotation, inheritanceClassName, inheritanceIsExtends, memberRules);
+ this.subsequentRule = subsequentRule;
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ @Override
+ String typeString() {
+ return "if";
+ }
+}
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 f504930..9c0dcdd 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -167,7 +167,10 @@
}
}
- if (rule.getClassNames().matches(clazz.type)) {
+ if (rule instanceof ProguardIfRule) {
+ // TODO(b/73708139): add support -if <class_spec>
+ // Check if -if part matches or subsequent -keep part matches.
+ } else if (rule.getClassNames().matches(clazz.type)) {
Collection<ProguardMemberRule> memberKeepRules = rule.getMemberRules();
if (rule instanceof ProguardKeepRule) {
switch (((ProguardKeepRule) rule).getType()) {
diff --git a/src/test/java/com/android/tools/r8/cf/NestedExceptionTest.java b/src/test/java/com/android/tools/r8/cf/NestedExceptionTest.java
new file mode 100644
index 0000000..f7823a9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/NestedExceptionTest.java
@@ -0,0 +1,19 @@
+// Copyright (c) 2018, 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.cf;
+
+public class NestedExceptionTest {
+
+ public static void main(String[] args) {
+ try {
+ new Integer(1);
+ } catch (Exception t) {
+ try {
+ new Integer(1);
+ } catch (Exception tInner) {
+ // This block must be empty
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/cf/NestedExceptionTestRunner.java b/src/test/java/com/android/tools/r8/cf/NestedExceptionTestRunner.java
new file mode 100644
index 0000000..4c7effd
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/cf/NestedExceptionTestRunner.java
@@ -0,0 +1,36 @@
+// Copyright (c) 2018, 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.cf;
+
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.OutputMode;
+import com.android.tools.r8.R8;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidAppConsumers;
+import java.nio.file.Path;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class NestedExceptionTestRunner {
+ static final Class CLASS = NestedExceptionTest.class;
+ @Rule public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest();
+
+ @Test
+ public void testNestedException() throws Exception {
+ AndroidAppConsumers a = new AndroidAppConsumers();
+ R8.run(
+ R8Command.builder()
+ .setMode(CompilationMode.DEBUG)
+ .addClassProgramData(ToolHelper.getClassAsBytes(CLASS), Origin.unknown())
+ .addLibraryFiles(ToolHelper.getAndroidJar(ToolHelper.getMinApiLevelForDexVm()))
+ .setProgramConsumer(a.wrapClassFileConsumer(null))
+ .build());
+ Path out = temp.newFolder().toPath();
+ a.build().writeToDirectory(out, OutputMode.ClassFile);
+ ToolHelper.runJava(out, CLASS.getCanonicalName());
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
index 00fa060..5f6a5cc 100644
--- a/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
@@ -1152,6 +1152,24 @@
for (int i = 0; i < 3; i++) {
assertTrue(handler.warnings.get(i).getDiagnosticMessage().contains("Ignoring option: -if"));
}
+ ProguardConfiguration config = parser.getConfig();
+ // Three -if rules and one independent -keepnames
+ assertEquals(4, config.getRules().size());
+ long ifCount =
+ config.getRules().stream().filter(rule -> rule instanceof ProguardIfRule).count();
+ assertEquals(3, ifCount);
+ ProguardIfRule if0 = (ProguardIfRule) config.getRules().get(0);
+ assertEquals("**$$ModuleAdapter", if0.getClassNames().toString());
+ assertEquals(ProguardKeepRuleType.KEEP, if0.subsequentRule.getType());
+ assertEquals("A", if0.subsequentRule.getClassNames().toString());
+ ProguardIfRule if1 = (ProguardIfRule) config.getRules().get(1);
+ assertEquals("**$$InjectAdapter", if1.getClassNames().toString());
+ assertEquals(ProguardKeepRuleType.KEEP, if1.subsequentRule.getType());
+ assertEquals("B", if1.subsequentRule.getClassNames().toString());
+ ProguardIfRule if2 = (ProguardIfRule) config.getRules().get(2);
+ assertEquals("**$$StaticInjection", if2.getClassNames().toString());
+ assertEquals(ProguardKeepRuleType.KEEP, if2.subsequentRule.getType());
+ assertEquals("C", if2.subsequentRule.getClassNames().toString());
}
@Test
