Handle StringSwitch with class name value
Bug: 135560746
Change-Id: I26f6512701a5ba55fe276f9286624afb190d5cfd
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 9964c7f..dd8228c 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -201,14 +201,11 @@
: null;
this.stringOptimizer = new StringOptimizer(appView);
this.stringBuilderOptimizer = new StringBuilderOptimizer(appView);
- this.stringSwitchRemover =
- options.isStringSwitchConversionEnabled() ? new StringSwitchRemover(appView) : null;
this.nonNullTracker = options.enableNonNullTracking ? new NonNullTracker(appView) : null;
if (appView.enableWholeProgramOptimizations()) {
assert appView.appInfo().hasLiveness();
assert appView.rootSet() != null;
AppView<AppInfoWithLiveness> appViewWithLiveness = appView.withLiveness();
- AppInfoWithLiveness appInfoWithLiveness = appViewWithLiveness.appInfo();
this.classInliner =
options.enableClassInlining && options.enableInlining
? new ClassInliner(lambdaRewriter)
@@ -259,6 +256,10 @@
}
this.deadCodeRemover = new DeadCodeRemover(appView, codeRewriter);
this.idempotentFunctionCallCanonicalizer = new IdempotentFunctionCallCanonicalizer(appView);
+ this.stringSwitchRemover =
+ options.isStringSwitchConversionEnabled()
+ ? new StringSwitchRemover(appView, identifierNameStringMarker)
+ : null;
}
public Set<DexCallSite> getDesugaredCallSites() {
@@ -1054,8 +1055,7 @@
codeRewriter.splitRangeInvokeConstants(code);
new SparseConditionalConstantPropagation(code).run();
if (stringSwitchRemover != null) {
- // TODO(b/135588279): Add support for string-switch instruction in the CF and DEX backend.
- stringSwitchRemover.run(code);
+ stringSwitchRemover.run(method, code);
}
codeRewriter.rewriteSwitch(code);
codeRewriter.processMethodsNeverReturningNormally(code);
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/StringSwitchRemover.java b/src/main/java/com/android/tools/r8/ir/conversion/StringSwitchRemover.java
index 6507f22..b6a2ac3 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/StringSwitchRemover.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/StringSwitchRemover.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.ir.conversion;
import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.ir.analysis.type.ClassTypeLatticeElement;
import com.android.tools.r8.ir.analysis.type.Nullability;
@@ -21,41 +22,51 @@
import com.android.tools.r8.ir.code.JumpInstruction;
import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.StringSwitch;
+import com.android.tools.r8.naming.IdentifierNameStringMarker;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import java.util.IdentityHashMap;
import java.util.ListIterator;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Set;
-// TODO(b/135588279): Add support for string-switch instruction in the CF and DEX backend.
public class StringSwitchRemover {
private final AppView<?> appView;
+ private final IdentifierNameStringMarker identifierNameStringMarker;
private final ClassTypeLatticeElement stringType;
private final ThrowingInfo throwingInfo;
- StringSwitchRemover(AppView<?> appView) {
+ StringSwitchRemover(AppView<?> appView, IdentifierNameStringMarker identifierNameStringMarker) {
this.appView = appView;
+ this.identifierNameStringMarker = identifierNameStringMarker;
this.stringType = TypeLatticeElement.stringClassType(appView, Nullability.definitelyNotNull());
this.throwingInfo = ThrowingInfo.defaultForConstString(appView.options());
}
- void run(IRCode code) {
+ void run(DexEncodedMethod method, IRCode code) {
if (!code.hasStringSwitch) {
assert Streams.stream(code.instructions()).noneMatch(Instruction::isStringSwitch);
return;
}
+ Set<BasicBlock> newBlocks = Sets.newIdentityHashSet();
+
ListIterator<BasicBlock> blockIterator = code.listIterator();
while (blockIterator.hasNext()) {
BasicBlock block = blockIterator.next();
JumpInstruction exit = block.exit();
if (exit.isStringSwitch()) {
- removeStringSwitch(code, blockIterator, block, exit.asStringSwitch());
+ removeStringSwitch(code, blockIterator, block, exit.asStringSwitch(), newBlocks);
}
}
+ if (identifierNameStringMarker != null) {
+ identifierNameStringMarker.decoupleIdentifierNameStringsInBlocks(method, code, newBlocks);
+ }
+
assert code.isConsistentSSA();
}
@@ -63,7 +74,8 @@
IRCode code,
ListIterator<BasicBlock> blockIterator,
BasicBlock block,
- StringSwitch theSwitch) {
+ StringSwitch theSwitch,
+ Set<BasicBlock> newBlocks) {
BasicBlock fallthroughBlock = theSwitch.fallthroughBlock();
Map<DexString, BasicBlock> stringToTargetMap = new IdentityHashMap<>();
theSwitch.forEachCase(stringToTargetMap::put);
@@ -96,6 +108,7 @@
code.blocks.size(), ifInstruction, constStringInstruction, invokeInstruction);
newBlock.link(entry.getValue());
blockIterator.add(newBlock);
+ newBlocks.add(newBlock);
if (previous == null) {
// Replace the string-switch instruction by a goto instruction.
diff --git a/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java b/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
index 87f4c10..34ce2c4 100644
--- a/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
+++ b/src/main/java/com/android/tools/r8/naming/IdentifierNameStringMarker.java
@@ -45,6 +45,7 @@
import java.util.List;
import java.util.ListIterator;
import java.util.Objects;
+import java.util.Set;
import java.util.stream.Collectors;
public class IdentifierNameStringMarker {
@@ -83,13 +84,21 @@
}
}
- public void decoupleIdentifierNameStringsInMethod(DexEncodedMethod encodedMethod, IRCode code) {
+ public void decoupleIdentifierNameStringsInMethod(DexEncodedMethod method, IRCode code) {
+ decoupleIdentifierNameStringsInBlocks(method, code, null);
+ }
+
+ public void decoupleIdentifierNameStringsInBlocks(
+ DexEncodedMethod method, IRCode code, Set<BasicBlock> blocks) {
if (!code.hasConstString) {
return;
}
- ListIterator<BasicBlock> blocks = code.listIterator();
- while (blocks.hasNext()) {
- BasicBlock block = blocks.next();
+ ListIterator<BasicBlock> blockIterator = code.listIterator();
+ while (blockIterator.hasNext()) {
+ BasicBlock block = blockIterator.next();
+ if (blocks != null && !blocks.contains(block)) {
+ continue;
+ }
InstructionListIterator iterator = block.listIterator();
while (iterator.hasNext()) {
Instruction instruction = iterator.next();
@@ -107,11 +116,11 @@
if (instruction.isStaticPut() || instruction.isInstancePut()) {
iterator =
decoupleIdentifierNameStringForFieldPutInstruction(
- code, encodedMethod, blocks, iterator, instruction.asFieldInstruction());
+ code, method, blockIterator, iterator, instruction.asFieldInstruction());
} else if (instruction.isInvokeMethod()) {
iterator =
decoupleIdentifierNameStringForInvokeInstruction(
- code, encodedMethod, blocks, iterator, instruction.asInvokeMethod());
+ code, method, blockIterator, iterator, instruction.asInvokeMethod());
}
}
}
diff --git a/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java b/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java
index 565c3e0..1f27829 100644
--- a/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java
+++ b/src/test/java/com/android/tools/r8/internal/proto/Proto2ShrinkingTest.java
@@ -33,8 +33,6 @@
.addProgramFiles(PROTO2_EXAMPLES_JAR, PROTO2_PROTO_JAR, PROTOBUF_LITE_JAR)
.addKeepMainRule("proto2.TestClass")
.addKeepRules(
- // TODO(b/112437944): Fix -identifiernamestring support.
- "-keepnames class * extends com.google.protobuf.GeneratedMessageLite",
// TODO(b/112437944): Use dex item based const strings for proto schema definitions.
"-keepclassmembernames class * extends com.google.protobuf.GeneratedMessageLite {",
" <fields>;",
diff --git a/src/test/java/com/android/tools/r8/naming/identifiernamestring/ClassNameComparisonSwitchTest.java b/src/test/java/com/android/tools/r8/naming/identifiernamestring/ClassNameComparisonSwitchTest.java
new file mode 100644
index 0000000..38f1534
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/identifiernamestring/ClassNameComparisonSwitchTest.java
@@ -0,0 +1,102 @@
+// 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.naming.identifiernamestring;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+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.google.common.collect.ImmutableList;
+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 ClassNameComparisonSwitchTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().build();
+ }
+
+ public ClassNameComparisonSwitchTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(ClassNameComparisonSwitchTest.class)
+ .addKeepMainRule(TestClass.class)
+ .addOptionsModification(
+ options -> {
+ assert !options.enableStringSwitchConversion;
+ options.enableStringSwitchConversion = true;
+ })
+ .enableInliningAnnotations()
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .inspect(this::verifyClassesHaveBeenMinified)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("A", "B", "C");
+ }
+
+ private void verifyClassesHaveBeenMinified(CodeInspector inspector) {
+ for (Class<?> clazz : ImmutableList.of(A.class, B.class, C.class)) {
+ ClassSubject classSubject = inspector.clazz(clazz);
+ assertThat(classSubject, isPresent());
+ assertThat(classSubject, isRenamed());
+ }
+ }
+
+ @Test
+ public void testCorrectnessOfNames() {
+ assertEquals(A.class.getName(), TestClass.NAME_A);
+ assertEquals(B.class.getName(), TestClass.NAME_B);
+ }
+
+ static class TestClass {
+
+ private static final String NAME_A =
+ "com.android.tools.r8.naming.identifiernamestring.ClassNameComparisonSwitchTest$A";
+
+ private static final String NAME_B =
+ "com.android.tools.r8.naming.identifiernamestring.ClassNameComparisonSwitchTest$B";
+
+ public static void main(String[] args) {
+ System.out.println(test(A.class));
+ System.out.println(test(B.class));
+ System.out.println(test(C.class));
+ }
+
+ @NeverInline
+ static String test(Class<?> clazz) {
+ switch (clazz.getName()) {
+ case NAME_A:
+ return "A";
+ case NAME_B:
+ return "B";
+ default:
+ return "C";
+ }
+ }
+ }
+
+ static class A {}
+
+ static class B {}
+
+ static class C {}
+}
