[Retrace] Add comparison of ambiguous frame results
Bug: 170797525
Change-Id: I174c39f60bfe3f79bf8e1bb9e6704e38d5fb42a8
diff --git a/src/main/java/com/android/tools/r8/retrace/Retrace.java b/src/main/java/com/android/tools/r8/retrace/Retrace.java
index e64d8e0..d9dd386 100644
--- a/src/main/java/com/android/tools/r8/retrace/Retrace.java
+++ b/src/main/java/com/android/tools/r8/retrace/Retrace.java
@@ -18,7 +18,9 @@
import com.android.tools.r8.retrace.internal.RetraceRegularExpression;
import com.android.tools.r8.retrace.internal.RetracerImpl;
import com.android.tools.r8.retrace.internal.StackTraceElementProxyRetracerImpl;
+import com.android.tools.r8.retrace.internal.StackTraceElementProxyRetracerImpl.RetraceStackTraceProxyImpl;
import com.android.tools.r8.retrace.internal.StackTraceElementStringProxy;
+import com.android.tools.r8.utils.Box;
import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.OptionsParsing;
import com.android.tools.r8.utils.OptionsParsing.ParseContext;
@@ -35,7 +37,9 @@
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.Scanner;
/**
@@ -186,20 +190,42 @@
List<String> retracedStrings = new ArrayList<>();
plainStackTraceVisitor.forEach(
stackTraceElement -> {
- List<String> retracedStringsForElement = new ArrayList<>();
+ Box<List<RetraceStackTraceProxyImpl<StackTraceElementStringProxy>>> currentList =
+ new Box<>();
+ Map<
+ RetraceStackTraceProxyImpl<StackTraceElementStringProxy>,
+ List<RetraceStackTraceProxyImpl<StackTraceElementStringProxy>>>
+ ambiguousBlocks = new HashMap<>();
proxyRetracer
.retrace(stackTraceElement)
.forEach(
retracedElement -> {
- StackTraceElementStringProxy originalItem =
- retracedElement.getOriginalItem();
- retracedStringsForElement.add(
- originalItem.toRetracedItem(
- retracedElement,
- !retracedStringsForElement.isEmpty(),
- command.isVerbose));
+ if (retracedElement.isTopFrame() || !retracedElement.hasRetracedClass()) {
+ List<RetraceStackTraceProxyImpl<StackTraceElementStringProxy>> block =
+ new ArrayList<>();
+ ambiguousBlocks.put(retracedElement, block);
+ currentList.set(block);
+ }
+ currentList.get().add(retracedElement);
});
- retracedStrings.addAll(retracedStringsForElement);
+ ambiguousBlocks.keySet().stream()
+ .sorted()
+ .forEach(
+ topFrame -> {
+ ambiguousBlocks
+ .get(topFrame)
+ .forEach(
+ frame -> {
+ StackTraceElementStringProxy originalItem =
+ frame.getOriginalItem();
+ retracedStrings.add(
+ originalItem.toRetracedItem(
+ frame, !currentList.isSet(), command.isVerbose));
+ // Use the current list as indicator for us seeing the first
+ // sorted element.
+ currentList.set(null);
+ });
+ });
});
result = new RetraceCommandLineResult(retracedStrings);
}
diff --git a/src/main/java/com/android/tools/r8/retrace/RetraceStackTraceProxy.java b/src/main/java/com/android/tools/r8/retrace/RetraceStackTraceProxy.java
index b69c237..ebae15c 100644
--- a/src/main/java/com/android/tools/r8/retrace/RetraceStackTraceProxy.java
+++ b/src/main/java/com/android/tools/r8/retrace/RetraceStackTraceProxy.java
@@ -7,10 +7,13 @@
import com.android.tools.r8.Keep;
@Keep
-public interface RetraceStackTraceProxy<T extends StackTraceElementProxy<?>> {
+public interface RetraceStackTraceProxy<T extends StackTraceElementProxy<?>>
+ extends Comparable<RetraceStackTraceProxy<T>> {
boolean isAmbiguous();
+ boolean isTopFrame();
+
boolean hasRetracedClass();
boolean hasRetracedMethod();
diff --git a/src/main/java/com/android/tools/r8/retrace/RetracedMethod.java b/src/main/java/com/android/tools/r8/retrace/RetracedMethod.java
index bfc95e9..936ea3e 100644
--- a/src/main/java/com/android/tools/r8/retrace/RetracedMethod.java
+++ b/src/main/java/com/android/tools/r8/retrace/RetracedMethod.java
@@ -10,7 +10,7 @@
import java.util.List;
@Keep
-public interface RetracedMethod extends RetracedClassMember {
+public interface RetracedMethod extends RetracedClassMember, Comparable<RetracedMethod> {
boolean isUnknown();
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetracedMethodImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetracedMethodImpl.java
index 998fe7d..1f4ad68 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetracedMethodImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetracedMethodImpl.java
@@ -8,6 +8,8 @@
import com.android.tools.r8.references.MethodReference;
import com.android.tools.r8.references.TypeReference;
import com.android.tools.r8.retrace.RetracedMethod;
+import com.android.tools.r8.utils.ComparatorUtils;
+import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
@@ -34,6 +36,27 @@
return null;
}
+ @Override
+ public int compareTo(RetracedMethod other) {
+ return Comparator.comparing(RetracedMethod::getMethodName)
+ .thenComparing(RetracedMethod::isKnown)
+ .thenComparing(
+ RetracedMethod::asKnown,
+ Comparator.nullsFirst(
+ Comparator.comparing(
+ (KnownRetracedMethod m) -> {
+ if (m == null) {
+ return null;
+ }
+ return m.isVoid() ? "void" : m.getReturnType().getTypeName();
+ }))
+ .thenComparing(
+ KnownRetracedMethod::getFormalTypes,
+ ComparatorUtils.listComparator(
+ Comparator.comparing(TypeReference::getTypeName))))
+ .compare(this, other);
+ }
+
public static final class KnownRetracedMethodImpl extends RetracedMethodImpl
implements KnownRetracedMethod {
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/StackTraceElementProxyRetracerImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/StackTraceElementProxyRetracerImpl.java
index c21b92d..458978d 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/StackTraceElementProxyRetracerImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/StackTraceElementProxyRetracerImpl.java
@@ -37,7 +37,8 @@
RetraceStackTraceProxyImpl.Builder<T> proxy =
RetraceStackTraceProxyImpl.builder(element)
.setRetracedClassElement(classElement.getRetracedClass())
- .setAmbiguous(classResult.isAmbiguous());
+ .setAmbiguous(classResult.isAmbiguous())
+ .setTopFrame(true);
if (element.hasFileName()) {
proxy.setSourceFile(classElement.retraceSourceFile(element.fileName()).getFilename());
}
@@ -56,7 +57,8 @@
RetraceStackTraceProxyImpl.builder(element)
.setRetracedClassElement(frame.getHolderClass())
.setRetracedMethodElement(frame)
- .setAmbiguous(frameResult.isAmbiguous() && index == 0);
+ .setAmbiguous(frameResult.isAmbiguous() && index == 0)
+ .setTopFrame(index == 0);
if (element.hasLineNumber()) {
proxy.setLineNumber(frame.getOriginalPositionOrDefault(element.lineNumber()));
}
@@ -80,6 +82,7 @@
private final String sourceFile;
private final int lineNumber;
private final boolean isAmbiguous;
+ private final boolean isTopFrame;
private RetraceStackTraceProxyImpl(
T originalItem,
@@ -87,7 +90,8 @@
RetracedMethod retracedMethod,
String sourceFile,
int lineNumber,
- boolean isAmbiguous) {
+ boolean isAmbiguous,
+ boolean isTopFrame) {
assert originalItem != null;
this.originalItem = originalItem;
this.retracedClass = retracedClass;
@@ -95,6 +99,7 @@
this.sourceFile = sourceFile;
this.lineNumber = lineNumber;
this.isAmbiguous = isAmbiguous;
+ this.isTopFrame = isTopFrame;
}
@Override
@@ -103,6 +108,11 @@
}
@Override
+ public boolean isTopFrame() {
+ return isTopFrame;
+ }
+
+ @Override
public boolean hasRetracedClass() {
return retracedClass != null;
}
@@ -151,6 +161,49 @@
return lineNumber;
}
+ @Override
+ public int compareTo(RetraceStackTraceProxy<T> other) {
+ int classCompare = Boolean.compare(hasRetracedClass(), other.hasRetracedClass());
+ if (classCompare != 0) {
+ return classCompare;
+ }
+ if (hasRetracedClass()) {
+ classCompare =
+ getRetracedClass().getTypeName().compareTo(other.getRetracedClass().getTypeName());
+ if (classCompare != 0) {
+ return classCompare;
+ }
+ }
+ int methodCompare = Boolean.compare(hasRetracedMethod(), other.hasRetracedMethod());
+ if (methodCompare != 0) {
+ return methodCompare;
+ }
+ if (hasRetracedMethod()) {
+ methodCompare = getRetracedMethod().compareTo(other.getRetracedMethod());
+ if (methodCompare != 0) {
+ return methodCompare;
+ }
+ }
+ int sourceFileCompare = Boolean.compare(hasSourceFile(), other.hasSourceFile());
+ if (sourceFileCompare != 0) {
+ return sourceFileCompare;
+ }
+ if (hasSourceFile()) {
+ sourceFileCompare = getSourceFile().compareTo(other.getSourceFile());
+ if (sourceFileCompare != 0) {
+ return sourceFileCompare;
+ }
+ }
+ int lineNumberCompare = Boolean.compare(hasLineNumber(), other.hasLineNumber());
+ if (lineNumberCompare != 0) {
+ return lineNumberCompare;
+ }
+ if (hasLineNumber()) {
+ return Integer.compare(lineNumber, other.getLineNumber());
+ }
+ return 0;
+ }
+
private static class Builder<T extends StackTraceElementProxy<?>> {
private final T originalElement;
@@ -159,6 +212,7 @@
private String sourceFile;
private int lineNumber = -1;
private boolean isAmbiguous;
+ private boolean isTopFrame;
private Builder(T originalElement) {
this.originalElement = originalElement;
@@ -189,6 +243,11 @@
return this;
}
+ private Builder<T> setTopFrame(boolean topFrame) {
+ isTopFrame = topFrame;
+ return this;
+ }
+
private RetraceStackTraceProxyImpl<T> build() {
RetracedClass retracedClass = classContext;
if (methodContext != null) {
@@ -200,7 +259,8 @@
methodContext,
sourceFile != null ? sourceFile : null,
lineNumber,
- isAmbiguous);
+ isAmbiguous,
+ isTopFrame);
}
}
}
diff --git a/src/test/java/com/android/tools/r8/retrace/RetraceTests.java b/src/test/java/com/android/tools/r8/retrace/RetraceTests.java
index edcb118..82d3915 100644
--- a/src/test/java/com/android/tools/r8/retrace/RetraceTests.java
+++ b/src/test/java/com/android/tools/r8/retrace/RetraceTests.java
@@ -10,7 +10,6 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeFalse;
-import static org.junit.Assume.assumeTrue;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestDiagnosticMessagesImpl;
@@ -136,15 +135,11 @@
@Test
public void testAmbiguousStackTrace() {
- // TODO(b/170797525): Remove when we have a fixed ordering.
- assumeTrue(useRegExpParsing);
runRetraceTest(new AmbiguousStackTrace());
}
@Test
public void testAmbiguousMissingLineStackTrace() {
- // TODO(b/170797525): Remove when we have a fixed ordering.
- assumeTrue(useRegExpParsing);
runRetraceTest(new AmbiguousMissingLineStackTrace());
}
@@ -190,8 +185,6 @@
@Test
public void testUnknownSourceStackTrace() {
- // TODO(b/170797525): Remove when we have a fixed ordering.
- assumeTrue(useRegExpParsing);
runRetraceTest(new UnknownSourceStackTrace());
}
diff --git a/src/test/java/com/android/tools/r8/retrace/RetraceVerboseTests.java b/src/test/java/com/android/tools/r8/retrace/RetraceVerboseTests.java
index fb65c1d..2e75b1d 100644
--- a/src/test/java/com/android/tools/r8/retrace/RetraceVerboseTests.java
+++ b/src/test/java/com/android/tools/r8/retrace/RetraceVerboseTests.java
@@ -6,6 +6,7 @@
import static com.android.tools.r8.retrace.Retrace.DEFAULT_REGULAR_EXPRESSION;
import static junit.framework.TestCase.assertEquals;
+import static org.junit.Assume.assumeFalse;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestDiagnosticMessagesImpl;
@@ -53,6 +54,8 @@
@Test
public void testAmbiguousMissingLineVerbose() {
+ // TODO(b/169346455): Enable when separated parser.
+ assumeFalse(useRegExpParsing);
runRetraceTest(new AmbiguousWithSignatureVerboseStackTrace());
}
diff --git a/src/test/java/com/android/tools/r8/retrace/stacktraces/AmbiguousWithSignatureVerboseStackTrace.java b/src/test/java/com/android/tools/r8/retrace/stacktraces/AmbiguousWithSignatureVerboseStackTrace.java
index 1dfb3c9..45c103f 100644
--- a/src/test/java/com/android/tools/r8/retrace/stacktraces/AmbiguousWithSignatureVerboseStackTrace.java
+++ b/src/test/java/com/android/tools/r8/retrace/stacktraces/AmbiguousWithSignatureVerboseStackTrace.java
@@ -33,10 +33,10 @@
return Arrays.asList(
"java.lang.IndexOutOfBoundsException",
"\tat java.util.ArrayList.get(ArrayList.java:411)",
- "\tat com.android.tools.r8.Internal.void foo(int)(Internal.java)",
- "\t<OR> at com.android.tools.r8.Internal.void foo(int,int)(Internal.java)",
+ "\tat com.android.tools.r8.Internal.boolean foo(int,int)(Internal.java)",
+ "\t<OR> at com.android.tools.r8.Internal.void foo(int)(Internal.java)",
"\t<OR> at com.android.tools.r8.Internal.void foo(int,boolean)(Internal.java)",
- "\t<OR> at com.android.tools.r8.Internal.boolean foo(int,int)(Internal.java)");
+ "\t<OR> at com.android.tools.r8.Internal.void foo(int,int)(Internal.java)");
}
@Override
