Merge "Add @NeverMerge annotation for testing"
diff --git a/src/main/java/com/android/tools/r8/shaking/ClassMergingRule.java b/src/main/java/com/android/tools/r8/shaking/ClassMergingRule.java
new file mode 100644
index 0000000..43bfe4b
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/shaking/ClassMergingRule.java
@@ -0,0 +1,82 @@
+// 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 com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.position.Position;
+import java.util.List;
+
+public class ClassMergingRule extends ProguardConfigurationRule {
+
+ public enum Type {
+ NEVER
+ }
+
+ public static class Builder extends ProguardConfigurationRule.Builder<ClassMergingRule, Builder> {
+
+ private Builder() {
+ super();
+ }
+
+ Type type;
+
+ @Override
+ public Builder self() {
+ return this;
+ }
+
+ public Builder setType(Type type) {
+ this.type = type;
+ return this;
+ }
+
+ @Override
+ public ClassMergingRule build() {
+ return new ClassMergingRule(origin, getPosition(), source, classAnnotation, classAccessFlags,
+ negatedClassAccessFlags, classTypeNegated, classType, classNames, inheritanceAnnotation,
+ inheritanceClassName, inheritanceIsExtends, memberRules, type);
+ }
+ }
+
+ private final Type type;
+
+ private ClassMergingRule(
+ Origin origin,
+ Position position,
+ String source,
+ ProguardTypeMatcher classAnnotation,
+ ProguardAccessFlags classAccessFlags,
+ ProguardAccessFlags negatedClassAccessFlags,
+ boolean classTypeNegated,
+ ProguardClassType classType,
+ ProguardClassNameList classNames,
+ ProguardTypeMatcher inheritanceAnnotation,
+ ProguardTypeMatcher inheritanceClassName,
+ boolean inheritanceIsExtends,
+ List<ProguardMemberRule> memberRules,
+ Type type) {
+ super(origin, position, source, classAnnotation, classAccessFlags, negatedClassAccessFlags,
+ classTypeNegated, classType, classNames, inheritanceAnnotation, inheritanceClassName,
+ inheritanceIsExtends, memberRules);
+ this.type = type;
+ }
+
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ public Type getType() {
+ return type;
+ }
+
+ @Override
+ String typeString() {
+ switch (type) {
+ case NEVER:
+ return "nevermerge";
+ }
+ throw new Unreachable("Unknown class merging type " + type);
+ }
+}
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 e0cf415..8574e6d 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1813,6 +1813,10 @@
*/
public final Set<DexMethod> neverInline;
/**
+ * All types that *must* never be merged due to a configuration directive (testing only).
+ */
+ public final Set<DexType> neverMerge;
+ /**
* All items with -identifiernamestring rule.
* Bound boolean value indicates the rule is explicitly specified by users (<code>true</code>)
* or not, i.e., implicitly added by R8 (<code>false</code>).
@@ -1871,6 +1875,7 @@
this.alwaysInline = enqueuer.rootSet.alwaysInline;
this.forceInline = enqueuer.rootSet.forceInline;
this.neverInline = enqueuer.rootSet.neverInline;
+ this.neverMerge = enqueuer.rootSet.neverMerge;
this.identifierNameStrings = joinIdentifierNameStrings(
enqueuer.rootSet.identifierNameStrings, enqueuer.identifierNameStrings);
this.prunedTypes = Collections.emptySet();
@@ -1913,6 +1918,7 @@
this.alwaysInline = previous.alwaysInline;
this.forceInline = previous.forceInline;
this.neverInline = previous.neverInline;
+ this.neverMerge = previous.neverMerge;
this.identifierNameStrings = previous.identifierNameStrings;
this.prunedTypes = mergeSets(previous.prunedTypes, removedClasses);
this.switchMaps = previous.switchMaps;
@@ -1965,6 +1971,10 @@
this.alwaysInline = previous.alwaysInline;
this.forceInline = lense.rewriteMethodsWithRenamedSignature(previous.forceInline);
this.neverInline = lense.rewriteMethodsWithRenamedSignature(previous.neverInline);
+ assert lense.assertDefinitionNotModified(
+ previous.neverMerge.stream().map(this::definitionFor).filter(Objects::nonNull)
+ .collect(Collectors.toList()));
+ this.neverMerge = previous.neverMerge;
this.identifierNameStrings =
lense.rewriteReferencesConservatively(previous.identifierNameStrings);
// Switchmap classes should never be affected by renaming.
@@ -2008,6 +2018,7 @@
this.alwaysInline = previous.alwaysInline;
this.forceInline = previous.forceInline;
this.neverInline = previous.neverInline;
+ this.neverMerge = previous.neverMerge;
this.identifierNameStrings = previous.identifierNameStrings;
this.prunedTypes = previous.prunedTypes;
this.switchMaps = switchMaps;
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 faf5317..7d5f9a5 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -364,6 +364,9 @@
} else if (allowTestOptions && acceptString("neverinline")) {
InlineRule rule = parseInlineRule(Type.NEVER, optionStart);
configurationBuilder.addRule(rule);
+ } else if (allowTestOptions && acceptString("nevermerge")) {
+ ClassMergingRule rule = parseClassMergingRule(ClassMergingRule.Type.NEVER, optionStart);
+ configurationBuilder.addRule(rule);
} else if (acceptString("useuniqueclassmembernames")) {
configurationBuilder.setUseUniqueClassMemberNames(true);
} else if (acceptString("adaptclassstrings")) {
@@ -594,6 +597,17 @@
return keepRuleBuilder.build();
}
+ private ClassMergingRule parseClassMergingRule(ClassMergingRule.Type type, Position start)
+ throws ProguardRuleParserException {
+ ClassMergingRule.Builder keepRuleBuilder =
+ ClassMergingRule.builder().setOrigin(origin).setStart(start).setType(type);
+ parseClassSpec(keepRuleBuilder, false);
+ Position end = getPosition();
+ keepRuleBuilder.setSource(getSourceSnippet(contents, start, end));
+ keepRuleBuilder.setEnd(end);
+ return keepRuleBuilder.build();
+ }
+
private InlineRule parseInlineRule(InlineRule.Type type, Position start)
throws ProguardRuleParserException {
InlineRule.Builder keepRuleBuilder = InlineRule.builder()
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 432ae10..19eb127 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -66,6 +66,7 @@
private final Set<DexMethod> alwaysInline = Sets.newIdentityHashSet();
private final Set<DexMethod> forceInline = Sets.newIdentityHashSet();
private final Set<DexMethod> neverInline = Sets.newIdentityHashSet();
+ private final Set<DexType> neverMerge = Sets.newIdentityHashSet();
private final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking =
new IdentityHashMap<>();
private final Map<DexDefinition, ProguardMemberRule> noSideEffects = new IdentityHashMap<>();
@@ -173,6 +174,10 @@
} else if (rule instanceof ProguardAssumeNoSideEffectRule) {
markMatchingVisibleMethods(clazz, memberKeepRules, rule, null);
markMatchingFields(clazz, memberKeepRules, rule, null);
+ } else if (rule instanceof ClassMergingRule) {
+ if (allRulesSatisfied(memberKeepRules, clazz)) {
+ markClass(clazz, rule);
+ }
} else if (rule instanceof InlineRule) {
markMatchingMethods(clazz, memberKeepRules, rule, null);
} else if (rule instanceof ProguardAssumeValuesRule) {
@@ -245,6 +250,7 @@
alwaysInline,
forceInline,
neverInline,
+ neverMerge,
noSideEffects,
assumedValues,
dependentNoShrinking,
@@ -844,6 +850,16 @@
default:
throw new Unreachable();
}
+ } else if (context instanceof ClassMergingRule) {
+ switch (((ClassMergingRule) context).getType()) {
+ case NEVER:
+ if (item.isDexClass()) {
+ neverMerge.add(item.asDexClass().type);
+ }
+ break;
+ default:
+ throw new Unreachable();
+ }
} else if (context instanceof ProguardIdentifierNameStringRule) {
if (item.isDexEncodedField()) {
identifierNameStrings.add(item.asDexEncodedField().field);
@@ -864,6 +880,7 @@
public final Set<DexMethod> alwaysInline;
public final Set<DexMethod> forceInline;
public final Set<DexMethod> neverInline;
+ public final Set<DexType> neverMerge;
public final Map<DexDefinition, ProguardMemberRule> noSideEffects;
public final Map<DexDefinition, ProguardMemberRule> assumedValues;
private final Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking;
@@ -880,6 +897,7 @@
Set<DexMethod> alwaysInline,
Set<DexMethod> forceInline,
Set<DexMethod> neverInline,
+ Set<DexType> neverMerge,
Map<DexDefinition, ProguardMemberRule> noSideEffects,
Map<DexDefinition, ProguardMemberRule> assumedValues,
Map<DexDefinition, Map<DexDefinition, ProguardKeepRule>> dependentNoShrinking,
@@ -894,6 +912,7 @@
this.alwaysInline = Collections.unmodifiableSet(alwaysInline);
this.forceInline = Collections.unmodifiableSet(forceInline);
this.neverInline = Collections.unmodifiableSet(neverInline);
+ this.neverMerge = Collections.unmodifiableSet(neverMerge);
this.noSideEffects = Collections.unmodifiableMap(noSideEffects);
this.assumedValues = Collections.unmodifiableMap(assumedValues);
this.dependentNoShrinking = dependentNoShrinking;
diff --git a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
index 5d88374..71a249f 100644
--- a/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
@@ -132,6 +132,9 @@
}
public boolean satisfiesMergeCriteria(DexProgramClass clazz) {
+ if (appView.appInfo().neverMerge.contains(clazz.type)) {
+ return false;
+ }
if (clazz.accessFlags.isInterface()) {
return false;
}
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 2824718..df4c786 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -353,7 +353,8 @@
if (appInfo.instantiatedTypes.contains(clazz.type)
|| appInfo.instantiatedLambdas.contains(clazz.type)
|| appInfo.isPinned(clazz.type)
- || pinnedTypes.contains(clazz.type)) {
+ || pinnedTypes.contains(clazz.type)
+ || appInfo.neverMerge.contains(clazz.type)) {
return false;
}
// Note that the property "singleSubtype == null" cannot change during merging, since we visit
diff --git a/src/test/java/com/android/tools/r8/NeverMerge.java b/src/test/java/com/android/tools/r8/NeverMerge.java
new file mode 100644
index 0000000..7fc97b9
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/NeverMerge.java
@@ -0,0 +1,6 @@
+// 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;
+
+public @interface NeverMerge {}
diff --git a/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java b/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java
index b694949..d83c891 100644
--- a/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java
+++ b/src/test/java/com/android/tools/r8/naming/b116840216/ReserveOuterClassNameTest.java
@@ -10,12 +10,12 @@
import com.android.tools.r8.CompatProguardCommandBuilder;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
-import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
@@ -27,8 +27,10 @@
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
+@NeverMerge
class Outer {
+ @NeverMerge
static class Inner {
@NeverInline
static void foo() {
@@ -80,12 +82,12 @@
// Note that reproducing b/116840216 relies on the order of following rules that cause
// the visiting of classes during class minification to be Outer$Inner before Outer.
"-keepnames class " + Outer.class.getCanonicalName() + "$Inner",
- keepOuterName ? "-keepnames class " + Outer.class.getCanonicalName() : ""
- ),
+ keepOuterName ? "-keepnames class " + Outer.class.getCanonicalName() : "",
+ "-nevermerge @com.android.tools.r8.NeverMerge class *"),
Origin.unknown());
ToolHelper.allowTestProguardOptions(builder);
- AndroidApp processedApp = ToolHelper.runR8(builder.build(), this::configure);
+ AndroidApp processedApp = ToolHelper.runR8(builder.build());
CodeInspector inspector = new CodeInspector(processedApp);
ClassSubject mainSubject = inspector.clazz(mainClass);
@@ -110,12 +112,6 @@
assertThat(foo, isRenamed());
}
- private void configure(InternalOptions options) {
- // Disable horizontal class merging to avoid that all members of Outer are moved to Outer$Inner
- // or vice versa (both Outer and Outer$Inner are merge candidates for the static class merger).
- options.enableHorizontalClassMerging = false;
- }
-
@Test
public void test_keepOuterName() throws Exception {
runTest(true);
diff --git a/src/test/java/com/android/tools/r8/shaking/testrules/A.java b/src/test/java/com/android/tools/r8/shaking/testrules/A.java
index af05562..cf97eeb 100644
--- a/src/test/java/com/android/tools/r8/shaking/testrules/A.java
+++ b/src/test/java/com/android/tools/r8/shaking/testrules/A.java
@@ -4,6 +4,9 @@
package com.android.tools.r8.shaking.testrules;
+import com.android.tools.r8.NeverMerge;
+
+@NeverMerge
public class A {
public static int m(int a, int b) {
diff --git a/src/test/java/com/android/tools/r8/shaking/testrules/C.java b/src/test/java/com/android/tools/r8/shaking/testrules/C.java
index 5ee4d53..79370a4 100644
--- a/src/test/java/com/android/tools/r8/shaking/testrules/C.java
+++ b/src/test/java/com/android/tools/r8/shaking/testrules/C.java
@@ -4,6 +4,9 @@
package com.android.tools.r8.shaking.testrules;
+import com.android.tools.r8.NeverMerge;
+
+@NeverMerge
public class C {
private static int i;
diff --git a/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java b/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
index 1b7ebe9..96894df 100644
--- a/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/testrules/ForceInlineTest.java
@@ -20,7 +20,6 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.origin.Origin;
-import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.google.common.collect.ImmutableList;
@@ -62,21 +61,17 @@
.addLibraryFiles(library);
ToolHelper.allowTestProguardOptions(builder);
builder.addProguardConfiguration(proguardConfiguration, Origin.unknown());
- return new CodeInspector(ToolHelper.runR8(builder.build(), this::configure));
- }
-
- private void configure(InternalOptions options) {
- // Disable horizontal class merging to prevent that A and C are merged (both classes are
- // candidates for the static class merger).
- options.enableHorizontalClassMerging = false;
+ return new CodeInspector(ToolHelper.runR8(builder.build()));
}
@Test
public void testDefaultInlining() throws Exception {
- CodeInspector inspector = runTest(ImmutableList.of(
- "-keep class **.Main { *; }",
- "-dontobfuscate"
- ));
+ CodeInspector inspector =
+ runTest(
+ ImmutableList.of(
+ "-keep class **.Main { *; }",
+ "-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-dontobfuscate"));
ClassSubject classA = inspector.clazz(A.class);
ClassSubject classB = inspector.clazz(B.class);
@@ -99,12 +94,14 @@
@Test
public void testNeverInline() throws Exception {
- CodeInspector inspector = runTest(ImmutableList.of(
- "-neverinline class **.A { method(); }",
- "-neverinline class **.B { method(); }",
- "-keep class **.Main { *; }",
- "-dontobfuscate"
- ));
+ CodeInspector inspector =
+ runTest(
+ ImmutableList.of(
+ "-neverinline class **.A { method(); }",
+ "-neverinline class **.B { method(); }",
+ "-keep class **.Main { *; }",
+ "-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-dontobfuscate"));
ClassSubject classA = inspector.clazz(A.class);
ClassSubject classB = inspector.clazz(B.class);
@@ -124,12 +121,14 @@
@Test
public void testForceInline() throws Exception {
- CodeInspector inspector = runTest(ImmutableList.of(
- "-forceinline class **.A { int m(int, int); }",
- "-forceinline class **.B { int m(int, int); }",
- "-keep class **.Main { *; }",
- "-dontobfuscate"
- ));
+ CodeInspector inspector =
+ runTest(
+ ImmutableList.of(
+ "-forceinline class **.A { int m(int, int); }",
+ "-forceinline class **.B { int m(int, int); }",
+ "-keep class **.Main { *; }",
+ "-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-dontobfuscate"));
ClassSubject classA = inspector.clazz(A.class);
ClassSubject classB = inspector.clazz(B.class);
@@ -146,11 +145,13 @@
@Test
public void testForceInlineFails() throws Exception {
try {
- CodeInspector inspector = runTest(ImmutableList.of(
- "-forceinline class **.A { int x(); }",
- "-keep class **.Main { *; }",
- "-dontobfuscate"
- ));
+ CodeInspector inspector =
+ runTest(
+ ImmutableList.of(
+ "-forceinline class **.A { int x(); }",
+ "-keep class **.Main { *; }",
+ "-nevermerge @com.android.tools.r8.NeverMerge class *",
+ "-dontobfuscate"));
fail("Force inline of non-inlinable method succeeded");
} catch (Throwable t) {
// Ignore assertion error.
