Skip to content

Commit

Permalink
[dart2wasm] Refactor ParameterInfo
Browse files Browse the repository at this point in the history 
- 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 <[email protected]>
Commit-Queue: Ömer Ağacan <[email protected]>
  • Loading branch information
@osa1
osa1 authored and Commit Queue committed Dec 5, 2024
1 parent 1b8e01f commit 33bdacc
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 54 deletions.
22 changes: 13 additions & 9 deletions pkg/dart2wasm/lib/dispatch_table.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
66 changes: 44 additions & 22 deletions pkg/dart2wasm/lib/param_info.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Constant?> positional;
final List<Constant?> positional;

/// Default values of named parameters. Similar to [positional], `null` means
/// the the parameter is not optional.
late final Map<String, Constant?> named;
final Map<String, Constant?> 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<String> names = named.keys.toList()..sort();

late final Map<String, int> nameIndex = {
for (int i = 0; i < names.length; i++) names[i]: positional.length + i
};
Expand All @@ -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]);
Expand Down
30 changes: 7 additions & 23 deletions pkg/dart2wasm/lib/translator.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -1394,25 +1389,17 @@ class _ClosureTrampolineGenerator implements CodeGenerator {
final int posArgCount;
final List<String> 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<w.Local> paramLocals,
w.Label? returnLabel) {
assert(returnLabel == null);

int targetIndex = 0;
if (takesContextOrReceiver) {
if (paramInfo.takesContextOrReceiver) {
w.Local receiver = trampoline.locals[0];
b.local_get(receiver);
translator.convertType(
Expand Down Expand Up @@ -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];
Expand All @@ -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);
Expand Down

0 comments on commit 33bdacc

Please sign in to comment.