Add pending inline frame on methods that are abstract or have canonicalized code
Bug: b/302509457
Bug: b/325913397
Change-Id: I5233f82ed8ac4cf9c7904ad480434727e60bc338
diff --git a/src/main/java/com/android/tools/r8/graph/Code.java b/src/main/java/com/android/tools/r8/graph/Code.java
index f00b07b..b4913b1 100644
--- a/src/main/java/com/android/tools/r8/graph/Code.java
+++ b/src/main/java/com/android/tools/r8/graph/Code.java
@@ -245,4 +245,8 @@
DexMethod method, boolean isD8R8Synthesized, Consumer<Position> positionConsumer) {
// Intentionally empty. Override where we have fully build CF or DEX code.
}
+
+ public boolean supportsPendingInlineFrame() {
+ return false;
+ }
}
diff --git a/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java b/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java
index 2ce1db6..229e8c0 100644
--- a/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java
+++ b/src/main/java/com/android/tools/r8/graph/DefaultInstanceInitializerCode.java
@@ -7,7 +7,9 @@
import com.android.tools.r8.cf.CfVersion;
import com.android.tools.r8.cf.code.CfInstruction;
import com.android.tools.r8.cf.code.CfInvoke;
+import com.android.tools.r8.cf.code.CfLabel;
import com.android.tools.r8.cf.code.CfLoad;
+import com.android.tools.r8.cf.code.CfPosition;
import com.android.tools.r8.cf.code.CfReturnVoid;
import com.android.tools.r8.dex.CodeToKeep;
import com.android.tools.r8.dex.IndexedItemCollection;
@@ -33,6 +35,7 @@
import com.android.tools.r8.ir.conversion.MethodConversionOptions;
import com.android.tools.r8.ir.conversion.MethodConversionOptions.MutableMethodConversionOptions;
import com.android.tools.r8.ir.conversion.SyntheticStraightLineSourceCode;
+import com.android.tools.r8.lightir.LirBuilder;
import com.android.tools.r8.lightir.LirCode;
import com.android.tools.r8.lightir.LirEncodingStrategy;
import com.android.tools.r8.lightir.LirStrategy;
@@ -42,7 +45,7 @@
import com.android.tools.r8.utils.structural.HashingVisitor;
import com.google.common.collect.ImmutableList;
import java.nio.ShortBuffer;
-import java.util.Arrays;
+import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.function.Consumer;
@@ -90,25 +93,20 @@
AppView<?> appView, ProgramMethod method, DexType superType) {
DexEncodedMethod definition = method.getDefinition();
assert definition.getCode().isDefaultInstanceInitializerCode();
+ Position position = method.getDefinition().getAndClearPendingInlineFrameAsPosition();
+ Code newCode;
if (appView.testing().isSupportedLirPhase()) {
- method.setCode(get().toLirCode(appView, method, superType), appView);
+ newCode = get().toLirCode(appView, method, superType, position);
} else {
assert appView.testing().isPreLirPhase();
- method.setCode(get().toCfCode(method, appView.dexItemFactory(), superType), appView);
+ newCode = get().toCfCode(method, appView.dexItemFactory(), superType, position);
}
+ method.setCode(newCode, appView);
}
@Override
- public Code getCodeAsInlining(
- DexMethod caller,
- boolean isCallerD8R8Synthesized,
- DexMethod callee,
- boolean isCalleeD8R8Synthesized,
- DexItemFactory factory) {
- // TODO(b/261971803): It is odd this code does not have an original position for the <init>.
- // See OverrideParentCollisionTest for a case hitting this inlining where callee is a
- // non-synthetic non-default <init> (it has argument String).
- return this;
+ public boolean supportsPendingInlineFrame() {
+ return true;
}
private static boolean hasDefaultInstanceInitializerCode(
@@ -168,7 +166,9 @@
MutableMethodConversionOptions conversionOptions) {
DefaultInstanceInitializerSourceCode source =
new DefaultInstanceInitializerSourceCode(
- method.getReference(), method.getDefinition().isD8R8Synthesized());
+ method.getReference(),
+ method.getDefinition().isD8R8Synthesized(),
+ method.getDefinition().getPendingInlineFrameAsPosition());
return IRBuilder.create(method, appView, source).build(method, conversionOptions);
}
@@ -183,7 +183,11 @@
RewrittenPrototypeDescription protoChanges) {
DefaultInstanceInitializerSourceCode source =
new DefaultInstanceInitializerSourceCode(
- method.getReference(), method.getDefinition().isD8R8Synthesized(), callerPosition);
+ method.getReference(),
+ method.getDefinition().isD8R8Synthesized(),
+ method
+ .getDefinition()
+ .getPendingInlineFrameAsPositionWithCallerPosition(callerPosition));
return IRBuilder.createForInlining(
method, appView, codeLens, source, valueNumberGenerator, protoChanges)
.build(context, MethodConversionOptions.nonConverting());
@@ -364,21 +368,24 @@
// Intentionally empty. This piece of code does not have any call sites.
}
- public CfCode toCfCode(ProgramMethod method, DexItemFactory dexItemFactory) {
- return toCfCode(method, dexItemFactory, method.getHolder().getSuperType());
- }
-
- public CfCode toCfCode(ProgramMethod method, DexItemFactory dexItemFactory, DexType supertype) {
- List<CfInstruction> instructions =
- Arrays.asList(
- new CfLoad(ValueType.OBJECT, 0),
- new CfInvoke(
- Opcodes.INVOKESPECIAL, dexItemFactory.createInstanceInitializer(supertype), false),
- new CfReturnVoid());
+ public CfCode toCfCode(
+ ProgramMethod method, DexItemFactory dexItemFactory, DexType supertype, Position position) {
+ List<CfInstruction> instructions = new ArrayList<>(position != null ? 5 : 3);
+ if (position != null) {
+ CfLabel entryLabel = new CfLabel();
+ instructions.add(entryLabel);
+ instructions.add(new CfPosition(entryLabel, position));
+ }
+ instructions.add(new CfLoad(ValueType.OBJECT, 0));
+ instructions.add(
+ new CfInvoke(
+ Opcodes.INVOKESPECIAL, dexItemFactory.createInstanceInitializer(supertype), false));
+ instructions.add(new CfReturnVoid());
return new CfCode(method.getHolderType(), getMaxStack(), getMaxLocals(method), instructions);
}
- public LirCode<?> toLirCode(AppView<?> appView, ProgramMethod method, DexType supertype) {
+ public LirCode<?> toLirCode(
+ AppView<?> appView, ProgramMethod method, DexType supertype, Position position) {
TypeElement receiverType =
method.getHolder().getType().toTypeElement(appView, Nullability.definitelyNotNull());
Value receiver = new Value(0, receiverType, null);
@@ -386,11 +393,16 @@
LirEncodingStrategy<Value, Integer> strategy =
LirStrategy.getDefaultStrategy().getEncodingStrategy();
strategy.defineValue(receiver, 0);
- return LirCode.builder(
+ LirBuilder<Value, Integer> builder =
+ LirCode.builder(
method.getReference(),
method.getDefinition().isD8R8Synthesized(),
strategy,
- appView.options())
+ appView.options());
+ if (position != null) {
+ builder.setCurrentPosition(position);
+ }
+ return builder
.addArgument(0, false)
.addInvokeDirect(invokedMethod, ImmutableList.of(receiver), false)
.addReturnVoid()
@@ -457,10 +469,6 @@
static class DefaultInstanceInitializerSourceCode extends SyntheticStraightLineSourceCode {
- DefaultInstanceInitializerSourceCode(DexMethod method, boolean isD8R8Synthesized) {
- this(method, isD8R8Synthesized, null);
- }
-
DefaultInstanceInitializerSourceCode(
DexMethod method, boolean isD8R8Synthesized, Position callerPosition) {
super(getInstructionBuilders(), getPosition(method, isD8R8Synthesized, callerPosition));
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index e71bb97..7c95315 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -38,6 +38,7 @@
import com.android.tools.r8.graph.proto.ArgumentInfoCollection;
import com.android.tools.r8.ir.analysis.type.TypeElement;
import com.android.tools.r8.ir.code.NumericType;
+import com.android.tools.r8.ir.code.Position;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.ir.code.ValueType;
import com.android.tools.r8.ir.optimize.Inliner.ConstraintWithTarget;
@@ -57,8 +58,10 @@
import com.android.tools.r8.naming.MemberNaming.Signature;
import com.android.tools.r8.naming.NamingLens;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.utils.BooleanBox;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.ConsumerUtils;
+import com.android.tools.r8.utils.ObjectUtils;
import com.android.tools.r8.utils.OptionalBool;
import com.android.tools.r8.utils.RetracerForCodePrinting;
import com.android.tools.r8.utils.structural.CompareToVisitor;
@@ -129,6 +132,7 @@
DexAnnotationSet.empty(),
ParameterAnnotationsList.empty(),
null,
+ null,
false,
ComputedApiLevel.notSet(),
ComputedApiLevel.notSet(),
@@ -142,6 +146,7 @@
public final boolean deprecated;
public ParameterAnnotationsList parameterAnnotationsList;
private Code code;
+ private DexMethod pendingInlineFrame;
// TODO(b/128967328): towards finer-grained inlining constraints,
// we need to maintain a set of states with (potentially different) contexts.
private CompilationState compilationState = CompilationState.NOT_PROCESSED;
@@ -205,20 +210,6 @@
return compilationState;
}
- /**
- * Flags this method as no longer being obsolete.
- *
- * <p>Example use case: The vertical class merger optimistically merges two classes before it is
- * guaranteed that the two classes can be merged. In this process, methods are moved from the
- * source class to the target class using {@link #toTypeSubstitutedMethodAsInlining(DexMethod,
- * DexItemFactory)}, which causes the original methods of the source class to become obsolete. If
- * vertical class merging is aborted, the original methods of the source class needs to be marked
- * as not being obsolete.
- */
- public void unsetObsolete() {
- obsolete = false;
- }
-
private DexEncodedMethod(
DexMethod method,
MethodAccessFlags accessFlags,
@@ -226,6 +217,7 @@
DexAnnotationSet annotations,
ParameterAnnotationsList parameterAnnotationsList,
Code code,
+ DexMethod pendingInlineFrame,
boolean d8R8Synthesized,
ComputedApiLevel apiLevelForDefinition,
ComputedApiLevel apiLevelForCode,
@@ -238,11 +230,13 @@
this.genericSignature = genericSignature;
this.parameterAnnotationsList = parameterAnnotationsList;
this.code = code;
+ this.pendingInlineFrame = pendingInlineFrame;
this.classFileVersion = classFileVersion;
this.apiLevelForCode = apiLevelForCode;
this.optimizationInfo = requireNonNull(optimizationInfo);
assert accessFlags != null;
assert code == null || !shouldNotHaveCode();
+ assert supportsPendingInlineFrame() || !hasPendingInlineFrame();
assert parameterAnnotationsList != null;
assert apiLevelForDefinition != null;
assert apiLevelForCode != null;
@@ -275,6 +269,7 @@
.withItem(m -> m.parameterAnnotationsList)
.withNullableItem(m -> m.classFileVersion)
.withBool(DexEncodedMember::isD8R8Synthesized)
+ .withNullableItem(m -> m.pendingInlineFrame)
// TODO(b/171867022): Make signatures structural and include it in the definition.
.withAssert(m -> m.genericSignature.hasNoSignature())
.withCustomItem(
@@ -747,10 +742,38 @@
public void setCode(Code code, Int2ReferenceMap<DebugLocalInfo> parameterInfo) {
checkIfObsolete();
+ if (code != null && !code.supportsPendingInlineFrame() && hasPendingInlineFrame()) {
+ // If the method is set abstract, or the code supports a pending inline frame, then it simply
+ // remains in place. Note that if we start supporting pending frames on instruction-code
+ // objects this implicit keeping of the pending frame would not be correct.
+ assert verifyPendingInlinePositionFoundInCode(code);
+ pendingInlineFrame = null;
+ }
this.code = code;
this.parameterInfo = parameterInfo;
}
+ private boolean verifyPendingInlinePositionFoundInCode(Code code) {
+ BooleanBox foundInlineFrame = new BooleanBox(false);
+ code.forEachPosition(
+ getReference(),
+ isDexEncodedMethod(),
+ position -> {
+ if (foundInlineFrame.isTrue()) {
+ return;
+ }
+ do {
+ if (!position.isD8R8Synthesized() && position.getMethod().equals(pendingInlineFrame)) {
+ foundInlineFrame.set(true);
+ break;
+ }
+ position = position.getCallerPosition();
+ } while (position != null);
+ });
+ assert foundInlineFrame.isTrue();
+ return true;
+ }
+
public void unsetCode() {
checkIfObsolete();
code = null;
@@ -787,6 +810,50 @@
return accessFlags.isAbstract() || accessFlags.isNative();
}
+ public boolean supportsPendingInlineFrame() {
+ return code == null || code.supportsPendingInlineFrame();
+ }
+
+ public boolean hasPendingInlineFrame() {
+ return pendingInlineFrame != null;
+ }
+
+ public DexMethod getPendingInlineFrame() {
+ return pendingInlineFrame;
+ }
+
+ public Position getAndClearPendingInlineFrameAsPosition() {
+ Position position = getPendingInlineFrameAsPosition();
+ pendingInlineFrame = null;
+ return position;
+ }
+
+ public Position getPendingInlineFrameAsPosition() {
+ if (!hasPendingInlineFrame()) {
+ return null;
+ }
+ // This method is the caller method and must be synthetic.
+ assert isD8R8Synthesized();
+ return getPendingInlineFrameAsPositionWithCallerPosition(
+ Position.SyntheticPosition.builder()
+ .setMethod(getReference())
+ .setIsD8R8Synthesized(isD8R8Synthesized())
+ .setLine(0)
+ .build());
+ }
+
+ public Position getPendingInlineFrameAsPositionWithCallerPosition(Position callerPosition) {
+ assert callerPosition != null;
+ if (!hasPendingInlineFrame()) {
+ return callerPosition;
+ }
+ return Position.SyntheticPosition.builder()
+ .setLine(0)
+ .setMethod(getPendingInlineFrame())
+ .setCallerPosition(callerPosition)
+ .build();
+ }
+
public boolean hasCode() {
return code != null;
}
@@ -1102,7 +1169,10 @@
method,
isCallerD8R8Synthesized,
builder -> {
- if (code != null) {
+ if (code == null || code.supportsPendingInlineFrame()) {
+ builder.setInlineFrame(getReference(), isD8R8Synthesized());
+ } else {
+ // TODO(b/302509457): Use prepend inline frame here too and delay code rewriting.
builder.setCode(
getCode()
.getCodeAsInlining(
@@ -1113,7 +1183,9 @@
factory));
}
if (consumer != null) {
+ Code oldCode = builder.code;
consumer.accept(builder);
+ assert ObjectUtils.identical(oldCode, builder.code);
}
});
}
@@ -1389,6 +1461,7 @@
private ComputedApiLevel apiLevelForCode = ComputedApiLevel.notSet();
private final boolean d8R8Synthesized;
private boolean deprecated = false;
+ private DexMethod pendingInlineFrame = null;
// Checks to impose on the built method. They should always be active to start with and be
// lowered on the use site.
@@ -1409,6 +1482,7 @@
genericSignature = from.getGenericSignature();
annotations = from.annotations();
code = from.getCode();
+ pendingInlineFrame = from.getPendingInlineFrame();
apiLevelForDefinition = from.getApiLevelForDefinition();
apiLevelForCode = from.getApiLevelForCode();
optimizationInfo =
@@ -1608,6 +1682,18 @@
});
}
+ public Builder setInlineFrame(DexMethod callee, boolean calleeIsD8R8Synthesized) {
+ assert code == null || code.supportsPendingInlineFrame();
+ if (calleeIsD8R8Synthesized) {
+ // No need to record a compiler synthesized inline frame.
+ return this;
+ }
+ // Two pending non-synthetic callee frames should never happen.
+ assert pendingInlineFrame == null;
+ pendingInlineFrame = callee;
+ return this;
+ }
+
public Builder setCode(Code code) {
this.code = code;
return this;
@@ -1672,6 +1758,7 @@
|| parameterAnnotations.size() == method.proto.parameters.size();
assert !checkAndroidApiLevels || apiLevelForDefinition != null;
assert !checkAndroidApiLevels || apiLevelForCode != null;
+ assert code == null || code.supportsPendingInlineFrame() || pendingInlineFrame == null;
DexEncodedMethod result =
new DexEncodedMethod(
method,
@@ -1680,6 +1767,7 @@
annotations,
parameterAnnotations,
code,
+ pendingInlineFrame,
d8R8Synthesized,
apiLevelForDefinition,
apiLevelForCode,
diff --git a/src/main/java/com/android/tools/r8/graph/ThrowExceptionCode.java b/src/main/java/com/android/tools/r8/graph/ThrowExceptionCode.java
index 903796b..45bdf68 100644
--- a/src/main/java/com/android/tools/r8/graph/ThrowExceptionCode.java
+++ b/src/main/java/com/android/tools/r8/graph/ThrowExceptionCode.java
@@ -49,6 +49,11 @@
}
@Override
+ public boolean supportsPendingInlineFrame() {
+ return true;
+ }
+
+ @Override
public IRCode buildIR(
ProgramMethod method,
AppView<?> appView,
diff --git a/src/main/java/com/android/tools/r8/graph/ThrowNullCode.java b/src/main/java/com/android/tools/r8/graph/ThrowNullCode.java
index af96367..8bdd8b6 100644
--- a/src/main/java/com/android/tools/r8/graph/ThrowNullCode.java
+++ b/src/main/java/com/android/tools/r8/graph/ThrowNullCode.java
@@ -54,15 +54,8 @@
}
@Override
- public Code getCodeAsInlining(
- DexMethod caller,
- boolean isCallerD8R8Synthesized,
- DexMethod callee,
- boolean isCalleeD8R8Synthesized,
- DexItemFactory factory) {
- // We don't maintain a position on the throwing stub. We may want to reconsider this as it
- // would allow retracing to recover inlinings of this stub.
- return this;
+ public boolean supportsPendingInlineFrame() {
+ return true;
}
@Override
@@ -70,7 +63,8 @@
ProgramMethod method,
AppView<?> appView,
MutableMethodConversionOptions conversionOptions) {
- ThrowNullSourceCode source = new ThrowNullSourceCode(method);
+ ThrowNullSourceCode source =
+ new ThrowNullSourceCode(method, method.getDefinition().getPendingInlineFrameAsPosition());
return IRBuilder.create(method, appView, source).build(method, conversionOptions);
}
@@ -83,7 +77,12 @@
NumberGenerator valueNumberGenerator,
Position callerPosition,
RewrittenPrototypeDescription protoChanges) {
- ThrowNullSourceCode source = new ThrowNullSourceCode(method, callerPosition);
+ ThrowNullSourceCode source =
+ new ThrowNullSourceCode(
+ method,
+ method
+ .getDefinition()
+ .getPendingInlineFrameAsPositionWithCallerPosition(callerPosition));
return IRBuilder.createForInlining(
method, appView, codeLens, source, valueNumberGenerator, protoChanges)
.build(context, MethodConversionOptions.nonConverting());
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
index e14f91d..ebf89d8 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingTreeFixer.java
@@ -951,25 +951,27 @@
LocalEnumUnboxingUtilityClass localUtilityClass,
Predicate<DexMethod> availableMethodSignatures) {
assert method.getAccessFlags().isAbstract();
- DexMethod newMethod =
- createFreshMethodSignature(
- method, localUtilityClass, availableMethodSignatures, method.getReference());
+ // Replace the code by a throwing stub and then rewrite the method as "inlining".
+ Position position = method.getDefinition().getAndClearPendingInlineFrameAsPosition();
+ method.setCode(
+ new ThrowCfCodeProvider(
+ appView, localUtilityClass.getType(), factory.abstractMethodErrorType, position)
+ .generateCfCode(),
+ appView);
DexEncodedMethod dexEncodedMethod =
method
.getDefinition()
.toTypeSubstitutedMethodAsInlining(
- newMethod,
+ createFreshMethodSignature(
+ method, localUtilityClass, availableMethodSignatures, method.getReference()),
factory,
builder ->
transformMethodForLocalUtility(builder, method)
.modifyAccessFlags(MethodAccessFlags::unsetAbstract)
- .setCode(
- new ThrowCfCodeProvider(
- appView, newMethod, factory.abstractMethodErrorType)
- .generateCfCode())
.setClassFileVersion(CfVersion.V1_8)
.setApiLevelForDefinition(appView.computedMinApiLevel())
.setApiLevelForCode(appView.computedMinApiLevel()));
+
methodsToProcess.add(new ProgramMethod(localUtilityClass.getDefinition(), dexEncodedMethod));
return dexEncodedMethod;
}
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/ThrowCfCodeProvider.java b/src/main/java/com/android/tools/r8/ir/synthetic/ThrowCfCodeProvider.java
index ff9609d..d6f9242 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/ThrowCfCodeProvider.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/ThrowCfCodeProvider.java
@@ -7,7 +7,9 @@
import com.android.tools.r8.cf.code.CfInstruction;
import com.android.tools.r8.cf.code.CfInvoke;
+import com.android.tools.r8.cf.code.CfLabel;
import com.android.tools.r8.cf.code.CfNew;
+import com.android.tools.r8.cf.code.CfPosition;
import com.android.tools.r8.cf.code.CfStackInstruction;
import com.android.tools.r8.cf.code.CfStackInstruction.Opcode;
import com.android.tools.r8.cf.code.CfThrow;
@@ -15,25 +17,34 @@
import com.android.tools.r8.graph.CfCode;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.ir.code.Position;
import java.util.ArrayList;
import java.util.List;
/**
- * Generates a method that just throws a exception with empty <init></init> with *any* signature
- * passed, so the method can be inserted in a hierarchy and be called with normal virtual dispatch.
+ * Generates a method that just throws an exception with empty <init> with *any* signature passed,
+ * so the method can be inserted in a hierarchy and be called with normal virtual dispatch.
*/
public class ThrowCfCodeProvider extends SyntheticCfCodeProvider {
private final DexType exceptionType;
+ private final Position position;
- public ThrowCfCodeProvider(AppView<?> appView, DexMethod method, DexType exceptionType) {
- super(appView, method.getHolderType());
+ public ThrowCfCodeProvider(
+ AppView<?> appView, DexType holder, DexType exceptionType, Position position) {
+ super(appView, holder);
this.exceptionType = exceptionType;
+ this.position = position;
}
@Override
public CfCode generateCfCode() {
List<CfInstruction> instructions = new ArrayList<>();
+ if (position != null) {
+ CfLabel entryLabel = new CfLabel();
+ instructions.add(entryLabel);
+ instructions.add(new CfPosition(entryLabel, position));
+ }
instructions.add(new CfNew(exceptionType));
instructions.add(new CfStackInstruction(Opcode.Dup));
DexMethod init = appView.dexItemFactory().createInstanceInitializer(exceptionType);
diff --git a/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java b/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
index 80e22b5..3da079c 100644
--- a/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
+++ b/src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
@@ -225,31 +225,58 @@
PositionRemapper positionRemapper,
boolean canUseDexPc) {
DexEncodedMethod definition = method.getDefinition();
-
- OneShotCollectionConsumer<MappingInformation> methodSpecificMappingInformation =
- OneShotCollectionConsumer.wrap(new ArrayList<>());
- // We only do global synthetic classes when using names from the library. For such classes it
- // is important that we do not filter out stack frames since they could appear from concrete
- // classes in the library. Additionally, this is one place where it is helpful for developers
- // to also get reported synthesized frames since stubbing can change control-flow and
- // exceptions.
- boolean canStripOuterFrame = canStripOuterFrame(method, mappedPositions);
- boolean residualIsD8R8Synthesized =
- method.getDefinition().isD8R8Synthesized()
- && !appView.getSyntheticItems().isGlobalSyntheticClass(method.getHolder())
- // TODO(b/302509457): Currently we can only represent moves on methods that have code
- // and thus positions. For methods with no code, use the lens to find the original.
- && method.getDefinition().hasCode();
- if (residualIsD8R8Synthesized && !canStripOuterFrame) {
- methodSpecificMappingInformation.add(CompilerSynthesizedMappingInformation.getInstance());
- }
-
DexMethod residualMethod =
appView.getNamingLens().lookupMethod(method.getReference(), appView.dexItemFactory());
MethodSignature residualSignature = MethodSignature.fromDexMethod(residualMethod);
- DexMethod originalMethod =
+ // TODO(b/261971803): This original method via lens is just to assert compatibility with the
+ // previous implementation. Remove this as part of cleaning-up / reducing lens usage.
+ DexMethod lensOriginalMethod =
appView.graphLens().getOriginalMethodSignatureForMapping(method.getReference());
+
+ DexMethod originalMethod;
+ boolean canStripOuterFrame;
+ boolean residualIsD8R8Synthesized;
+ // TODO(b/325913397): Use residual methods. This requires some test updates to not use names.
+ boolean useResidualForSynthetics = false;
+ if (!definition.supportsPendingInlineFrame()) {
+ residualIsD8R8Synthesized = markAsCompilerSynthetic(method);
+ canStripOuterFrame = canStripOuterFrame(method, mappedPositions);
+ // TODO(b/261971803): Amend code generation of global synthetics to include original
+ // position as an inline frame and remove this special handling of globals.
+ originalMethod =
+ isGlobalSyntheticMethod(method)
+ ? lensOriginalMethod
+ : (useResidualForSynthetics && residualIsD8R8Synthesized
+ ? residualMethod
+ : method.getReference());
+ } else {
+ assert mappedPositions.isEmpty();
+ canStripOuterFrame = false;
+ if (definition.hasPendingInlineFrame()) {
+ originalMethod = definition.getPendingInlineFrame();
+ residualIsD8R8Synthesized = false;
+ } else {
+ residualIsD8R8Synthesized = markAsCompilerSynthetic(method);
+ // TODO(b/261971803): Amend code generation of global synthetics to include original
+ // position as an inline frame and remove this special handling of globals.
+ originalMethod =
+ isGlobalSyntheticMethod(method)
+ ? lensOriginalMethod
+ : (useResidualForSynthetics && residualIsD8R8Synthesized
+ ? residualMethod
+ : definition.getReference());
+ }
+ }
+ assert residualIsD8R8Synthesized || originalMethod.isIdenticalTo(lensOriginalMethod);
+
+ OneShotCollectionConsumer<MappingInformation> methodSpecificMappingInformation =
+ OneShotCollectionConsumer.wrap(new ArrayList<>());
+
+ if (residualIsD8R8Synthesized && !canStripOuterFrame) {
+ methodSpecificMappingInformation.add(CompilerSynthesizedMappingInformation.getInstance());
+ }
+
MethodSignature originalSignature =
MethodSignature.fromDexMethod(originalMethod, originalMethod.holder != originalType);
@@ -444,6 +471,20 @@
return this;
}
+ private boolean isGlobalSyntheticMethod(ProgramMethod method) {
+ return method.getDefinition().isD8R8Synthesized()
+ && appView.getSyntheticItems().isGlobalSyntheticClass(method.getHolder());
+ }
+
+ private boolean markAsCompilerSynthetic(ProgramMethod method) {
+ // We only do global synthetic classes when using names from the library. For such classes it
+ // is important that we do not filter out stack frames since they could appear from concrete
+ // classes in the library. Additionally, this is one place where it is helpful for developers
+ // to also get reported synthesized frames since stubbing can change control-flow and
+ // exceptions.
+ return method.getDefinition().isD8R8Synthesized() && !isGlobalSyntheticMethod(method);
+ }
+
private boolean canStripOuterFrame(ProgramMethod method, List<MappedPosition> mappedPositions) {
assert verifySyntheticPositions(method, mappedPositions);
if (!method.getDefinition().isD8R8Synthesized() || mappedPositions.isEmpty()) {
@@ -452,7 +493,7 @@
for (MappedPosition mappedPosition : mappedPositions) {
Position position = mappedPosition.getPosition();
if (!position.hasCallerPosition()) {
- // At least one position only has the synthetic method as its frame, so we can't strip it.
+ // At least one position has only the synthetic method as its frame, so we can't strip it.
return false;
}
if (position.isOutline() || position.isOutlineCaller()) {
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideParentCollisionTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideParentCollisionTest.java
index f819541..1f593bf 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideParentCollisionTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/dispatch/OverrideParentCollisionTest.java
@@ -5,11 +5,13 @@
package com.android.tools.r8.classmerging.horizontal.dispatch;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsNot.not;
import com.android.tools.r8.NeverClassInline;
import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.R8TestCompileResult;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.classmerging.horizontal.HorizontalClassMergingTestBase;
import org.junit.Test;
@@ -28,6 +30,8 @@
.enableInliningAnnotations()
.enableNeverClassInliningAnnotations()
.setMinApi(parameters)
+ .compile()
+ .apply(this::checkMappingOutput)
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("foo", "bar", "foo", "parent")
.inspect(
@@ -37,6 +41,12 @@
});
}
+ private void checkMappingOutput(R8TestCompileResult result) {
+ // Merging of Parent, A and B should still emit a stack trace mapping back to the Parent frame.
+ // That frame will be encoded as a qualified method mapping.
+ assertThat(result.getProguardMap(), containsString(Parent.class.getTypeName() + ".<init>()"));
+ }
+
@NeverClassInline
public static class Parent {
@NeverInline
