Skip to content

Commit b9d83b9

Browse files
nshahanCommit Queue
authored andcommitted
[ddc] Share some helpers methods
Moves helpers from local functions and instance methods in the compiler to static extensions on kernel nodes. These can be shared between the old and new compilers. Change-Id: Ifc2b5fe8945753823d7671171673c6c7add1e8cd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/439806 Reviewed-by: Mark Zhou <[email protected]> Reviewed-by: Nate Biggs <[email protected]>
1 parent eb4ebd3 commit b9d83b9

File tree

3 files changed

+127
-154
lines changed

3 files changed

+127
-154
lines changed

pkg/dev_compiler/lib/src/kernel/compiler.dart

Lines changed: 5 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -5575,8 +5575,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
55755575
return jsReceiver;
55765576
}
55775577
var memberName = node.name.text;
5578-
if (_isObjectGetter(memberName) &&
5579-
_shouldCallObjectMemberHelper(receiver)) {
5578+
if (member.isCoreObjectGetter && _shouldCallObjectMemberHelper(receiver)) {
55805579
// The names of the static helper methods in the runtime must match the
55815580
// names of the Object instance getters.
55825581
return _runtimeCall('#(#)', [memberName, jsReceiver]);
@@ -5623,7 +5622,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
56235622
return jsReceiver;
56245623
}
56255624
var memberName = node.name.text;
5626-
if (_isObjectMethodTearoff(memberName) &&
5625+
if (member.isToStringOrNoSuchMethod &&
56275626
_shouldCallObjectMemberHelper(receiver)) {
56285627
// The names of the static helper methods in the runtime must start with
56295628
// the names of the Object instance methods.
@@ -6012,48 +6011,11 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
60126011
}
60136012

60146013
js_ast.Expression _emitInstanceInvocation(InstanceInvocation node) {
6015-
/// Returns `true` when [node] represents an invocation of `List.add()` that
6016-
/// can be optimized.
6017-
///
6018-
/// The optimized add operation can skip checks for a growable or modifiable
6019-
/// list and the element type is known to be invariant so it can skip the
6020-
/// type check.
6021-
bool isNativeListInvariantAdd(InstanceInvocation node) {
6022-
if (node.isInvariant && node.name.text == 'add') {
6023-
// The call to add is marked as invariant, so the type check on the
6024-
// parameter to add is not needed.
6025-
var receiver = node.receiver;
6026-
if (receiver is VariableGet &&
6027-
receiver.variable.isFinal &&
6028-
!receiver.variable.isLate) {
6029-
// The receiver is a final variable, so it only contains the
6030-
// initializer value. Also, avoid late variables in case the CFE
6031-
// lowering of late variables is changed in the future.
6032-
var initializer = receiver.variable.initializer;
6033-
if (initializer is ListLiteral) {
6034-
// The initializer is a list literal, so we know the list can be
6035-
// grown, modified, and is represented by a JavaScript Array.
6036-
return true;
6037-
}
6038-
if (initializer is StaticInvocation &&
6039-
initializer.target.enclosingClass == _coreTypes.listClass &&
6040-
initializer.target.name.text == 'of' &&
6041-
initializer.arguments.named.isEmpty) {
6042-
// The initializer is a `List.of()` call from the dart:core library
6043-
// and the growable named argument has not been passed (it defaults
6044-
// to true).
6045-
return true;
6046-
}
6047-
}
6048-
}
6049-
return false;
6050-
}
6051-
60526014
var name = node.name.text;
60536015
var receiver = node.receiver;
60546016
var arguments = node.arguments;
60556017
var target = node.interfaceTarget;
6056-
if (isOperatorMethodName(name) && arguments.named.isEmpty) {
6018+
if (target.isPrimitiveOperator && arguments.named.isEmpty) {
60576019
var argLength = arguments.positional.length;
60586020
if (argLength == 0) {
60596021
return _emitUnaryOperator(receiver, target, node);
@@ -6068,7 +6030,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
60686030
}
60696031
var jsReceiver = _visitExpression(receiver);
60706032
var args = _emitArgumentList(arguments, target: target);
6071-
if (isNativeListInvariantAdd(node)) {
6033+
if (node.isNativeListInvariantAddInvocation(_coreTypes.listClass)) {
60726034
return js.call('#.push(#)', [jsReceiver, args]);
60736035
}
60746036
if (name == 'call') {
@@ -6084,7 +6046,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
60846046
return js_ast.Call(jsReceiver, args);
60856047
}
60866048
}
6087-
if (_isObjectMethodCall(name, arguments) &&
6049+
if (target.isToStringOrNoSuchMethodWithDefaultSignature &&
60886050
_shouldCallObjectMemberHelper(receiver)) {
60896051
// Handle Object methods that are supported by `null` and possibly
60906052
// JavaScript interop values with static helper methods.
@@ -9306,24 +9268,6 @@ bool _isObjectMember(String name) {
93069268
return false;
93079269
}
93089270

9309-
bool _isObjectGetter(String name) =>
9310-
name == 'hashCode' || name == 'runtimeType';
9311-
9312-
bool _isObjectMethodTearoff(String name) =>
9313-
// "==" isn't in here because there is no syntax to tear it off.
9314-
name == 'toString' || name == 'noSuchMethod';
9315-
9316-
bool _isObjectMethodCall(String name, Arguments args) {
9317-
if (name == 'toString') {
9318-
return args.positional.isEmpty && args.named.isEmpty && args.types.isEmpty;
9319-
} else if (name == 'noSuchMethod') {
9320-
return args.positional.length == 1 &&
9321-
args.named.isEmpty &&
9322-
args.types.isEmpty;
9323-
}
9324-
return false;
9325-
}
9326-
93279271
class _SwitchLabelState {
93289272
String label;
93299273
js_ast.Identifier variable;

pkg/dev_compiler/lib/src/kernel/compiler_new.dart

Lines changed: 6 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -2205,7 +2205,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
22052205

22062206
var memberName = _declareMemberName(
22072207
member,
2208-
useExtension: _isObjectMethodTearoff(member.name.text),
2208+
useExtension: member.isToStringOrNoSuchMethod,
22092209
);
22102210
if (!member.isAccessor) {
22112211
var immediateTarget = js.string(fullyResolvedTargetLabel(member));
@@ -6156,8 +6156,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
61566156
return jsReceiver;
61576157
}
61586158
var memberName = node.name.text;
6159-
if (_isObjectGetter(memberName) &&
6160-
_shouldCallObjectMemberHelper(receiver)) {
6159+
if (member.isCoreObjectGetter && _shouldCallObjectMemberHelper(receiver)) {
61616160
// The names of the static helper methods in the runtime must match the
61626161
// names of the Object instance getters.
61636162
return _runtimeCall('#(#)', [memberName, jsReceiver]);
@@ -6219,7 +6218,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
62196218
return jsReceiver;
62206219
}
62216220
var memberName = node.name.text;
6222-
if (_isObjectMethodTearoff(memberName) &&
6221+
if (member.isToStringOrNoSuchMethod &&
62236222
_shouldCallObjectMemberHelper(receiver)) {
62246223
// The names of the static helper methods in the runtime must start with
62256224
// the names of the Object instance methods.
@@ -6626,48 +6625,11 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
66266625
}
66276626

66286627
js_ast.Expression _emitInstanceInvocation(InstanceInvocation node) {
6629-
/// Returns `true` when [node] represents an invocation of `List.add()` that
6630-
/// can be optimized.
6631-
///
6632-
/// The optimized add operation can skip checks for a growable or modifiable
6633-
/// list and the element type is known to be invariant so it can skip the
6634-
/// type check.
6635-
bool isNativeListInvariantAdd(InstanceInvocation node) {
6636-
if (node.isInvariant && node.name.text == 'add') {
6637-
// The call to add is marked as invariant, so the type check on the
6638-
// parameter to add is not needed.
6639-
var receiver = node.receiver;
6640-
if (receiver is VariableGet &&
6641-
receiver.variable.isFinal &&
6642-
!receiver.variable.isLate) {
6643-
// The receiver is a final variable, so it only contains the
6644-
// initializer value. Also, avoid late variables in case the CFE
6645-
// lowering of late variables is changed in the future.
6646-
var initializer = receiver.variable.initializer;
6647-
if (initializer is ListLiteral) {
6648-
// The initializer is a list literal, so we know the list can be
6649-
// grown, modified, and is represented by a JavaScript Array.
6650-
return true;
6651-
}
6652-
if (initializer is StaticInvocation &&
6653-
initializer.target.enclosingClass == _coreTypes.listClass &&
6654-
initializer.target.name.text == 'of' &&
6655-
initializer.arguments.named.isEmpty) {
6656-
// The initializer is a `List.of()` call from the dart:core library
6657-
// and the growable named argument has not been passed (it defaults
6658-
// to true).
6659-
return true;
6660-
}
6661-
}
6662-
}
6663-
return false;
6664-
}
6665-
66666628
var name = node.name.text;
66676629
var receiver = node.receiver;
66686630
var arguments = node.arguments;
66696631
var target = node.interfaceTarget;
6670-
if (isOperatorMethodName(name) && arguments.named.isEmpty) {
6632+
if (target.isPrimitiveOperator && arguments.named.isEmpty) {
66716633
var argLength = arguments.positional.length;
66726634
if (argLength == 0) {
66736635
return _emitUnaryOperator(receiver, target, node);
@@ -6682,7 +6644,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
66826644
}
66836645
var jsReceiver = _visitExpression(receiver);
66846646
var args = _emitArgumentList(arguments, target: target);
6685-
if (isNativeListInvariantAdd(node)) {
6647+
if (node.isNativeListInvariantAddInvocation(_coreTypes.listClass)) {
66866648
return js.call('#.push(#)', [jsReceiver, args]);
66876649
}
66886650
if (name == 'call') {
@@ -6698,7 +6660,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
66986660
return js_ast.Call(jsReceiver, args);
66996661
}
67006662
}
6701-
if (_isObjectMethodCall(name, arguments) &&
6663+
if (target.isToStringOrNoSuchMethodWithDefaultSignature &&
67026664
_shouldCallObjectMemberHelper(receiver)) {
67036665
// Handle Object methods when the receiver could potentially be `null` or
67046666
// JavaScript interop values with static helper methods.
@@ -10018,24 +9980,6 @@ bool _isObjectMember(String name) {
100189980
return false;
100199981
}
100209982

10021-
bool _isObjectGetter(String name) =>
10022-
name == 'hashCode' || name == 'runtimeType';
10023-
10024-
bool _isObjectMethodTearoff(String name) =>
10025-
// "==" isn't in here because there is no syntax to tear it off.
10026-
name == 'toString' || name == 'noSuchMethod';
10027-
10028-
bool _isObjectMethodCall(String name, Arguments args) {
10029-
if (name == 'toString') {
10030-
return args.positional.isEmpty && args.named.isEmpty && args.types.isEmpty;
10031-
} else if (name == 'noSuchMethod') {
10032-
return args.positional.length == 1 &&
10033-
args.named.isEmpty &&
10034-
args.types.isEmpty;
10035-
}
10036-
return false;
10037-
}
10038-
100399983
class _SwitchLabelState {
100409984
String label;
100419985
js_ast.Identifier variable;

pkg/dev_compiler/lib/src/kernel/kernel_helpers.dart

Lines changed: 116 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -146,37 +146,6 @@ Class? getAnnotationClass(Expression node) {
146146
return null;
147147
}
148148

149-
/// Returns true if [name] is an operator method that is available on primitive
150-
/// types (`int`, `double`, `num`, `String`, `bool`).
151-
///
152-
/// This does not include logical operators that cannot be user-defined
153-
/// (`!`, `&&` and `||`).
154-
bool isOperatorMethodName(String name) {
155-
switch (name) {
156-
case '==':
157-
case '~':
158-
case '^':
159-
case '|':
160-
case '&':
161-
case '>>':
162-
case '<<':
163-
case '>>>':
164-
case '+':
165-
case 'unary-':
166-
case '-':
167-
case '*':
168-
case '/':
169-
case '~/':
170-
case '>':
171-
case '<':
172-
case '>=':
173-
case '<=':
174-
case '%':
175-
return true;
176-
}
177-
return false;
178-
}
179-
180149
bool isFromEnvironmentInvocation(CoreTypes coreTypes, StaticInvocation node) {
181150
var target = node.target;
182151
return node.isConst &&
@@ -626,3 +595,119 @@ class BasicInlineTester extends TreeVisitorDefault<bool> {
626595
return node.target.accept(this);
627596
}
628597
}
598+
599+
extension MemberHelpers on Member {
600+
/// Returns `true` if this is the `hashCode` or `runtimeType` getter.
601+
bool get isCoreObjectGetter =>
602+
isInstanceMember &&
603+
(name.text == 'hashCode' || name.text == 'runtimeType');
604+
}
605+
606+
extension ProcedureHelpers on Procedure {
607+
/// Returns `true` if [name] is an operator method that is available on
608+
/// primitive types (`int`, `double`, `num`, `String`, `bool`).
609+
///
610+
/// This does not include logical operators that cannot be user-defined
611+
/// (`!`, `&&` and `||`).
612+
bool get isPrimitiveOperator {
613+
switch (name.text) {
614+
case '==':
615+
case '~':
616+
case '^':
617+
case '|':
618+
case '&':
619+
case '>>':
620+
case '<<':
621+
case '>>>':
622+
case '+':
623+
case 'unary-':
624+
case '-':
625+
case '*':
626+
case '/':
627+
case '~/':
628+
case '>':
629+
case '<':
630+
case '>=':
631+
case '<=':
632+
case '%':
633+
return true;
634+
}
635+
return false;
636+
}
637+
638+
/// Returns `true` if this is any `toString` or `noSuchMethod` method
639+
/// implementation.
640+
///
641+
/// See [isToStringOrNoSuchMethodWithDefaultSignature] to test for a
642+
/// `toString` or `noSuchMethod` implementation with a default signature.
643+
bool get isToStringOrNoSuchMethod =>
644+
isInstanceMember &&
645+
(name.text == 'toString' || name.text == 'noSuchMethod');
646+
647+
/// Returns `true` if this is any `toString` or `noSuchMethod` method
648+
/// implementation with the default signature.
649+
///
650+
/// For example this will return `false` if this is an override with optional
651+
/// positional or named parameters.
652+
///
653+
/// See [isToStringOrNoSuchMethod] to test for any `toString` or
654+
/// `noSuchMethod` implementation.
655+
bool get isToStringOrNoSuchMethodWithDefaultSignature {
656+
if (function.namedParameters.isNotEmpty ||
657+
function.typeParameters.isNotEmpty) {
658+
return false;
659+
}
660+
if (name.text == 'toString') {
661+
return function.positionalParameters.isEmpty;
662+
} else if (name.text == 'noSuchMethod') {
663+
return function.positionalParameters.length == 1;
664+
}
665+
return false;
666+
}
667+
}
668+
669+
extension InstanceInvocationHelpers on InstanceInvocation {
670+
/// Returns `true` when this represents an invocation of `List.add()` that
671+
/// can be optimized.
672+
///
673+
/// For example, a List literal that uses control flow collections to add
674+
/// elements can safely push directly to the backing array:
675+
///
676+
/// ```
677+
/// final myList = [if (test) 1, ...otherList, 2, 3, if (otherTest) 1];
678+
/// ```
679+
///
680+
/// The optimized add operation can skip checks for a growable or modifiable
681+
/// list and the element type is known to be invariant so it can skip the
682+
/// type check.
683+
bool isNativeListInvariantAddInvocation(Class coreListClass) {
684+
if (isInvariant && name.text == 'add') {
685+
// The call to add is marked as invariant, so the type check on the
686+
// parameter to add is not needed.
687+
var receiver_ = receiver;
688+
if (receiver_ is VariableGet &&
689+
receiver_.variable.isFinal &&
690+
!receiver_.variable.isLate) {
691+
// The receiver is a final variable, so it only contains the
692+
// initializer value. Also, avoid late variables in case the CFE
693+
// lowering of late variables is changed in the future.
694+
var initializer = receiver_.variable.initializer;
695+
if (initializer is ListLiteral) {
696+
// The initializer is a list literal, so we know the list can be
697+
// grown, modified, and is represented by a JavaScript Array.
698+
return true;
699+
}
700+
if (initializer is StaticInvocation &&
701+
initializer.target.enclosingClass == coreListClass &&
702+
initializer.target.name.text == 'of' &&
703+
initializer.arguments.named.isEmpty) {
704+
// The initializer is a `List.of()` call from the dart:core library
705+
// and the growable named argument has not been passed (it defaults
706+
// to true).
707+
return true;
708+
}
709+
}
710+
}
711+
return false;
712+
}
713+
}

0 commit comments

Comments
 (0)