Repackaging of classes with members having package dependent visibility
RELNOTES: R8 will start to repackage classes, if possible, where the class or members have package dependent visibility.
Bug: b/250671873
Change-Id: Icb492d737aeed5aea7260abaae089c6eef44f1f5
diff --git a/src/main/java/com/android/tools/r8/repackaging/Repackaging.java b/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
index f292409..d3aee17 100644
--- a/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
+++ b/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
@@ -440,7 +440,7 @@
+ simpleName.substring(outerClassSimpleName.length());
repackagedDexType = repackagedDexType.withSimpleName(newSimpleName, dexItemFactory);
} else {
- assert false
+ assert options.disableInnerClassSeparatorValidationWhenRepackaging
: "Unexpected name for inner class: "
+ clazz.getType().toSourceString()
+ " (outer class: "
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index a8058ac..9e0db7a 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -64,7 +64,6 @@
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.PredicateSet;
import com.android.tools.r8.utils.ThreadUtils;
-import com.android.tools.r8.utils.TraversalContinuation;
import com.android.tools.r8.utils.Visibility;
import com.android.tools.r8.utils.WorkList;
import com.android.tools.r8.utils.collections.ProgramMethodSet;
@@ -1089,22 +1088,11 @@
if (!options().isRepackagingEnabled()) {
return false;
}
- if (!keepInfo.getInfo(clazz).isRepackagingAllowed(clazz, options())) {
+ if (!keepInfo.getInfo(clazz).isRepackagingAllowed(options())) {
return false;
}
SeedMapper applyMappingSeedMapper = appView.getApplyMappingSeedMapper();
- if (applyMappingSeedMapper != null && applyMappingSeedMapper.hasMapping(clazz.type)) {
- return false;
- }
- return clazz
- .traverseProgramMembers(
- member -> {
- if (keepInfo.getInfo(member).isRepackagingAllowed(member, options())) {
- return TraversalContinuation.doContinue();
- }
- return TraversalContinuation.doBreak();
- })
- .shouldContinue();
+ return applyMappingSeedMapper == null || !applyMappingSeedMapper.hasMapping(clazz.type);
}
public boolean isPinned(DexReference reference) {
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
index 9785028..f504792 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
@@ -7,7 +7,6 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexProgramClass;
import com.android.tools.r8.graph.DexType;
-import com.android.tools.r8.graph.ProgramDefinition;
import com.android.tools.r8.utils.InternalOptions;
import java.util.function.Function;
@@ -15,10 +14,10 @@
public final class KeepClassInfo extends KeepInfo<KeepClassInfo.Builder, KeepClassInfo> {
// Requires all aspects of a class to be kept.
- private static final KeepClassInfo TOP = new Builder().makeTop().build();
+ private static final KeepClassInfo TOP = new Builder().makeTop().disallowRepackaging().build();
// Requires no aspects of a class to be kept.
- private static final KeepClassInfo BOTTOM = new Builder().makeBottom().build();
+ private static final KeepClassInfo BOTTOM = new Builder().makeBottom().allowRepackaging().build();
public static KeepClassInfo top() {
return TOP;
@@ -32,10 +31,12 @@
return bottom().joiner();
}
+ private final boolean allowRepackaging;
private final boolean checkEnumUnboxed;
private KeepClassInfo(Builder builder) {
super(builder);
+ this.allowRepackaging = builder.isRepackagingAllowed();
this.checkEnumUnboxed = builder.isCheckEnumUnboxedEnabled();
}
@@ -57,13 +58,18 @@
return new Joiner(this);
}
- @Override
- public boolean isRepackagingAllowed(
- ProgramDefinition definition, GlobalKeepInfoConfiguration configuration) {
- return configuration.isRepackagingEnabled()
- && internalIsRepackagingAllowed()
- && (definition.getAccessFlags().isPublic()
- || !internalIsAccessModificationRequiredForRepackaging());
+ /**
+ * True if an item may be repackaged.
+ *
+ * <p>This method requires knowledge of the global configuration as that can override the concrete
+ * value on a given item.
+ */
+ public boolean isRepackagingAllowed(GlobalKeepInfoConfiguration configuration) {
+ return configuration.isRepackagingEnabled() && internalIsRepackagingAllowed();
+ }
+
+ boolean internalIsRepackagingAllowed() {
+ return allowRepackaging;
}
public boolean isKotlinMetadataRemovalAllowed(
@@ -98,6 +104,7 @@
public static class Builder extends KeepInfo.Builder<Builder, KeepClassInfo> {
+ private boolean allowRepackaging;
private boolean checkEnumUnboxed;
private Builder() {
@@ -106,6 +113,7 @@
private Builder(KeepClassInfo original) {
super(original);
+ allowRepackaging = original.internalIsRepackagingAllowed();
checkEnumUnboxed = original.internalIsCheckEnumUnboxedEnabled();
}
@@ -120,6 +128,23 @@
return self();
}
+ public boolean isRepackagingAllowed() {
+ return allowRepackaging;
+ }
+
+ private Builder setAllowRepackaging(boolean allowRepackaging) {
+ this.allowRepackaging = allowRepackaging;
+ return self();
+ }
+
+ public Builder allowRepackaging() {
+ return setAllowRepackaging(true);
+ }
+
+ public Builder disallowRepackaging() {
+ return setAllowRepackaging(false);
+ }
+
public Builder setCheckEnumUnboxed() {
return setCheckEnumUnboxed(true);
}
@@ -151,6 +176,7 @@
@Override
boolean internalIsEqualTo(KeepClassInfo other) {
return super.internalIsEqualTo(other)
+ && isRepackagingAllowed() == other.internalIsRepackagingAllowed()
&& isCheckEnumUnboxedEnabled() == other.internalIsCheckEnumUnboxedEnabled();
}
@@ -161,12 +187,12 @@
@Override
public Builder makeTop() {
- return super.makeTop().unsetCheckEnumUnboxed();
+ return super.makeTop().unsetCheckEnumUnboxed().disallowRepackaging();
}
@Override
public Builder makeBottom() {
- return super.makeBottom().unsetCheckEnumUnboxed();
+ return super.makeBottom().unsetCheckEnumUnboxed().allowRepackaging();
}
}
@@ -181,6 +207,11 @@
return self();
}
+ public Joiner disallowRepackaging() {
+ builder.disallowRepackaging();
+ return self();
+ }
+
@Override
public Joiner asClassJoiner() {
return this;
@@ -190,7 +221,8 @@
public Joiner merge(Joiner joiner) {
// Should be extended to merge the fields of this class in case any are added.
return super.merge(joiner)
- .applyIf(joiner.builder.isCheckEnumUnboxedEnabled(), Joiner::setCheckEnumUnboxed);
+ .applyIf(joiner.builder.isCheckEnumUnboxedEnabled(), Joiner::setCheckEnumUnboxed)
+ .applyIf(!joiner.builder.isRepackagingAllowed(), Joiner::disallowRepackaging);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
index d9708ce..092f4ea 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
@@ -6,7 +6,6 @@
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.DexReference;
import com.android.tools.r8.graph.EnclosingMethodAttribute;
-import com.android.tools.r8.graph.ProgramDefinition;
import com.android.tools.r8.shaking.KeepInfo.Builder;
import com.android.tools.r8.shaking.KeepReason.ReflectiveUseFrom;
import com.android.tools.r8.utils.InternalOptions;
@@ -22,31 +21,25 @@
private final boolean allowAnnotationRemoval;
private final boolean allowMinification;
private final boolean allowOptimization;
- private final boolean allowRepackaging;
private final boolean allowShrinking;
private final boolean allowSignatureRemoval;
private final boolean checkDiscarded;
- private final boolean requireAccessModificationForRepackaging;
private KeepInfo(
boolean allowAccessModification,
boolean allowAnnotationRemoval,
boolean allowMinification,
boolean allowOptimization,
- boolean allowRepackaging,
boolean allowShrinking,
boolean allowSignatureRemoval,
- boolean checkDiscarded,
- boolean requireAccessModificationForRepackaging) {
+ boolean checkDiscarded) {
this.allowAccessModification = allowAccessModification;
this.allowAnnotationRemoval = allowAnnotationRemoval;
this.allowMinification = allowMinification;
this.allowOptimization = allowOptimization;
- this.allowRepackaging = allowRepackaging;
this.allowShrinking = allowShrinking;
this.allowSignatureRemoval = allowSignatureRemoval;
this.checkDiscarded = checkDiscarded;
- this.requireAccessModificationForRepackaging = requireAccessModificationForRepackaging;
}
KeepInfo(B builder) {
@@ -55,11 +48,9 @@
builder.isAnnotationRemovalAllowed(),
builder.isMinificationAllowed(),
builder.isOptimizationAllowed(),
- builder.isRepackagingAllowed(),
builder.isShrinkingAllowed(),
builder.isSignatureRemovalAllowed(),
- builder.isCheckDiscardedEnabled(),
- builder.isAccessModificationRequiredForRepackaging());
+ builder.isCheckDiscardedEnabled());
}
public static Joiner<?, ?, ?> newEmptyJoinerFor(DexReference reference) {
@@ -164,23 +155,6 @@
}
/**
- * True if an item may be repackaged.
- *
- * <p>This method requires knowledge of the global configuration as that can override the concrete
- * value on a given item.
- */
- public abstract boolean isRepackagingAllowed(
- ProgramDefinition definition, GlobalKeepInfoConfiguration configuration);
-
- boolean internalIsRepackagingAllowed() {
- return allowRepackaging;
- }
-
- boolean internalIsAccessModificationRequiredForRepackaging() {
- return requireAccessModificationForRepackaging;
- }
-
- /**
* True if an item may have its access flags modified.
*
* <p>This method requires knowledge of the global access modification as that will override the
@@ -242,7 +216,6 @@
&& (allowAnnotationRemoval || !other.internalIsAnnotationRemovalAllowed())
&& (allowMinification || !other.internalIsMinificationAllowed())
&& (allowOptimization || !other.internalIsOptimizationAllowed())
- && (allowRepackaging || !other.internalIsRepackagingAllowed())
&& (allowShrinking || !other.internalIsShrinkingAllowed())
&& (allowSignatureRemoval || !other.internalIsSignatureRemovalAllowed())
&& (!checkDiscarded || other.internalIsCheckDiscardedEnabled());
@@ -265,12 +238,10 @@
private boolean allowAccessModification;
private boolean allowAnnotationRemoval;
private boolean allowMinification;
- private boolean allowRepackaging;
private boolean allowOptimization;
private boolean allowShrinking;
private boolean allowSignatureRemoval;
private boolean checkDiscarded;
- private boolean requireAccessModificationForRepackaging;
Builder() {
// Default initialized. Use should be followed by makeTop/makeBottom.
@@ -282,12 +253,9 @@
allowAnnotationRemoval = original.internalIsAnnotationRemovalAllowed();
allowMinification = original.internalIsMinificationAllowed();
allowOptimization = original.internalIsOptimizationAllowed();
- allowRepackaging = original.internalIsRepackagingAllowed();
allowShrinking = original.internalIsShrinkingAllowed();
allowSignatureRemoval = original.internalIsSignatureRemovalAllowed();
checkDiscarded = original.internalIsCheckDiscardedEnabled();
- requireAccessModificationForRepackaging =
- original.internalIsAccessModificationRequiredForRepackaging();
}
B makeTop() {
@@ -295,11 +263,9 @@
disallowAnnotationRemoval();
disallowMinification();
disallowOptimization();
- disallowRepackaging();
disallowShrinking();
disallowSignatureRemoval();
unsetCheckDiscarded();
- requireAccessModificationForRepackaging();
return self();
}
@@ -308,11 +274,9 @@
allowAnnotationRemoval();
allowMinification();
allowOptimization();
- allowRepackaging();
allowShrinking();
allowSignatureRemoval();
unsetCheckDiscarded();
- unsetRequireAccessModificationForRepackaging();
return self();
}
@@ -336,16 +300,9 @@
&& isAnnotationRemovalAllowed() == other.internalIsAnnotationRemovalAllowed()
&& isMinificationAllowed() == other.internalIsMinificationAllowed()
&& isOptimizationAllowed() == other.internalIsOptimizationAllowed()
- && isRepackagingAllowed() == other.internalIsRepackagingAllowed()
&& isShrinkingAllowed() == other.internalIsShrinkingAllowed()
&& isSignatureRemovalAllowed() == other.internalIsSignatureRemovalAllowed()
- && isCheckDiscardedEnabled() == other.internalIsCheckDiscardedEnabled()
- && isAccessModificationRequiredForRepackaging()
- == other.internalIsAccessModificationRequiredForRepackaging();
- }
-
- public boolean isAccessModificationRequiredForRepackaging() {
- return requireAccessModificationForRepackaging;
+ && isCheckDiscardedEnabled() == other.internalIsCheckDiscardedEnabled();
}
public boolean isAccessModificationAllowed() {
@@ -368,10 +325,6 @@
return allowOptimization;
}
- public boolean isRepackagingAllowed() {
- return allowRepackaging;
- }
-
public boolean isShrinkingAllowed() {
return allowShrinking;
}
@@ -393,19 +346,6 @@
return setAllowMinification(false);
}
- public B setAllowRepackaging(boolean allowRepackaging) {
- this.allowRepackaging = allowRepackaging;
- return self();
- }
-
- public B allowRepackaging() {
- return setAllowRepackaging(true);
- }
-
- public B disallowRepackaging() {
- return setAllowRepackaging(false);
- }
-
public B setAllowOptimization(boolean allowOptimization) {
this.allowOptimization = allowOptimization;
return self();
@@ -445,20 +385,6 @@
return setCheckDiscarded(false);
}
- public B setRequireAccessModificationForRepackaging(
- boolean requireAccessModificationForRepackaging) {
- this.requireAccessModificationForRepackaging = requireAccessModificationForRepackaging;
- return self();
- }
-
- public B requireAccessModificationForRepackaging() {
- return setRequireAccessModificationForRepackaging(true);
- }
-
- public B unsetRequireAccessModificationForRepackaging() {
- return setRequireAccessModificationForRepackaging(false);
- }
-
public B setAllowAccessModification(boolean allowAccessModification) {
this.allowAccessModification = allowAccessModification;
return self();
@@ -608,11 +534,6 @@
return self();
}
- public J disallowRepackaging() {
- builder.disallowRepackaging();
- return self();
- }
-
public J disallowOptimization() {
builder.disallowOptimization();
return self();
@@ -633,24 +554,15 @@
return self();
}
- public J requireAccessModificationForRepackaging() {
- builder.requireAccessModificationForRepackaging();
- return self();
- }
-
public J merge(J joiner) {
Builder<B, K> builder = joiner.builder;
applyIf(!builder.isAccessModificationAllowed(), Joiner::disallowAccessModification);
applyIf(!builder.isAnnotationRemovalAllowed(), Joiner::disallowAnnotationRemoval);
applyIf(!builder.isMinificationAllowed(), Joiner::disallowMinification);
applyIf(!builder.isOptimizationAllowed(), Joiner::disallowOptimization);
- applyIf(!builder.isRepackagingAllowed(), Joiner::disallowRepackaging);
applyIf(!builder.isShrinkingAllowed(), Joiner::disallowShrinking);
applyIf(!builder.isSignatureRemovalAllowed(), Joiner::disallowSignatureRemoval);
applyIf(builder.isCheckDiscardedEnabled(), Joiner::setCheckDiscarded);
- applyIf(
- builder.isAccessModificationRequiredForRepackaging(),
- Joiner::requireAccessModificationForRepackaging);
reasons.addAll(joiner.reasons);
rules.addAll(joiner.rules);
return self();
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java b/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java
index 62529ed..c78e39a 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfoCollection.java
@@ -140,6 +140,10 @@
throw new Unreachable();
}
+ public final KeepClassInfo getInfo(DexProgramClass clazz) {
+ return getClassInfo(clazz);
+ }
+
public final KeepInfo<?, ?> getInfo(ProgramDefinition definition) {
if (definition.isProgramClass()) {
return getClassInfo(definition.asProgramClass());
@@ -279,8 +283,7 @@
assert newType == type
|| !info.isPinned(options)
|| info.isMinificationAllowed(options)
- || info.isRepackagingAllowed(
- definitions.definitionFor(newType).asProgramClass(), options);
+ || info.isRepackagingAllowed(options);
KeepClassInfo previous = newClassInfo.put(newType, info);
assert previous == null;
});
@@ -461,29 +464,6 @@
joinMethod(method, KeepInfo.Joiner::top);
}
- public void unsetRequireAllowAccessModificationForRepackaging(ProgramDefinition definition) {
- if (definition.isProgramClass()) {
- DexProgramClass clazz = definition.asProgramClass();
- KeepClassInfo info = getClassInfo(clazz);
- keepClassInfo.put(
- clazz.getType(), info.builder().unsetRequireAccessModificationForRepackaging().build());
- } else if (definition.isProgramMethod()) {
- ProgramMethod method = definition.asProgramMethod();
- KeepMethodInfo info = getMethodInfo(method);
- keepMethodInfo.put(
- method.getReference(),
- info.builder().unsetRequireAccessModificationForRepackaging().build());
- } else if (definition.isProgramField()) {
- ProgramField field = definition.asProgramField();
- KeepFieldInfo info = getFieldInfo(field);
- keepFieldInfo.put(
- field.getReference(),
- info.builder().unsetRequireAccessModificationForRepackaging().build());
- } else {
- throw new Unreachable();
- }
- }
-
public void joinField(ProgramField field, Consumer<? super KeepFieldInfo.Joiner> fn) {
KeepFieldInfo info = getFieldInfo(field);
if (info.isTop()) {
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepMemberInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepMemberInfo.java
index 2225e14..e77c27e 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepMemberInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepMemberInfo.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.shaking;
import com.android.tools.r8.graph.DexProgramClass;
-import com.android.tools.r8.graph.ProgramDefinition;
import com.android.tools.r8.shaking.KeepInfo.Builder;
/** Immutable keep requirements for a member. */
@@ -15,14 +14,6 @@
super(builder);
}
- @Override
- public boolean isRepackagingAllowed(
- ProgramDefinition definition, GlobalKeepInfoConfiguration configuration) {
- return configuration.isRepackagingEnabled()
- && (definition.getAccessFlags().isPublic()
- || !internalIsAccessModificationRequiredForRepackaging());
- }
-
public boolean isKotlinMetadataRemovalAllowed(
DexProgramClass holder, GlobalKeepInfoConfiguration configuration) {
// Checking the holder for missing kotlin information relies on the holder being processed
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java b/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
index 3b46564..cc39429 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
@@ -1135,6 +1135,7 @@
dependentMinimumKeepInfo
.getOrCreateMinimumKeepInfoFor(preconditionEvent, clazz.getReference())
.disallowMinification()
+ .asClassJoiner()
.disallowRepackaging();
}
}
@@ -1638,14 +1639,14 @@
dependentMinimumKeepInfo
.getOrCreateMinimumKeepInfoFor(preconditionEvent, item.getReference())
.disallowMinification()
- .disallowRepackaging();
+ .applyIf(item.isProgramClass(), joiner -> joiner.asClassJoiner().disallowRepackaging());
context.markAsUsed();
}
if (appView.options().isRepackagingEnabled() && !modifiers.allowsObfuscation) {
dependentMinimumKeepInfo
.getOrCreateMinimumKeepInfoFor(preconditionEvent, item.getReference())
- .disallowRepackaging();
+ .applyIf(item.isProgramClass(), joiner -> joiner.asClassJoiner().disallowRepackaging());
context.markAsUsed();
}
@@ -1664,10 +1665,6 @@
.addRule(keepRule)
.disallowShrinking();
context.markAsUsed();
-
- if (item.getAccessFlags().isPackagePrivateOrProtected()) {
- minimumKeepInfoForItem.requireAccessModificationForRepackaging();
- }
}
if (modifiers.includeDescriptorClasses) {
@@ -1981,7 +1978,8 @@
getDependentMinimumKeepInfo()
.getOrCreateUnconditionalMinimumKeepInfoFor(definition.getReference())
.disallowMinification()
- .disallowRepackaging();
+ .applyIf(
+ definition.isProgramClass(), joiner -> joiner.asClassJoiner().disallowRepackaging());
}
public boolean verifyKeptFieldsAreAccessedAndLive(AppView<AppInfoWithLiveness> appView) {
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 68c055f..9af4f06 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -804,6 +804,7 @@
public boolean reportMissingClassesInEnclosingMethodAttribute = false;
public boolean reportMissingClassesInInnerClassAttributes = false;
public boolean disableGenericSignatureValidation = false;
+ public boolean disableInnerClassSeparatorValidationWhenRepackaging = false;
// EXPERIMENTAL flag to get behaviour as close to Proguard as possible.
public boolean forceProguardCompatibility = false;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java b/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java
index 8b33acb..374fb98 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/reflection/InnerClassNameTestRunner.java
@@ -225,12 +225,16 @@
R8TestCompileResult r8CompileResult =
testForR8(parameters.getBackend())
.addKeepMainRule(MAIN_CLASS)
- .addKeepRules("-keep,allowobfuscation class * { *; }")
- .addKeepRules("-keepattributes InnerClasses,EnclosingMethod")
+ .addKeepAllClassesRuleWithAllowObfuscation()
+ .addKeepAttributeInnerClassesAndEnclosingMethod()
.addProgramClassFileData(InnerClassNameTestDump.dump(config, parameters))
.allowDiagnosticInfoMessages(hasMalformedInnerClassAttribute())
.minification(minify)
- .addOptionsModification(InternalOptions::disableNameReflectionOptimization)
+ .addOptionsModification(
+ options -> {
+ options.disableInnerClassSeparatorValidationWhenRepackaging = true;
+ options.disableNameReflectionOptimization();
+ })
.setMinApi(parameters.getApiLevel())
.compile()
.apply(this::checkWarningsAboutMalformedAttribute);
diff --git a/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java b/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java
index 487498f..fa9de18 100644
--- a/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java
+++ b/src/test/java/com/android/tools/r8/naming/PackageNamingTest.java
@@ -356,13 +356,11 @@
ba.getDexProgramClass().getType().getPackageName(),
bb.getDexProgramClass().getType().getPackageName());
- // We cannot repackage c or d since these have package-private members and a
- // keep,allowobfuscation. For us to be able to repackage them, we have to use
- // -allowaccesmodification.
+ // We can repackage c and d since these have no external package-private references.
List<String> klasses = ImmutableList.of("naming101.c", "naming101.d");
for (String klass : klasses) {
ClassSubject k = inspector.clazz(klass);
- assertNotEquals("naming101.a", k.getDexProgramClass().getType().getPackageName());
+ assertEquals("naming101.a", k.getDexProgramClass().getType().getPackageName());
}
// All other classes can be repackaged to naming101.a, but naming101.a.a exists to make a name
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageProtectedInSamePackageTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageProtectedInSamePackageTest.java
index a6c9ea3..8152688 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageProtectedInSamePackageTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageProtectedInSamePackageTest.java
@@ -45,7 +45,7 @@
.addKeepMainRule(Main.class)
.addDontWarn(RepackageProtectedInSamePackageTest.class, NeverClassInline.class)
.run(parameters.getRuntime(), Main.class, typeName(RepackageForKeepClassMembers.class))
- .inspect(inspector -> inspect(inspector, true))
+ .inspect(this::inspect)
.assertSuccessWithOutputLines(EXPECTED);
}
@@ -61,7 +61,7 @@
.addKeepMainRule(Main.class)
.enableNeverClassInliningAnnotations()
.run(parameters.getRuntime(), Main.class)
- .inspect(inspector -> inspect(inspector, false))
+ .inspect(this::inspect)
.assertSuccessWithOutputLines(EXPECTED);
}
@@ -82,16 +82,10 @@
.transform();
}
- private void inspect(CodeInspector inspector, boolean isProguard) {
+ private void inspect(CodeInspector inspector) {
ClassSubject clazz = inspector.clazz(RepackageForKeepClassMembers.class);
assertThat(clazz, isPresent());
- // TODO(b/250671873): We should be able to repackage the Sub class since the only reference
- // to Sub.class is in the same package and we have allowobfuscation.
- if (isProguard) {
- assertThat(RepackageForKeepClassMembers.class, isRepackaged(inspector));
- } else {
- assertThat(RepackageForKeepClassMembers.class, isNotRepackaged(inspector));
- }
+ assertThat(RepackageForKeepClassMembers.class, isRepackaged(inspector));
assertThat(clazz.uniqueFieldWithOriginalName("hashCodeCache"), isPresentAndNotRenamed());
assertThat(clazz.uniqueMethodWithOriginalName("calculateHashCode"), isPresentAndNotRenamed());
}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageTest.java
index 179845d..9742b6a 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageTest.java
@@ -163,18 +163,16 @@
// 3.A, 3.B, 3.C) Accessing a kept method is OK.
markShouldAlwaysBeEligible.accept(AccessPublicKeptMethodOnReachableClass.class);
- markEligibleWithAllowAccessModification.accept(
- AccessPackagePrivateKeptMethodOnReachableClassDirect.class);
- markEligibleWithAllowAccessModification.accept(
- AccessPackagePrivateKeptMethodOnReachableClassIndirect.class);
+ markShouldAlwaysBeEligible.accept(AccessPackagePrivateKeptMethodOnReachableClassDirect.class);
+ markShouldAlwaysBeEligible.accept(AccessPackagePrivateKeptMethodOnReachableClassIndirect.class);
// 4) -keepclassmembers,allowobfuscation class ReachableClassWithKeptMethod { <methods>; }
// 4.A, 4.B, 4.C) Accessing a kept method is OK.
markShouldAlwaysBeEligible.accept(AccessPublicKeptMethodAllowRenamingOnReachableClass.class);
- markEligibleWithAllowAccessModification.accept(
+ markShouldAlwaysBeEligible.accept(
AccessPackagePrivateKeptMethodAllowRenamingOnReachableClassDirect.class);
- markEligibleWithAllowAccessModification.accept(
+ markShouldAlwaysBeEligible.accept(
AccessPackagePrivateKeptMethodAllowRenamingOnReachableClassIndirect.class);
// 5) No keep rule.
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageWithKeepPackagePrivateTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageWithKeepPackagePrivateTest.java
index 8020e78..fda2011 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageWithKeepPackagePrivateTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageWithKeepPackagePrivateTest.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.R8TestCompileResult;
import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.google.common.collect.ImmutableList;
@@ -62,7 +63,12 @@
.compile()
.addRunClasspathFiles(compileResult.writeToZip())
.run(parameters.getRuntime(), TestClass.class)
- .assertSuccessWithOutputLines("Hello world!");
+ .assertFailureWithErrorThatThrowsIf(hasIllegalAccessError(), IllegalAccessError.class)
+ .assertSuccessWithOutputLinesIf(!hasIllegalAccessError(), "Hello world!");
+ }
+
+ private boolean hasIllegalAccessError() {
+ return !allowAccessModification && !parameters.isDexRuntimeVersion(Version.V8_1_0);
}
private List<String> getKeepRules() {
@@ -76,8 +82,8 @@
}
private void inspect(CodeInspector inspector) {
- assertThat(A.class, isRepackagedIf(inspector, allowAccessModification));
- assertThat(B.class, isRepackagedIf(inspector, allowAccessModification));
+ assertThat(A.class, isRepackaged(inspector));
+ assertThat(B.class, isRepackaged(inspector));
}
public static class TestClass {
