Skip to content

Commit d850801

Browse files
alexmarkovcommit-bot@chromium.org
authored andcommitted
[vm] Cleanup code related to interface calls through fields/getters from TFA and bytecode generator
Since https://dart-review.googlesource.com/c/sdk/+/129702 CFE desugars interface calls through fields/getters, so there is no need to handle them in the TFA and bytecode generator. Note that VM can still see such calls if they are dynamic, so handling of such calls is not removed entirely from TFA or VM. Also, VM should support those as long as it supports older kernel binary versions. Issue: #34497 Change-Id: Ic49f109b0e9264f0e20a7e1a3b7a46011fa76c86 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/137700 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
1 parent 18f366c commit d850801

File tree

4 files changed

+19
-103
lines changed

4 files changed

+19
-103
lines changed

pkg/vm/lib/bytecode/gen_bytecode.dart

+8-56
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ import 'generics.dart'
4242
hasFreeTypeParameters,
4343
hasInstantiatorTypeArguments,
4444
isAllDynamic,
45-
isCallThroughGetter,
4645
isInstantiatedInterfaceCall,
4746
isUncheckedCall,
4847
isUncheckedClosureCall;
@@ -3362,57 +3361,6 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
33623361
}
33633362
}
33643363

3365-
/// Generates bytecode for o.foo(a0, ..., aN) call where o.foo is a
3366-
/// field or getter.
3367-
void _genCallThroughGetter(MethodInvocation node, int totalArgCount) {
3368-
final arguments = node.arguments;
3369-
if (arguments.types.isNotEmpty) {
3370-
_genTypeArguments(arguments.types);
3371-
}
3372-
3373-
// Keep order of evaluation: o, a0, .., aN, o.foo, o.foo.call(a0, ..., aN).
3374-
final receiver = node.receiver;
3375-
_generateNode(receiver);
3376-
final receiverTemp = locals.tempIndexInFrame(node, tempIndex: 0);
3377-
final checkForNull =
3378-
node.interfaceTarget.enclosingClass != coreTypes.objectClass;
3379-
if (checkForNull) {
3380-
asm.emitStoreLocal(receiverTemp);
3381-
}
3382-
3383-
int count = 0;
3384-
for (var arg in arguments.positional) {
3385-
_generateNode(arg);
3386-
asm.emitPopLocal(locals.tempIndexInFrame(node, tempIndex: count + 1));
3387-
++count;
3388-
}
3389-
for (var arg in arguments.named) {
3390-
_generateNode(arg.value);
3391-
asm.emitPopLocal(locals.tempIndexInFrame(node, tempIndex: count + 1));
3392-
++count;
3393-
}
3394-
3395-
// Check receiver for null before calling getter to report
3396-
// correct noSuchMethod for method call.
3397-
if (checkForNull) {
3398-
asm.emitPush(receiverTemp);
3399-
asm.emitNullCheck(cp.addSelectorName(node.name, InvocationKind.method));
3400-
}
3401-
3402-
_genInstanceCall(null, InvocationKind.getter, node.interfaceTarget,
3403-
node.name, receiver, 1, objectTable.getArgDescHandle(1));
3404-
3405-
for (int i = 0; i < count; ++i) {
3406-
asm.emitPush(locals.tempIndexInFrame(node, tempIndex: i + 1));
3407-
}
3408-
3409-
final argDesc =
3410-
objectTable.getArgDescHandleByArguments(arguments, hasReceiver: true);
3411-
3412-
_genInstanceCall(node, InvocationKind.method, null, callName, null,
3413-
totalArgCount, argDesc);
3414-
}
3415-
34163364
@override
34173365
visitMethodInvocation(MethodInvocation node) {
34183366
final directCall =
@@ -3443,10 +3391,10 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
34433391
}
34443392

34453393
final Member interfaceTarget = node.interfaceTarget;
3446-
if (isCallThroughGetter(interfaceTarget)) {
3447-
assert(directCall == null);
3448-
_genCallThroughGetter(node, totalArgCount);
3449-
return;
3394+
if (!(interfaceTarget == null ||
3395+
interfaceTarget is Procedure && !interfaceTarget.isGetter)) {
3396+
throw new UnsupportedOperationError(
3397+
'Unsupported MethodInvocation with interface target ${interfaceTarget.runtimeType} $interfaceTarget');
34503398
}
34513399

34523400
if (directCall != null && directCall.checkReceiverForNull) {
@@ -3559,6 +3507,10 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
35593507
..addAll(args.named.map((x) => x.value)));
35603508
return;
35613509
}
3510+
if (!(target is Procedure && !target.isGetter)) {
3511+
throw new UnsupportedOperationError(
3512+
'Unsupported SuperMethodInvocation with target ${target.runtimeType} $target');
3513+
}
35623514
_genArguments(new ThisExpression(), args);
35633515
_genDirectCallWithArgs(target, args,
35643516
hasReceiver: true, isUnchecked: true, node: node);

pkg/vm/lib/bytecode/generics.dart

-6
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,3 @@ bool isUncheckedClosureCall(MethodInvocation node,
299299
node.name.name == 'call' &&
300300
getStaticType(node.receiver, staticTypeContext) is FunctionType &&
301301
!options.avoidClosureCallInstructions;
302-
303-
/// Returns true if [MethodInvocation] node with given [interfaceTarget] is
304-
/// a call through field or getter.
305-
bool isCallThroughGetter(Member interfaceTarget) =>
306-
interfaceTarget is Field ||
307-
interfaceTarget is Procedure && interfaceTarget.isGetter;

pkg/vm/lib/bytecode/local_vars.dart

-3
Original file line numberDiff line numberDiff line change
@@ -1301,9 +1301,6 @@ class _Allocator extends RecursiveVisitor<Null> {
13011301
if (isUncheckedClosureCall(
13021302
node, locals.staticTypeContext, locals.options)) {
13031303
numTemps = 1;
1304-
} else if (isCallThroughGetter(node.interfaceTarget)) {
1305-
final args = node.arguments;
1306-
numTemps = 1 + args.positional.length + args.named.length;
13071304
} else if (locals.directCallMetadata != null) {
13081305
final directCall = locals.directCallMetadata[node];
13091306
if (directCall != null && directCall.checkReceiverForNull) {

pkg/vm/lib/transformations/type_flow/summary_collector.dart

+11-38
Original file line numberDiff line numberDiff line change
@@ -1581,29 +1581,14 @@ class SummaryCollector extends RecursiveVisitor<TypeExpr> {
15811581
result = _makeCall(
15821582
node, new DynamicSelector(CallKind.Method, node.name), args);
15831583
} else {
1584-
// TODO(dartbug.com/34497): Once front-end desugars calls via
1585-
// fields/getters, handling of field and getter targets here
1586-
// can be turned into assertions.
1587-
if ((target is Field) || ((target is Procedure) && target.isGetter)) {
1588-
// Call via field/getter.
1589-
final value = _makeCall(
1590-
null,
1591-
(receiverNode is ThisExpression)
1592-
? new VirtualSelector(target, callKind: CallKind.PropertyGet)
1593-
: new InterfaceSelector(target, callKind: CallKind.PropertyGet),
1594-
new Args<TypeExpr>([receiver]));
1595-
_makeCall(
1596-
null, DynamicSelector.kCall, new Args.withReceiver(args, value));
1597-
result = _staticType(node);
1598-
} else {
1599-
// TODO(alexmarkov): overloaded arithmetic operators
1600-
result = _makeCall(
1601-
node,
1602-
(node.receiver is ThisExpression)
1603-
? new VirtualSelector(target)
1604-
: new InterfaceSelector(target),
1605-
args);
1606-
}
1584+
assertx(target is Procedure && !target.isGetter);
1585+
// TODO(alexmarkov): overloaded arithmetic operators
1586+
result = _makeCall(
1587+
node,
1588+
(node.receiver is ThisExpression)
1589+
? new VirtualSelector(target)
1590+
: new InterfaceSelector(target),
1591+
args);
16071592
}
16081593
_updateReceiverAfterCall(receiverNode, receiver, node.name);
16091594
return result;
@@ -1681,21 +1666,9 @@ class SummaryCollector extends RecursiveVisitor<TypeExpr> {
16811666
if (target == null) {
16821667
return const EmptyType();
16831668
} else {
1684-
if ((target is Field) || ((target is Procedure) && target.isGetter)) {
1685-
// Call via field/getter.
1686-
// TODO(alexmarkov): Consider cleaning up this code as it duplicates
1687-
// processing in DirectInvocation.
1688-
final fieldValue = _makeCall(
1689-
node,
1690-
new DirectSelector(target, callKind: CallKind.PropertyGet),
1691-
new Args<TypeExpr>([_receiver]));
1692-
_makeCall(null, DynamicSelector.kCall,
1693-
new Args.withReceiver(args, fieldValue));
1694-
return _staticType(node);
1695-
} else {
1696-
_entryPointsListener.recordMemberCalledViaThis(target);
1697-
return _makeCall(node, new DirectSelector(target), args);
1698-
}
1669+
assertx(target is Procedure && !target.isGetter);
1670+
_entryPointsListener.recordMemberCalledViaThis(target);
1671+
return _makeCall(node, new DirectSelector(target), args);
16991672
}
17001673
}
17011674

0 commit comments

Comments
 (0)