Merge "Add tests to cover the implicit keeping of default constructors"
diff --git a/src/main/java/com/android/tools/r8/StringConsumer.java b/src/main/java/com/android/tools/r8/StringConsumer.java
index 543e8cd..9232e08 100644
--- a/src/main/java/com/android/tools/r8/StringConsumer.java
+++ b/src/main/java/com/android/tools/r8/StringConsumer.java
@@ -14,7 +14,6 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.util.Collections;
/** Interface for receiving String resource. */
public interface StringConsumer {
@@ -101,7 +100,7 @@
public void accept(String string, DiagnosticsHandler handler) {
super.accept(string, handler);
try {
- Files.write(outputPath, Collections.singletonList(string), encoding);
+ Files.write(outputPath, string.getBytes(encoding));
} catch (IOException e) {
handler.error(new IOExceptionDiagnostic(e, new PathOrigin(outputPath)));
}
diff --git a/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java b/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
index 1fa8aaa..9e85188 100644
--- a/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MemberNameMinifier.java
@@ -32,8 +32,8 @@
this.dictionary = options.proguardConfiguration.getObfuscationDictionary();
this.useUniqueMemberNames = options.proguardConfiguration.isUseUniqueClassMemberNames();
this.overloadAggressively = options.proguardConfiguration.isOverloadAggressively();
- this.globalState =
- NamingState.createRoot(appInfo.dexItemFactory, dictionary, getKeyTransform());
+ this.globalState = NamingState.createRoot(
+ appInfo.dexItemFactory, dictionary, getKeyTransform(), useUniqueMemberNames);
}
abstract Function<StateType, ?> getKeyTransform();
diff --git a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
index 3138d32..ce37763 100644
--- a/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/MethodNameMinifier.java
@@ -343,7 +343,8 @@
computeStateIfAbsent(
libraryFrontier,
ignore -> parent == null
- ? NamingState.createRoot(appInfo.dexItemFactory, dictionary, getKeyTransform())
+ ? NamingState.createRoot(
+ appInfo.dexItemFactory, dictionary, getKeyTransform(), useUniqueMemberNames)
: parent.createChild());
if (holder != null) {
boolean keepAll = holder.isLibraryClass() || holder.accessFlags.isAnnotation();
diff --git a/src/main/java/com/android/tools/r8/naming/NamingState.java b/src/main/java/com/android/tools/r8/naming/NamingState.java
index b3ff544..abfa3c1 100644
--- a/src/main/java/com/android/tools/r8/naming/NamingState.java
+++ b/src/main/java/com/android/tools/r8/naming/NamingState.java
@@ -7,9 +7,11 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexString;
import com.android.tools.r8.utils.StringUtils;
-import com.google.common.collect.HashBiMap;
+import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
+import com.google.common.collect.Table;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
@@ -20,80 +22,86 @@
class NamingState<ProtoType extends CachedHashValueDexItem, KeyType> {
private final NamingState<ProtoType, KeyType> parent;
- private final Map<KeyType, InternalState> usedNames = new HashMap<>();
+ private final Map<KeyType, InternalState<ProtoType>> usedNames = new HashMap<>();
private final DexItemFactory itemFactory;
private final ImmutableList<String> dictionary;
private final Function<ProtoType, KeyType> keyTransform;
+ private final boolean useUniqueMemberNames;
static <S, T extends CachedHashValueDexItem> NamingState<T, S> createRoot(
- DexItemFactory itemFactory, ImmutableList<String> dictionary, Function<T, S> keyTransform) {
- return new NamingState<>(null, itemFactory, dictionary, keyTransform);
+ DexItemFactory itemFactory,
+ ImmutableList<String> dictionary,
+ Function<T, S> keyTransform,
+ boolean useUniqueMemberNames) {
+ return new NamingState<>(null, itemFactory, dictionary, keyTransform, useUniqueMemberNames);
}
private NamingState(
NamingState<ProtoType, KeyType> parent,
DexItemFactory itemFactory,
ImmutableList<String> dictionary,
- Function<ProtoType, KeyType> keyTransform) {
+ Function<ProtoType, KeyType> keyTransform,
+ boolean useUniqueMemberNames) {
this.parent = parent;
this.itemFactory = itemFactory;
this.dictionary = dictionary;
this.keyTransform = keyTransform;
+ this.useUniqueMemberNames = useUniqueMemberNames;
}
public NamingState<ProtoType, KeyType> createChild() {
- return new NamingState<>(this, itemFactory, dictionary, keyTransform);
+ return new NamingState<>(this, itemFactory, dictionary, keyTransform, useUniqueMemberNames);
}
- private InternalState findInternalStateFor(ProtoType proto) {
+ private InternalState<ProtoType> findInternalStateFor(ProtoType proto) {
KeyType key = keyTransform.apply(proto);
- InternalState result = usedNames.get(key);
+ InternalState<ProtoType> result = usedNames.get(key);
if (result == null && parent != null) {
result = parent.findInternalStateFor(proto);
}
return result;
}
- private InternalState getOrCreateInternalStateFor(ProtoType proto) {
+ private InternalState<ProtoType> getOrCreateInternalStateFor(ProtoType proto) {
// TODO(herhut): Maybe allocate these sparsely and search via state chain.
KeyType key = keyTransform.apply(proto);
- InternalState result = usedNames.get(key);
+ InternalState<ProtoType> result = usedNames.get(key);
if (result == null) {
if (parent != null) {
- InternalState parentState = parent.getOrCreateInternalStateFor(proto);
+ InternalState<ProtoType> parentState = parent.getOrCreateInternalStateFor(proto);
result = parentState.createChild();
} else {
- result = new InternalState(itemFactory, null, dictionary);
+ result = new InternalState<>(itemFactory, null, dictionary);
}
usedNames.put(key, result);
}
return result;
}
- public DexString getAssignedNameFor(DexString name, ProtoType proto) {
- InternalState state = findInternalStateFor(proto);
+ private DexString getAssignedNameFor(DexString name, ProtoType proto) {
+ InternalState<ProtoType> state = findInternalStateFor(proto);
if (state == null) {
return null;
}
- return state.getAssignedNameFor(name);
+ return state.getAssignedNameFor(name, proto);
}
public DexString assignNewNameFor(DexString original, ProtoType proto, boolean markAsUsed) {
DexString result = getAssignedNameFor(original, proto);
if (result == null) {
- InternalState state = getOrCreateInternalStateFor(proto);
- result = state.getNameFor(original, markAsUsed);
+ InternalState<ProtoType> state = getOrCreateInternalStateFor(proto);
+ result = state.getNameFor(original, proto, markAsUsed);
}
return result;
}
public void reserveName(DexString name, ProtoType proto) {
- InternalState state = getOrCreateInternalStateFor(proto);
+ InternalState<ProtoType> state = getOrCreateInternalStateFor(proto);
state.reserveName(name);
}
public boolean isReserved(DexString name, ProtoType proto) {
- InternalState state = findInternalStateFor(proto);
+ InternalState<ProtoType> state = findInternalStateFor(proto);
if (state == null) {
return false;
}
@@ -101,32 +109,34 @@
}
public boolean isAvailable(DexString original, ProtoType proto, DexString candidate) {
- InternalState state = findInternalStateFor(proto);
+ InternalState<ProtoType> state = findInternalStateFor(proto);
if (state == null) {
return true;
}
- assert state.getAssignedNameFor(original) != candidate;
+ assert state.getAssignedNameFor(original, proto) != candidate;
return state.isAvailable(candidate);
}
public void addRenaming(DexString original, ProtoType proto, DexString newName) {
- InternalState state = getOrCreateInternalStateFor(proto);
- state.addRenaming(original, newName);
+ InternalState<ProtoType> state = getOrCreateInternalStateFor(proto);
+ state.addRenaming(original, proto, newName);
}
- private static class InternalState {
+ private class InternalState<InternalProtoType extends CachedHashValueDexItem> {
private static final int INITIAL_NAME_COUNT = 1;
- private final static char[] EMPTY_CHAR_ARRARY = new char[0];
+ private final char[] EMPTY_CHAR_ARRARY = new char[0];
protected final DexItemFactory itemFactory;
- private final InternalState parentInternalState;
+ private final InternalState<InternalProtoType> parentInternalState;
private Set<DexString> reservedNames = null;
- private Map<DexString, DexString> renamings = null;
+ private Table<DexString, InternalProtoType, DexString> renamings = null;
private int nameCount;
private final Iterator<String> dictionaryIterator;
- private InternalState(DexItemFactory itemFactory, InternalState parentInternalState,
+ private InternalState(
+ DexItemFactory itemFactory,
+ InternalState<InternalProtoType> parentInternalState,
Iterator<String> dictionaryIterator) {
this.itemFactory = itemFactory;
this.parentInternalState = parentInternalState;
@@ -135,7 +145,9 @@
this.dictionaryIterator = dictionaryIterator;
}
- private InternalState(DexItemFactory itemFactory, InternalState parentInternalState,
+ private InternalState(
+ DexItemFactory itemFactory,
+ InternalState<InternalProtoType> parentInternalState,
List<String> dictionary) {
this(itemFactory, parentInternalState, dictionary.iterator());
}
@@ -151,27 +163,40 @@
&& (parentInternalState == null || parentInternalState.isAvailable(name));
}
- public InternalState createChild() {
- return new InternalState(itemFactory, this, dictionaryIterator);
+ InternalState<InternalProtoType> createChild() {
+ return new InternalState<>(itemFactory, this, dictionaryIterator);
}
- public void reserveName(DexString name) {
+ void reserveName(DexString name) {
if (reservedNames == null) {
reservedNames = Sets.newIdentityHashSet();
}
reservedNames.add(name);
}
- public DexString getAssignedNameFor(DexString original) {
- DexString result = renamings == null ? null : renamings.get(original);
+ DexString getAssignedNameFor(DexString original, InternalProtoType proto) {
+ DexString result = null;
+ if (renamings != null) {
+ if (useUniqueMemberNames) {
+ Map<InternalProtoType, DexString> row = renamings.row(original);
+ if (row != null) {
+ // Either not renamed yet (0) or renamed (1). If renamed, return the same renamed name
+ // so that other members with the same name can be renamed to the same renamed name.
+ assert row.values().size() <= 1;
+ result = Iterables.getOnlyElement(row.values(), null);
+ }
+ } else {
+ result = renamings.get(original, proto);
+ }
+ }
if (result == null && parentInternalState != null) {
- result = parentInternalState.getAssignedNameFor(original);
+ result = parentInternalState.getAssignedNameFor(original, proto);
}
return result;
}
- public DexString getNameFor(DexString original, boolean markAsUsed) {
- DexString name = getAssignedNameFor(original);
+ DexString getNameFor(DexString original, InternalProtoType proto, boolean markAsUsed) {
+ DexString name = getAssignedNameFor(original, proto);
if (name != null) {
return name;
}
@@ -179,19 +204,19 @@
name = itemFactory.createString(nextSuggestedName());
} while (!isAvailable(name));
if (markAsUsed) {
- addRenaming(original, name);
+ addRenaming(original, proto, name);
}
return name;
}
- public void addRenaming(DexString original, DexString newName) {
+ void addRenaming(DexString original, InternalProtoType proto, DexString newName) {
if (renamings == null) {
- renamings = HashBiMap.create();
+ renamings = HashBasedTable.create();
}
- renamings.put(original, newName);
+ renamings.put(original, proto, newName);
}
- protected String nextSuggestedName() {
+ String nextSuggestedName() {
if (dictionaryIterator.hasNext()) {
return dictionaryIterator.next();
} else {
diff --git a/src/test/java/com/android/tools/r8/naming/overloadaggressively/ValidNameConflictTest.java b/src/test/java/com/android/tools/r8/naming/overloadaggressively/ValidNameConflictTest.java
index f949aea..a510cee 100644
--- a/src/test/java/com/android/tools/r8/naming/overloadaggressively/ValidNameConflictTest.java
+++ b/src/test/java/com/android/tools/r8/naming/overloadaggressively/ValidNameConflictTest.java
@@ -290,7 +290,6 @@
+ " static <methods>;"
+ "}\n",
keepMainProguardConfiguration(CLASS_NAME),
- "-useuniqueclassmembernames",
"-dontshrink");
AndroidApp app = compileWithR8(builder, pgConfigs, null);
@@ -478,7 +477,6 @@
+ " <methods>;"
+ "}\n",
keepMainProguardConfiguration(CLASS_NAME),
- "-useuniqueclassmembernames",
"-dontshrink");
AndroidApp app = compileWithR8(builder, pgConfigs, null);
@@ -631,15 +629,11 @@
MethodSubject subM2 = sub.method("java.lang.Object", "same", ImmutableList.of());
assertTrue(subM2.isPresent());
assertTrue(subM2.isRenamed());
- // TODO(b/73149686): R8 should be able to fix this conflict w/o -overloadaggressively.
- assertEquals(subM1.getFinalName(), subM2.getFinalName());
+ assertNotEquals(subM1.getFinalName(), subM2.getFinalName());
- // TODO(b/73149686): The current impl of method minifier visits classes in a hierarchical order.
- // Hence, already renamed same names in Super are not resolvable as the internal naming state
- // uses only _name_ to resolve methods in the hierarchy.
- // // No matter what, overloading methods should be renamed to the same name.
- // assertEquals(m1.getFinalName(), subM1.getFinalName());
- // assertEquals(m2.getFinalName(), subM2.getFinalName());
+ // No matter what, overloading methods should be renamed to the same name.
+ assertEquals(m1.getFinalName(), subM1.getFinalName());
+ assertEquals(m2.getFinalName(), subM2.getFinalName());
ProcessResult artOutput = runOnArtRaw(app, CLASS_NAME, dexVm);
assertEquals(0, artOutput.exitCode);
