From 33bdacc604f09199d5b89685ea4c0de8a91833e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Thu, 5 Dec 2024 09:56:21 +0000 Subject: [PATCH] [dart2wasm] Refactor ParameterInfo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove the `Member? member` field: - Knowing a `ParameterInfo`s member is not too useful, because it doesn't tell us for which use of the member (tear-off getter, invocation) the `ParameterInfo` is for. - The `member` field has two uses: - To compute whether the function being called takes a receiver or context parameter. - In dispatch table builder: to check whether a selector (via its `ParameterInfo`) is for `noSuchMethod`. Both of these can be implemented more directly, as done in this CL: - Add `final bool takesContextOrReceiver` member to `ParameterInfo`. - Add `final bool alwaysAddToDispatchTable` member to `SelectorInfo` to handle the special case with `noSuchMethod`. - Add various assertions in `ParameterInfo` members. Change-Id: I473657f594c6306605677cf001854407d43f1347 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398883 Reviewed-by: Martin Kustermann Commit-Queue: Ömer Ağacan --- pkg/dart2wasm/lib/dispatch_table.dart | 22 +++++---- pkg/dart2wasm/lib/param_info.dart | 66 ++++++++++++++++++--------- pkg/dart2wasm/lib/translator.dart | 30 +++--------- 3 files changed, 64 insertions(+), 54 deletions(-) diff --git a/pkg/dart2wasm/lib/dispatch_table.dart b/pkg/dart2wasm/lib/dispatch_table.dart index b270441f3037..bf9e8c2a6f18 100644 --- a/pkg/dart2wasm/lib/dispatch_table.dart +++ b/pkg/dart2wasm/lib/dispatch_table.dart @@ -70,10 +70,16 @@ class SelectorInfo { int? offset; /// The selector's member's name. - String get name => paramInfo.member!.name.text; + final String name; - SelectorInfo._(this.translator, this.id, this.callCount, this.paramInfo, - {required this.isSetter}); + /// Whether the selector is for `Object.noSuchMethod`. We introduce instance + /// invocations of `noSuchMethod` in dynamic calls, so we always add + /// `noSuchMethod` overrides to the dispatch table. + final bool isNoSuchMethod; + + SelectorInfo._( + this.translator, this.id, this.name, this.callCount, this.paramInfo, + {required this.isSetter, required this.isNoSuchMethod}); /// Compute the signature for the functions implementing members targeted by /// this selector. @@ -285,9 +291,10 @@ class DispatchTable { final selector = _selectorInfo.putIfAbsent( selectorId, - () => SelectorInfo._(translator, selectorId, + () => SelectorInfo._(translator, selectorId, member.name.text, _selectorMetadata[selectorId].callCount, paramInfo, - isSetter: isSetter)); + isSetter: isSetter, + isNoSuchMethod: member == translator.objectNoSuchMethod)); assert(selector.isSetter == isSetter); selector.hasTearOffUses |= metadata.hasTearOffUses; selector.hasNonThisUses |= metadata.hasNonThisUses; @@ -452,10 +459,7 @@ class DispatchTable { // Assign selector offsets bool isUsedViaDispatchTableCall(SelectorInfo selector) { - // Special case for `objectNoSuchMethod`: we introduce instance - // invocations of `objectNoSuchMethod` in dynamic calls, so keep it alive - // even if there was no references to it from the Dart code. - if (selector.paramInfo.member! == translator.objectNoSuchMethod) { + if (selector.isNoSuchMethod) { return true; } if (selector.callCount == 0) return false; diff --git a/pkg/dart2wasm/lib/param_info.dart b/pkg/dart2wasm/lib/param_info.dart index 4166676cf48f..2aeb82b38246 100644 --- a/pkg/dart2wasm/lib/param_info.dart +++ b/pkg/dart2wasm/lib/param_info.dart @@ -9,19 +9,22 @@ import 'reference_extensions.dart'; /// Information about optional parameters and their default values for a member /// or a set of members belonging to the same override group. class ParameterInfo { - final Member? member; - int typeParamCount = 0; + final int typeParamCount; /// Default values of optional positonal parameters. `positional[i] == null` /// means positional parameter `i` is not optional. - late final List positional; + final List positional; /// Default values of named parameters. Similar to [positional], `null` means /// the the parameter is not optional. - late final Map named; + final Map named; - // Do not access these until the info is complete. + final bool takesContextOrReceiver; + + // Dispatch table builder updates `ParameterInfo`s, do not access late fields + // until the `ParameterInfo` is complete. late final List names = named.keys.toList()..sort(); + late final Map nameIndex = { for (int i = 0; i < names.length; i++) names[i]: positional.length + i }; @@ -45,47 +48,66 @@ class ParameterInfo { } } - ParameterInfo.fromMember(Reference target) : member = target.asMember { - FunctionNode? function = member!.function; + ParameterInfo._(this.takesContextOrReceiver, this.typeParamCount, + this.positional, this.named); + + factory ParameterInfo.fromMember(Reference target) { + final member = target.asMember; // Constructor, Field, or Procedure + final function = member.function; + if (target.isTearOffReference) { - positional = []; - named = {}; - } else if (function != null) { - typeParamCount = (member is Constructor - ? member!.enclosingClass!.typeParameters + // Tear-off getters don't take type parameters even if the member is + // generic. + return ParameterInfo._(true, 0, [], {}); + } + + if (function != null) { + // Constructor, or static or instance method. + assert(member is Constructor || member is Procedure); + + final typeParamCount = (member is Constructor + ? member.enclosingClass.typeParameters : function.typeParameters) .length; - positional = List.generate(function.positionalParameters.length, (i) { + + final positional = + List.generate(function.positionalParameters.length, (i) { // A required parameter has no default value. if (i < function.requiredParameterCount) return null; return _defaultValue(function.positionalParameters[i]); }); - named = { + + final named = { for (VariableDeclaration param in function.namedParameters) param.name!: _defaultValue(param) }; - } else { - // A setter parameter has no default value. - positional = [if (target.isSetter) null]; - named = {}; + + return ParameterInfo._( + member.isInstanceMember, typeParamCount, positional, named); } + + // A setter or getter. A setter parameter has no default value. + assert(member is Field); + return ParameterInfo._(true, 0, [if (target.isSetter) null], {}); } - ParameterInfo.fromLocalFunction(FunctionNode function) : member = null { - typeParamCount = function.typeParameters.length; - positional = List.generate(function.positionalParameters.length, (i) { + factory ParameterInfo.fromLocalFunction(FunctionNode function) { + final typeParamCount = function.typeParameters.length; + final positional = List.generate(function.positionalParameters.length, (i) { // A required parameter has no default value. if (i < function.requiredParameterCount) return null; return _defaultValue(function.positionalParameters[i]); }); - named = { + final named = { for (VariableDeclaration param in function.namedParameters) param.name!: _defaultValue(param) }; + return ParameterInfo._(true, typeParamCount, positional, named); } void merge(ParameterInfo other) { assert(typeParamCount == other.typeParamCount); + assert(takesContextOrReceiver == other.takesContextOrReceiver); for (int i = 0; i < other.positional.length; i++) { if (i >= positional.length) { positional.add(other.positional[i]); diff --git a/pkg/dart2wasm/lib/translator.dart b/pkg/dart2wasm/lib/translator.dart index c482593106ed..58101c05a921 100644 --- a/pkg/dart2wasm/lib/translator.dart +++ b/pkg/dart2wasm/lib/translator.dart @@ -724,11 +724,6 @@ class Translator with KernelNodes { ClosureImplementation getClosure(FunctionNode functionNode, w.BaseFunction target, ParameterInfo paramInfo, String name) { final targetModule = target.enclosingModule; - // The target function takes an extra initial parameter if it's a function - // expression / local function (which takes a context) or a tear-off of an - // instance method (which takes a receiver). - bool takesContextOrReceiver = - paramInfo.member == null || paramInfo.member!.isInstanceMember; // Look up the closure representation for the signature. int typeCount = functionNode.typeParameters.length; @@ -741,7 +736,7 @@ class Translator with KernelNodes { assert(positionalCount <= paramInfo.positional.length); assert(names.length <= paramInfo.named.length); assert(target.type.inputs.length == - (takesContextOrReceiver ? 1 : 0) + + (paramInfo.takesContextOrReceiver ? 1 : 0) + paramInfo.typeParamCount + paramInfo.positional.length + paramInfo.named.length); @@ -805,7 +800,7 @@ class Translator with KernelNodes { compilationQueue.add(CompilationTask( trampoline, _ClosureTrampolineGenerator(this, trampoline, target, typeCount, - posArgCount, argNames, paramInfo, takesContextOrReceiver))); + posArgCount, argNames, paramInfo))); return trampoline; } @@ -1394,17 +1389,9 @@ class _ClosureTrampolineGenerator implements CodeGenerator { final int posArgCount; final List argNames; final ParameterInfo paramInfo; - final bool takesContextOrReceiver; - - _ClosureTrampolineGenerator( - this.translator, - this.trampoline, - this.target, - this.typeCount, - this.posArgCount, - this.argNames, - this.paramInfo, - this.takesContextOrReceiver); + + _ClosureTrampolineGenerator(this.translator, this.trampoline, this.target, + this.typeCount, this.posArgCount, this.argNames, this.paramInfo); @override void generate(w.InstructionsBuilder b, List paramLocals, @@ -1412,7 +1399,7 @@ class _ClosureTrampolineGenerator implements CodeGenerator { assert(returnLabel == null); int targetIndex = 0; - if (takesContextOrReceiver) { + if (paramInfo.takesContextOrReceiver) { w.Local receiver = trampoline.locals[0]; b.local_get(receiver); translator.convertType( @@ -1478,9 +1465,6 @@ class _ClosureDynamicEntryGenerator implements CodeGenerator { final b = function.body; - final bool takesContextOrReceiver = - paramInfo.member == null || paramInfo.member!.isInstanceMember; - final int typeCount = functionNode.typeParameters.length; final closureLocal = function.locals[0]; @@ -1500,7 +1484,7 @@ class _ClosureDynamicEntryGenerator implements CodeGenerator { int inputIdx = 0; // Push context or receiver - if (takesContextOrReceiver) { + if (paramInfo.takesContextOrReceiver) { final closureBaseType = w.RefType.def( translator.closureLayouter.closureBaseStruct, nullable: false);