From eb6f1b6abd2f968d4e369ca2b4f88ca731569d2d Mon Sep 17 00:00:00 2001 From: Johnni Winther Date: Fri, 15 Mar 2019 20:15:25 +0000 Subject: [PATCH] Handle effectively constant fields in class stub generator Closes #36222 Change-Id: Ic8b7a3963e1364903b4b2d7871ff0b12cb134572 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97082 Commit-Queue: Johnni Winther Reviewed-by: Sigmund Cherem --- .../src/js_emitter/class_stub_generator.dart | 11 +- .../dart2js/codegen/model_data/capture.dart | 10 +- .../codegen/model_data/regress_36222.dart | 27 ++++ .../compiler/dart2js/codegen/model_test.dart | 152 ++++++++++-------- .../field_analysis/jdata/regress_36222.dart | 24 +++ .../field_analysis/kdata/regress_36222.dart | 24 +++ .../dart2js_extra/regress_36222_test.dart | 23 +++ 7 files changed, 199 insertions(+), 72 deletions(-) create mode 100644 tests/compiler/dart2js/codegen/model_data/regress_36222.dart create mode 100644 tests/compiler/dart2js/field_analysis/jdata/regress_36222.dart create mode 100644 tests/compiler/dart2js/field_analysis/kdata/regress_36222.dart create mode 100644 tests/compiler/dart2js_extra/regress_36222_test.dart diff --git a/pkg/compiler/lib/src/js_emitter/class_stub_generator.dart b/pkg/compiler/lib/src/js_emitter/class_stub_generator.dart index fb31e92d6f9..81b9f9e4ecd 100644 --- a/pkg/compiler/lib/src/js_emitter/class_stub_generator.dart +++ b/pkg/compiler/lib/src/js_emitter/class_stub_generator.dart @@ -9,6 +9,7 @@ import '../common_elements.dart' show CommonElements; import '../elements/entities.dart'; import '../js/js.dart' as jsAst; import '../js/js.dart' show js; +import '../js_backend/field_analysis.dart'; import '../js_backend/namer.dart' show Namer; import '../js_backend/interceptor_data.dart' show InterceptorData; import '../options.dart'; @@ -108,8 +109,14 @@ class ClassStubGenerator { } return js('#.#()', [receiver, getterName]); } else { - jsAst.Name fieldName = _namer.instanceFieldPropertyName(member); - return js('#.#', [receiver, fieldName]); + FieldAnalysisData fieldData = + _closedWorld.fieldAnalysis.getFieldData(member); + if (fieldData.isEffectivelyConstant) { + return _emitter.constantReference(fieldData.constantValue); + } else { + jsAst.Name fieldName = _namer.instanceFieldPropertyName(member); + return js('#.#', [receiver, fieldName]); + } } } diff --git a/tests/compiler/dart2js/codegen/model_data/capture.dart b/tests/compiler/dart2js/codegen/model_data/capture.dart index ca9279e7ee3..5e44fb01cbf 100644 --- a/tests/compiler/dart2js/codegen/model_data/capture.dart +++ b/tests/compiler/dart2js/codegen/model_data/capture.dart @@ -4,7 +4,7 @@ /*element: method1:params=0*/ @pragma('dart2js:noInline') -method1([a]) => /*params=0*/ () => a; +method1([a]) => /*access=[a],params=0*/ () => a; class Class { /*element: Class.f:emitted*/ @@ -13,14 +13,14 @@ class Class { /*element: Class.capture:params=0*/ @pragma('dart2js:noInline') - Class.capture([a]) : f = (/*params=0*/ () => a); + Class.capture([a]) : f = (/*access=[a],params=0*/ () => a); // TODO(johnniwinther): Remove the redundant assignment of elided boxed // parameters. /*element: Class.box:assign=[a,a],params=0*/ @pragma('dart2js:noInline') Class.box([a]) - : f = (/*assign=[a],params=0*/ () { + : f = (/*access=[_box_0],assign=[a],params=0*/ () { a = 42; }); @@ -30,12 +30,12 @@ class Class { class Subclass extends Class { /*element: Subclass.capture:params=0*/ @pragma('dart2js:noInline') - Subclass.capture([a]) : super.internal(/*params=0*/ () => a); + Subclass.capture([a]) : super.internal(/*access=[a],params=0*/ () => a); /*element: Subclass.box:assign=[a,a],params=0*/ @pragma('dart2js:noInline') Subclass.box([a]) - : super.internal(/*assign=[a],params=0*/ () { + : super.internal(/*access=[_box_0],assign=[a],params=0*/ () { a = 42; }); } diff --git a/tests/compiler/dart2js/codegen/model_data/regress_36222.dart b/tests/compiler/dart2js/codegen/model_data/regress_36222.dart new file mode 100644 index 00000000000..8ab6ea309f8 --- /dev/null +++ b/tests/compiler/dart2js/codegen/model_data/regress_36222.dart @@ -0,0 +1,27 @@ +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +typedef int BinaryFunc(int x, int y); + +class A { + const A({this.foo = A.defaultFoo}); + + /*element: A.defaultFoo:params=2*/ + static int defaultFoo(int x, int y) { + return x + y; + } + + /*element: A.foo:elided,stubCalls=[foo$2:call$2(arg0,arg1),foo$2:main_A_defaultFoo$closure(0)]*/ + final BinaryFunc foo; +} + +/*element: test:calls=[foo$2(2)],params=1*/ +@pragma('dart2js:assumeDynamic') +@pragma('dart2js:noInline') +test(dynamic a) => a.foo(1, 2); + +/*element: main:calls=[test(1)],params=0*/ +main() { + test(new A()); +} diff --git a/tests/compiler/dart2js/codegen/model_test.dart b/tests/compiler/dart2js/codegen/model_test.dart index 84cda3ff2a6..af0b3c80b4d 100644 --- a/tests/compiler/dart2js/codegen/model_test.dart +++ b/tests/compiler/dart2js/codegen/model_test.dart @@ -59,6 +59,8 @@ class Tags { static const String parameterCount = 'params'; static const String call = 'calls'; static const String parameterStub = 'stubs'; + static const String callStubCall = 'stubCalls'; + static const String callStubAccesses = 'stubAccesses'; static const String isEmitted = 'emitted'; static const String isElided = 'elided'; static const String assignment = 'assign'; @@ -83,6 +85,75 @@ class ModelIrComputer extends IrDataExtractor { : _programLookup = new ProgramLookup(compiler), super(reporter, actualMap); + void registerCalls(Features features, String tag, js.Node node, + {String prefix = '', Set handledAccesses}) { + forEachNode(node, onCall: (js.Call node) { + js.Node target = node.target; + if (target is js.PropertyAccess) { + js.Node selector = target.selector; + bool fixedNameCall = false; + String name; + if (selector is js.Name) { + name = selector.key; + fixedNameCall = selector is StringBackedName; + } else if (selector is js.LiteralString) { + /// Call to fixed backend name, so we include the argument + /// values to test encoding of optional parameters in native + /// methods. + name = selector.value.substring(1, selector.value.length - 1); + fixedNameCall = true; + } + if (name != null) { + if (fixedNameCall) { + String arguments = node.arguments.map(js.nodeToString).join(','); + features.addElement(tag, '${prefix}${name}(${arguments})'); + } else { + features.addElement( + tag, '${prefix}${name}(${node.arguments.length})'); + } + handledAccesses?.add(target); + } + } + }); + } + + void registerAccesses(Features features, String tag, js.Node code, + {String prefix = '', Set handledAccesses}) { + forEachNode(code, onPropertyAccess: (js.PropertyAccess node) { + if (handledAccesses?.contains(node) ?? false) { + return; + } + + js.Node receiver = node.receiver; + String receiverName; + if (receiver is js.VariableUse) { + receiverName = receiver.name; + if (receiverName == receiverName.toUpperCase() && + receiverName != r'$') { + // Skip holder access. + receiverName = null; + } + } else if (receiver is js.This) { + receiverName = 'this'; + } + + js.Node selector = node.selector; + String name; + if (selector is js.Name) { + name = selector.key; + } else if (selector is js.LiteralString) { + /// Call to fixed backend name, so we include the argument + /// values to test encoding of optional parameters in native + /// methods. + name = selector.value.substring(1, selector.value.length - 1); + } + + if (receiverName != null && name != null) { + features.addElement(tag, '${prefix}${name}'); + } + }); + } + Features getMemberValue(MemberEntity member) { if (member is FieldEntity) { Field field = _programLookup.getField(member); @@ -115,6 +186,16 @@ class ModelIrComputer extends IrDataExtractor { registerFlags(Tags.getterFlags, field.getterFlags); registerFlags(Tags.setterFlags, field.setterFlags); + Class cls = _programLookup.getClass(member.enclosingClass); + for (StubMethod stub in cls.callStubs) { + if (stub.element == member) { + registerCalls(features, Tags.callStubCall, stub.code, + prefix: '${stub.name.key}:'); + registerAccesses(features, Tags.callStubAccesses, stub.code, + prefix: '${stub.name.key}:'); + } + } + return features; } StaticField staticField = _programLookup.getStaticField(member); @@ -137,42 +218,12 @@ class ModelIrComputer extends IrDataExtractor { Set handledAccesses = new Set(); - void registerCalls(String tag, js.Node node, [String prefix = '']) { - forEachNode(node, onCall: (js.Call node) { - js.Node target = node.target; - if (target is js.PropertyAccess) { - js.Node selector = target.selector; - bool fixedNameCall = false; - String name; - if (selector is js.Name) { - name = selector.key; - fixedNameCall = selector is StringBackedName; - } else if (selector is js.LiteralString) { - /// Call to fixed backend name, so we include the argument - /// values to test encoding of optional parameters in native - /// methods. - name = selector.value.substring(1, selector.value.length - 1); - fixedNameCall = true; - } - if (name != null) { - if (fixedNameCall) { - String arguments = - node.arguments.map(js.nodeToString).join(','); - features.addElement(tag, '${prefix}${name}(${arguments})'); - } else { - features.addElement( - tag, '${prefix}${name}(${node.arguments.length})'); - } - handledAccesses.add(target); - } - } - }); - } - - registerCalls(Tags.call, code); + registerCalls(features, Tags.call, code, + handledAccesses: handledAccesses); if (method is DartMethod) { for (ParameterStubMethod stub in method.parameterStubs) { - registerCalls(Tags.parameterStub, stub.code, '${stub.name.key}:'); + registerCalls(features, Tags.parameterStub, stub.code, + prefix: '${stub.name.key}:', handledAccesses: handledAccesses); } } @@ -193,37 +244,8 @@ class ModelIrComputer extends IrDataExtractor { } }); - forEachNode(code, onPropertyAccess: (js.PropertyAccess node) { - if (handledAccesses.contains(node)) { - return; - } - - js.Node receiver = node.receiver; - String receiverName; - if (receiver is js.VariableUse) { - receiverName = receiver.name; - if (receiverName == receiverName.toUpperCase() && - receiverName != r'$') { - // Skip holder access. - receiverName = null; - } - } - - js.Node selector = node.selector; - String name; - if (selector is js.Name) { - name = selector.key; - } else if (selector is js.LiteralString) { - /// Call to fixed backend name, so we include the argument - /// values to test encoding of optional parameters in native - /// methods. - name = selector.value.substring(1, selector.value.length - 1); - } - - if (receiverName != null && name != null) { - features.addElement(Tags.propertyAccess, '${name}'); - } - }); + registerAccesses(features, Tags.propertyAccess, code, + handledAccesses: handledAccesses); forEachNode(code, onSwitch: (js.Switch node) { features.add(Tags.switchCase); diff --git a/tests/compiler/dart2js/field_analysis/jdata/regress_36222.dart b/tests/compiler/dart2js/field_analysis/jdata/regress_36222.dart new file mode 100644 index 00000000000..d798780b895 --- /dev/null +++ b/tests/compiler/dart2js/field_analysis/jdata/regress_36222.dart @@ -0,0 +1,24 @@ +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +typedef int BinaryFunc(int x, int y); + +class A { + const A({this.foo = A.defaultFoo}); + + static int defaultFoo(int x, int y) { + return x + y; + } + + /*element: A.foo:constant=FunctionConstant(A.defaultFoo)*/ + final BinaryFunc foo; +} + +@pragma('dart2js:assumeDynamic') +@pragma('dart2js:noInline') +test(dynamic a) => a.foo(1, 2); + +main() { + test(new A()); +} diff --git a/tests/compiler/dart2js/field_analysis/kdata/regress_36222.dart b/tests/compiler/dart2js/field_analysis/kdata/regress_36222.dart new file mode 100644 index 00000000000..6794ac8fd59 --- /dev/null +++ b/tests/compiler/dart2js/field_analysis/kdata/regress_36222.dart @@ -0,0 +1,24 @@ +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +typedef int BinaryFunc(int x, int y); + +class A { + const A({this.foo = A.defaultFoo}); + + static int defaultFoo(int x, int y) { + return x + y; + } + + /*element: A.foo:A.=foo:FunctionConstant(A.defaultFoo),initial=NullConstant*/ + final BinaryFunc foo; +} + +@pragma('dart2js:assumeDynamic') +@pragma('dart2js:noInline') +test(dynamic a) => a.foo(1, 2); + +main() { + test(new A()); +} diff --git a/tests/compiler/dart2js_extra/regress_36222_test.dart b/tests/compiler/dart2js_extra/regress_36222_test.dart new file mode 100644 index 00000000000..f0308337133 --- /dev/null +++ b/tests/compiler/dart2js_extra/regress_36222_test.dart @@ -0,0 +1,23 @@ +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +typedef int BinaryFunc(int x, int y); + +class A { + const A({this.foo = A.defaultFoo}); + + static int defaultFoo(int x, int y) { + return x + y; + } + + final BinaryFunc foo; +} + +@pragma('dart2js:assumeDynamic') +@pragma('dart2js:noInline') +test(dynamic a) => a.foo(1, 2); + +main() { + test(new A()); +}