[Retrace] Use original source file names for all classes
Bug: b/226885646
Bug: b/234361164
Change-Id: I7a8a07e5c6bb9b8ebfa27b98fc7a63ce564b8ea6
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java b/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java
index b29dbc6..9f01beb 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMapper.java
@@ -47,19 +47,20 @@
private final Map<String, ClassNamingForNameMapper.Builder> mapping = new HashMap<>();
private LinkedHashSet<MapVersionMappingInformation> mapVersions = new LinkedHashSet<>();
+ private final Map<String, String> originalSourceFiles = new HashMap<>();
@Override
public ClassNamingForNameMapper.Builder classNamingBuilder(
String renamedName, String originalName, Position position) {
ClassNamingForNameMapper.Builder classNamingBuilder =
- ClassNamingForNameMapper.builder(renamedName, originalName);
+ ClassNamingForNameMapper.builder(renamedName, originalName, originalSourceFiles::put);
mapping.put(renamedName, classNamingBuilder);
return classNamingBuilder;
}
@Override
public ClassNameMapper build() {
- return new ClassNameMapper(buildClassNameMappings(), mapVersions);
+ return new ClassNameMapper(buildClassNameMappings(), mapVersions, originalSourceFiles);
}
private ImmutableMap<String, ClassNamingForNameMapper> buildClassNameMappings() {
@@ -177,12 +178,15 @@
private BiMapContainer<String, String> nameMapping;
private final Map<Signature, Signature> signatureMap = new HashMap<>();
private final Set<MapVersionMappingInformation> mapVersions;
+ private final Map<String, String> originalSourceFiles;
private ClassNameMapper(
ImmutableMap<String, ClassNamingForNameMapper> classNameMappings,
- Set<MapVersionMappingInformation> mapVersions) {
+ Set<MapVersionMappingInformation> mapVersions,
+ Map<String, String> originalSourceFiles) {
this.classNameMappings = classNameMappings;
this.mapVersions = mapVersions;
+ this.originalSourceFiles = originalSourceFiles;
}
public Map<String, ClassNamingForNameMapper> getClassNameMappings() {
@@ -234,6 +238,10 @@
return descriptorToJavaType(asString, this);
}
+ public String getSourceFile(String typeName) {
+ return originalSourceFiles.get(typeName);
+ }
+
@Override
public boolean hasMapping(DexType type) {
String decoded = descriptorToJavaType(type.descriptor.toString());
@@ -258,7 +266,7 @@
ImmutableMap.Builder<String, ClassNamingForNameMapper> builder = ImmutableMap.builder();
builder.orderEntriesByValue(Comparator.comparing(x -> x.originalName));
classNameMappings.forEach(builder::put);
- return new ClassNameMapper(builder.build(), mapVersions);
+ return new ClassNameMapper(builder.build(), mapVersions, originalSourceFiles);
}
public boolean verifyIsSorted() {
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java b/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java
index 373f584..c1e5de7 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java
@@ -23,6 +23,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
@@ -43,10 +44,15 @@
private final Map<String, List<MappedRange>> mappedRangesByName = Maps.newHashMap();
private final Map<String, List<MemberNaming>> mappedFieldNamingsByName = Maps.newHashMap();
private final List<MappingInformation> additionalMappingInfo = new ArrayList<>();
+ private final BiConsumer<String, String> originalSourceFileConsumer;
- private Builder(String renamedName, String originalName) {
+ private Builder(
+ String renamedName,
+ String originalName,
+ BiConsumer<String, String> originalSourceFileConsumer) {
this.originalName = originalName;
this.renamedName = renamedName;
+ this.originalSourceFileConsumer = originalSourceFileConsumer;
}
@Override
@@ -108,6 +114,9 @@
}
}
additionalMappingInfo.add(info);
+ if (info.isFileNameInformation()) {
+ originalSourceFileConsumer.accept(originalName, info.asFileNameInformation().getFileName());
+ }
}
}
@@ -205,8 +214,11 @@
}
}
- static Builder builder(String renamedName, String originalName) {
- return new Builder(renamedName, originalName);
+ static Builder builder(
+ String renamedName,
+ String originalName,
+ BiConsumer<String, String> originalSourceFileConsumer) {
+ return new Builder(renamedName, originalName, originalSourceFileConsumer);
}
public final String originalName;
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/MappingProviderInternal.java b/src/main/java/com/android/tools/r8/retrace/internal/MappingProviderInternal.java
index 1ff65a4..13187cc 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/MappingProviderInternal.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/MappingProviderInternal.java
@@ -12,5 +12,7 @@
abstract ClassNamingForNameMapper getClassNaming(String typeName);
+ abstract String getSourceFileForClass(String typeName);
+
abstract Set<MapVersionMappingInformation> getMapVersions();
}
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/ProguardMappingProviderImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/ProguardMappingProviderImpl.java
index dcfe2cb..c15efef 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/ProguardMappingProviderImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/ProguardMappingProviderImpl.java
@@ -43,4 +43,9 @@
}
return classNameMapper.getClassNaming(typeName);
}
+
+ @Override
+ String getSourceFileForClass(String typeName) {
+ return classNameMapper.getSourceFile(typeName);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetraceClassResultImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetraceClassResultImpl.java
index 7f71ad6..80931c3 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetraceClassResultImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetraceClassResultImpl.java
@@ -201,16 +201,7 @@
@Override
public RetracedSourceFile getSourceFile() {
- String sourceFile = null;
- if (mapper != null) {
- for (MappingInformation info : mapper.getAdditionalMappingInfo()) {
- if (info.isFileNameInformation()) {
- sourceFile = info.asFileNameInformation().getFileName();
- break;
- }
- }
- }
- return new RetracedSourceFileImpl(getRetracedClass().getClassReference(), sourceFile);
+ return RetraceUtils.getSourceFile(classReference, classResult.retracer);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetraceFrameResultImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetraceFrameResultImpl.java
index 0aa210c..8243d4a 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetraceFrameResultImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetraceFrameResultImpl.java
@@ -221,7 +221,7 @@
@Override
public boolean isEmpty() {
- return !mappedRanges.isEmpty();
+ return mappedRanges.isEmpty();
}
public static class ElementImpl implements RetraceFrameElement {
@@ -348,8 +348,7 @@
@Override
public RetracedSourceFile getSourceFile(RetracedClassMemberReference frame) {
- return RetraceUtils.getSourceFileOrLookup(
- frame.getHolderClass(), classElement, retraceFrameResult.retracer);
+ return RetraceUtils.getSourceFile(frame.getHolderClass(), retraceFrameResult.retracer);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetraceMethodResultImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetraceMethodResultImpl.java
index e5a2579..af8526c 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetraceMethodResultImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetraceMethodResultImpl.java
@@ -197,8 +197,8 @@
@Override
public RetracedSourceFile getSourceFile() {
- return RetraceUtils.getSourceFileOrLookup(
- methodReference.getHolderClass(), classElement, retraceMethodResult.retracer);
+ return RetraceUtils.getSourceFile(
+ methodReference.getHolderClass(), retraceMethodResult.retracer);
}
}
}
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetraceUtils.java b/src/main/java/com/android/tools/r8/retrace/internal/RetraceUtils.java
index c4ed860..2e1411a 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetraceUtils.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetraceUtils.java
@@ -12,14 +12,10 @@
import com.android.tools.r8.references.MethodReference;
import com.android.tools.r8.references.Reference;
import com.android.tools.r8.references.TypeReference;
-import com.android.tools.r8.retrace.RetraceClassElement;
-import com.android.tools.r8.retrace.RetraceClassResult;
import com.android.tools.r8.retrace.RetracedClassReference;
import com.android.tools.r8.retrace.RetracedMethodReference;
import com.android.tools.r8.retrace.RetracedMethodReference.KnownRetracedMethodReference;
import com.android.tools.r8.retrace.RetracedSourceFile;
-import com.android.tools.r8.retrace.Retracer;
-import com.android.tools.r8.utils.Box;
import com.android.tools.r8.utils.DescriptorUtils;
import com.google.common.collect.Sets;
import com.google.common.io.Files;
@@ -75,16 +71,10 @@
return clazz.substring(lastIndexOfPeriod + 1, endIndex);
}
- // TODO(b/226885646): Retracing of a source file should not be dependent on retraced information.
- public static RetracedSourceFile getSourceFileOrLookup(
- RetracedClassReference holder, RetraceClassElement context, Retracer retracer) {
- if (holder.equals(context.getRetracedClass())) {
- return context.getSourceFile();
- }
- RetraceClassResult contextClassResult = retracer.retraceClass(holder.getClassReference());
- Box<RetracedSourceFile> retraceSourceFile = new Box<>();
- contextClassResult.forEach(element -> retraceSourceFile.set(element.getSourceFile()));
- return retraceSourceFile.get();
+ public static RetracedSourceFile getSourceFile(
+ RetracedClassReference holder, RetracerImpl retracer) {
+ ClassReference holderReference = holder.getClassReference();
+ return new RetracedSourceFileImpl(holderReference, retracer.getSourceFile(holderReference));
}
public static String inferSourceFile(
diff --git a/src/main/java/com/android/tools/r8/retrace/internal/RetracerImpl.java b/src/main/java/com/android/tools/r8/retrace/internal/RetracerImpl.java
index 92e42f1..346b417 100644
--- a/src/main/java/com/android/tools/r8/retrace/internal/RetracerImpl.java
+++ b/src/main/java/com/android/tools/r8/retrace/internal/RetracerImpl.java
@@ -87,6 +87,10 @@
return classNameMapperProvider.getMapVersions();
}
+ public String getSourceFile(ClassReference classReference) {
+ return classNameMapperProvider.getSourceFileForClass(classReference.getTypeName());
+ }
+
public static Builder builder() {
return new Builder();
}
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 987aead..a8b5e71 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
@@ -131,7 +131,7 @@
? OptionalInt.of(element.getLineNumber())
: OptionalInt.empty(),
element.getMethodName());
- if (!frameResult.isEmpty()) {
+ if (frameResult.isEmpty()) {
return classResult.stream()
.map(
classElement ->
diff --git a/src/test/java/com/android/tools/r8/retrace/stacktraces/InlineSourceFileStackTrace.java b/src/test/java/com/android/tools/r8/retrace/stacktraces/InlineSourceFileStackTrace.java
index 069a7fd..9128e47 100644
--- a/src/test/java/com/android/tools/r8/retrace/stacktraces/InlineSourceFileStackTrace.java
+++ b/src/test/java/com/android/tools/r8/retrace/stacktraces/InlineSourceFileStackTrace.java
@@ -29,15 +29,13 @@
@Override
public List<String> retracedStackTrace() {
return Arrays.asList(
- // TODO(b/234361164): Should have correct source file.
- " at foo.Bar.method(Bar.java:22)", " at foo.Baz.main(SourceFile2.kt:32)");
+ " at foo.Bar.method(SourceFile1.kt:22)", " at foo.Baz.main(SourceFile2.kt:32)");
}
@Override
public List<String> retraceVerboseStackTrace() {
return Arrays.asList(
- // TODO(b/234361164): Should have correct source file.
- " at foo.Bar.void method()(Bar.java:22)",
+ " at foo.Bar.void method()(SourceFile1.kt:22)",
" at foo.Baz.void main(java.lang.String[])(SourceFile2.kt:32)");
}
