Sort named parameters in native methods by declaration.

Change-Id: Ic4a7f9ad6cf0d362b6582a59c783cc05cc404a31
Reviewed-on: https://dart-review.googlesource.com/c/91828
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This commit is contained in:
Johnni Winther 2019-02-04 10:02:23 +00:00 committed by commit-bot@chromium.org
parent 8c891ec25e
commit 0fd1679620
8 changed files with 91 additions and 85 deletions

View file

@ -20,12 +20,17 @@ String nodeToDebugString(ir.Node node, [int textLength = 40]) {
return '(${node.runtimeType}:${node.hashCode})${blockText}';
}
/// Comparator for the canonical order or named arguments.
/// Comparator for the canonical order or named parameters.
// TODO(johnniwinther): Remove this when named parameters are sorted in dill.
int namedOrdering(ir.VariableDeclaration a, ir.VariableDeclaration b) {
return a.name.compareTo(b.name);
}
/// Comparator for the declaration order of parameters.
int nativeOrdering(ir.VariableDeclaration a, ir.VariableDeclaration b) {
return a.fileOffset.compareTo(b.fileOffset);
}
SourceSpan computeSourceSpanFromTreeNode(ir.TreeNode node) {
// TODO(johnniwinther): Use [ir.Location] directly as a [SourceSpan].
Uri uri;

View file

@ -13,7 +13,6 @@ import '../elements/names.dart' show Name;
import '../elements/types.dart';
import '../ir/closure.dart';
import '../ir/element_map.dart';
import '../ir/util.dart';
import '../js_model/element_map.dart';
import '../js_model/env.dart';
import '../ordered_typeset.dart';
@ -1032,7 +1031,7 @@ abstract class ClosureMemberData implements JMemberData {
}
class ClosureFunctionData extends ClosureMemberData
with FunctionDataMixin
with FunctionDataMixin, FunctionNodeMixin
implements FunctionData {
/// Tag used for identifying serialized [ClosureFunctionData] objects in a
/// debugging data stream.
@ -1075,31 +1074,6 @@ class ClosureFunctionData extends ClosureMemberData
sink.end(tag);
}
void forEachParameter(JsToElementMap elementMap,
void f(DartType type, String name, ConstantValue defaultValue)) {
void handleParameter(ir.VariableDeclaration node, {bool isOptional: true}) {
DartType type = elementMap.getDartType(node.type);
String name = node.name;
ConstantValue defaultValue;
if (isOptional) {
if (node.initializer != null) {
defaultValue = elementMap.getConstantValue(node.initializer);
} else {
defaultValue = new NullConstantValue();
}
}
f(type, name, defaultValue);
}
for (int i = 0; i < functionNode.positionalParameters.length; i++) {
handleParameter(functionNode.positionalParameters[i],
isOptional: i >= functionNode.requiredParameterCount);
}
functionNode.namedParameters.toList()
..sort(namedOrdering)
..forEach(handleParameter);
}
@override
FunctionType getFunctionType(IrToElementMap elementMap) {
return functionType;

View file

@ -69,7 +69,8 @@ abstract class JsToWorldBuilder implements JsToElementMap {
/// Calls [f] for each parameter of [function] providing the type and name of
/// the parameter and the [defaultValue] if the parameter is optional.
void forEachParameter(FunctionEntity function,
void f(DartType type, String name, ConstantValue defaultValue));
void f(DartType type, String name, ConstantValue defaultValue),
{bool isNative: false});
}
class JsKernelToElementMap
@ -1744,9 +1745,10 @@ class JsKernelToElementMap
@override
void forEachParameter(covariant IndexedFunction function,
void f(DartType type, String name, ConstantValue defaultValue)) {
void f(DartType type, String name, ConstantValue defaultValue),
{bool isNative: false}) {
FunctionData data = members.getData(function);
data.forEachParameter(this, f);
data.forEachParameter(this, f, isNative: isNative);
}
void forEachConstructorBody(

View file

@ -566,7 +566,8 @@ abstract class FunctionData implements JMemberData {
List<TypeVariableType> getFunctionTypeVariables(IrToElementMap elementMap);
void forEachParameter(JsToElementMap elementMap,
void f(DartType type, String name, ConstantValue defaultValue));
void f(DartType type, String name, ConstantValue defaultValue),
{bool isNative: false});
}
abstract class FunctionDataMixin implements FunctionData {
@ -597,8 +598,47 @@ abstract class FunctionDataMixin implements FunctionData {
}
}
abstract class FunctionNodeMixin implements FunctionData {
ir.FunctionNode get functionNode;
@override
void forEachParameter(JsToElementMap elementMap,
void f(DartType type, String name, ConstantValue defaultValue),
{bool isNative: false}) {
void handleParameter(ir.VariableDeclaration node, {bool isOptional: true}) {
DartType type = elementMap.getDartType(node.type);
String name = node.name;
ConstantValue defaultValue;
if (isOptional) {
if (node.initializer != null) {
defaultValue = elementMap.getConstantValue(node.initializer);
} else {
defaultValue = new NullConstantValue();
}
}
f(type, name, defaultValue);
}
for (int i = 0; i < functionNode.positionalParameters.length; i++) {
handleParameter(functionNode.positionalParameters[i],
isOptional: i >= functionNode.requiredParameterCount);
}
if (functionNode.namedParameters.isEmpty) {
return;
}
List<ir.VariableDeclaration> namedParameters =
functionNode.namedParameters.toList();
if (isNative) {
namedParameters.sort(nativeOrdering);
} else {
namedParameters.sort(namedOrdering);
}
namedParameters.forEach(handleParameter);
}
}
class FunctionDataImpl extends JMemberDataImpl
with FunctionDataMixin
with FunctionDataMixin, FunctionNodeMixin
implements FunctionData {
/// Tag used for identifying serialized [FunctionDataImpl] objects in a
/// debugging data stream.
@ -644,31 +684,6 @@ class FunctionDataImpl extends JMemberDataImpl
return _type ??= elementMap.getFunctionType(functionNode);
}
void forEachParameter(JsToElementMap elementMap,
void f(DartType type, String name, ConstantValue defaultValue)) {
void handleParameter(ir.VariableDeclaration node, {bool isOptional: true}) {
DartType type = elementMap.getDartType(node.type);
String name = node.name;
ConstantValue defaultValue;
if (isOptional) {
if (node.initializer != null) {
defaultValue = elementMap.getConstantValue(node.initializer);
} else {
defaultValue = new NullConstantValue();
}
}
f(type, name, defaultValue);
}
for (int i = 0; i < functionNode.positionalParameters.length; i++) {
handleParameter(functionNode.positionalParameters[i],
isOptional: i >= functionNode.requiredParameterCount);
}
functionNode.namedParameters.toList()
..sort(namedOrdering)
..forEach(handleParameter);
}
@override
ClassTypeVariableAccess get classTypeVariableAccess {
if (node.isInstanceMember) return ClassTypeVariableAccess.property;
@ -727,7 +742,8 @@ class SignatureFunctionData implements FunctionData {
}
void forEachParameter(JsToElementMap elementMap,
void f(DartType type, String name, ConstantValue defaultValue)) {
void f(DartType type, String name, ConstantValue defaultValue),
{bool isNative: false}) {
throw new UnimplementedError('SignatureData.forEachParameter');
}
@ -750,8 +766,9 @@ abstract class DelegatedFunctionData implements FunctionData {
}
void forEachParameter(JsToElementMap elementMap,
void f(DartType type, String name, ConstantValue defaultValue)) {
return baseData.forEachParameter(elementMap, f);
void f(DartType type, String name, ConstantValue defaultValue),
{bool isNative: false}) {
return baseData.forEachParameter(elementMap, f, isNative: isNative);
}
InterfaceType getMemberThisType(JsToElementMap elementMap) {

View file

@ -1249,7 +1249,7 @@ class KernelSsaGraphBuilder extends ir.Visitor
_sourceInformationBuilder.buildGet(functionNode)));
}
for (ir.VariableDeclaration param in functionNode.positionalParameters) {
void handleParameter(ir.VariableDeclaration param) {
templateArguments.add('#');
Local local = localsMap.getLocalVariable(param);
// Convert Dart function to JavaScript function.
@ -1268,6 +1268,15 @@ class KernelSsaGraphBuilder extends ir.Visitor
inputs.add(argument);
}
functionNode.positionalParameters.forEach(handleParameter);
if (functionNode.namedParameters.isNotEmpty) {
List<ir.VariableDeclaration> namedParameters =
functionNode.namedParameters.toList();
// Sort by file offset to visit parameters in declaration order.
namedParameters.sort(nativeOrdering);
namedParameters.forEach(handleParameter);
}
String arguments = templateArguments.join(',');
// TODO(sra): Use declared type or NativeBehavior type.

View file

@ -687,7 +687,8 @@ class CodegenWorldBuilderImpl extends WorldBuilderBase
@override
void forEachParameter(FunctionEntity function,
void f(DartType type, String name, ConstantValue defaultValue)) {
_elementMap.forEachParameter(function, f);
_elementMap.forEachParameter(function, f,
isNative: _world.nativeData.isNativeMember(function));
}
@override

View file

@ -23,9 +23,8 @@ class Class {
// ignore: NATIVE_FUNCTION_BODY_IN_NON_SDK_CODE
native;
// TODO(johnniwinther): Include named parameters in the main call.
/*element: Class.method2:
calls=[method2(a)],
calls=[method2(a,b,c)],
params=4,
stubs=[
method2$1:method2(a),
@ -41,12 +40,12 @@ class Class {
// we sort them lexicographically but that doesn't match the target
// expectations.
/*element: Class.method3:
calls=[method3(a)],
calls=[method3(a,c,b)],
params=4,
stubs=[
method3$1:method3(a),
method3$2$b:method3(a,b),
method3$2$c:method3(a,null,c)]
method3$2$b:method3(a,null,b),
method3$2$c:method3(a,c)]
*/
@pragma('dart2js:noInline')
method3(a, {c, b})
@ -57,15 +56,15 @@ class Class {
// we sort them lexicographically but that doesn't match the target
// expectations.
/*element: Class.method4:
calls=[method4(a)],
calls=[method4(a,c,d,b)],
params=5,
stubs=[
method4$1:method4(a),
method4$2$b:method4(a,b),
method4$2$c:method4(a,null,c),
method4$3$b$c:method4(a,b,c),
method4$3$b$d:method4(a,b,null,d),
method4$3$c$d:method4(a,null,c,d)]
method4$2$b:method4(a,null,null,b),
method4$2$c:method4(a,c),
method4$3$b$c:method4(a,c,null,b),
method4$3$b$d:method4(a,null,d,b),
method4$3$c$d:method4(a,c,d)]
*/
@pragma('dart2js:noInline')
method4(a, {c, d, b})

View file

@ -15,9 +15,8 @@ class Class {
// ignore: NATIVE_FUNCTION_BODY_IN_NON_SDK_CODE
native;
// TODO(johnniwinther): Include named parameters in the main call.
/*element: Class.method2:
calls=[method2(a)],
calls=[method2(a,b,c)],
params=4,
stubs=[
method2$1:method2(a),
@ -32,12 +31,12 @@ class Class {
// we sort them lexicographically but that doesn't match the target
// expectations.
/*element: Class.method3:
calls=[method3(a)],
calls=[method3(a,c,b)],
params=4,
stubs=[
method3$1:method3(a),
method3$2$b:method3(a,b),
method3$2$c:method3(a,null,c)]
method3$2$b:method3(a,null,b),
method3$2$c:method3(a,c)]
*/
method3(a, {c, b})
// ignore: NATIVE_FUNCTION_BODY_IN_NON_SDK_CODE
@ -47,15 +46,15 @@ class Class {
// we sort them lexicographically but that doesn't match the target
// expectations.
/*element: Class.method4:
calls=[method4(a)],
calls=[method4(a,c,d,b)],
params=5,
stubs=[
method4$1:method4(a),
method4$2$b:method4(a,b),
method4$2$c:method4(a,null,c),
method4$3$b$c:method4(a,b,c),
method4$3$b$d:method4(a,b,null,d),
method4$3$c$d:method4(a,null,c,d)]
method4$2$b:method4(a,null,null,b),
method4$2$c:method4(a,c),
method4$3$b$c:method4(a,c,null,b),
method4$3$b$d:method4(a,null,d,b),
method4$3$c$d:method4(a,c,d)]
*/
method4(a, {c, d, b})
// ignore: NATIVE_FUNCTION_BODY_IN_NON_SDK_CODE