Skip to content

Commit

Permalink
[dart2wasm] Fix instantiation context types in instantiation trampolines
Browse files Browse the repository at this point in the history
Type for a closure with one argument (Closure-0-1) is subtype of type of
a closure with no arguments (Closure-0-0).

This allows calling the closure with no arguments in the generated code.

(This only happens when the Dart types align, i.e. the argument is
optional)

However instantiation vtable functions for e.g. Closure-1-1 currently
can't be as Closure-1-0, resulting in a type cast error in a code like:

    void test<T>([T? x]) {}

    void Function() x = runtimeTrue ? test : () {};
    x();

Relax the instantiation context struct type in instantiation vtable
functions and downcast the generic closure struct type (type of the
instantiation closure) to the right type before calling instantiation
clsoure.

Fixes #56534.

Change-Id: I59f9b019c4dc8d2e26ee21f1f6c3c607af0be4d8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381641
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
  • Loading branch information
osa1 authored and Commit Queue committed Aug 23, 2024
1 parent 6378a26 commit 9dc5754
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 10 deletions.
25 changes: 15 additions & 10 deletions pkg/dart2wasm/lib/closures.dart
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ class ClosureLayouter extends RecursiveVisitor {
w.BaseFunction trampoline = _createInstantiationTrampoline(
typeCount,
closureStruct,
instantiationContextStruct!,
_getInstantiationContextBaseStruct(typeCount),
instantiatedRepresentation!.vtableStruct,
vtableBaseIndexNonGeneric + instantiationTrampolines.length,
vtableStruct,
Expand All @@ -463,7 +463,7 @@ class ClosureLayouter extends RecursiveVisitor {
? _createInstantiationTrampoline(
typeCount,
closureStruct,
instantiationContextStruct!,
_getInstantiationContextBaseStruct(typeCount),
instantiatedRepresentation.vtableStruct,
vtableBaseIndexNonGeneric + instantiationTrampolines.length,
vtableStruct,
Expand Down Expand Up @@ -509,12 +509,12 @@ class ClosureLayouter extends RecursiveVisitor {
w.BaseFunction _createInstantiationTrampoline(
int typeCount,
w.StructType genericClosureStruct,
w.StructType contextStruct,
w.StructType instantiationContextBaseStruct,
w.StructType instantiatedVtableStruct,
int instantiatedVtableFieldIndex,
w.StructType genericVtableStruct,
int genericVtableFieldIndex) {
assert(contextStruct.fields.length == 1 + typeCount);
assert(instantiationContextBaseStruct.fields.length == 1 + typeCount);
w.FunctionType instantiatedFunctionType = (instantiatedVtableStruct
.fields[instantiatedVtableFieldIndex].type as w.RefType)
.heapType as w.FunctionType;
Expand All @@ -528,21 +528,23 @@ class ClosureLayouter extends RecursiveVisitor {
final b = trampoline.body;

// Cast context reference to actual context type.
w.RefType contextType = w.RefType.def(contextStruct, nullable: false);
w.RefType contextType =
w.RefType.def(instantiationContextBaseStruct, nullable: false);
w.Local contextLocal = b.addLocal(contextType);
b.local_get(trampoline.locals[0]);
b.ref_cast(contextType);
b.local_tee(contextLocal);

// Push inner context
b.struct_get(contextStruct, FieldIndex.instantiationContextInner);
b.struct_get(genericClosureStruct, FieldIndex.closureContext);
b.struct_get(
instantiationContextBaseStruct, FieldIndex.instantiationContextInner);
b.struct_get(closureBaseStruct, FieldIndex.closureContext);

// Push type arguments
for (int t = 0; t < typeCount; t++) {
b.local_get(contextLocal);
b.struct_get(
contextStruct, FieldIndex.instantiationContextTypeArgumentsBase + t);
b.struct_get(instantiationContextBaseStruct,
FieldIndex.instantiationContextTypeArgumentsBase + t);
}

// Push arguments
Expand All @@ -552,7 +554,10 @@ class ClosureLayouter extends RecursiveVisitor {

// Call inner
b.local_get(contextLocal);
b.struct_get(contextStruct, FieldIndex.instantiationContextInner);
b.struct_get(
instantiationContextBaseStruct, FieldIndex.instantiationContextInner);
// #ClosureBase to closure struct with the right arguments
b.ref_cast(w.RefType(genericClosureStruct, nullable: false));
b.struct_get(genericClosureStruct, FieldIndex.closureVtable);
b.struct_get(genericVtableStruct, genericVtableFieldIndex);
b.call_ref(genericFunctionType);
Expand Down
12 changes: 12 additions & 0 deletions tests/language/regress/regress_56534_1_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) 2024, 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.

bool get runtimeTrue => int.parse('1') == 1;

void test<T>([T? x]) {}

void main() {
void Function() x = runtimeTrue ? test : () {};
x();
}
15 changes: 15 additions & 0 deletions tests/language/regress/regress_56534_2_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) 2024, 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.

bool get runtimeTrue => int.parse('1') == 1;

class C {
void test<T>([T? x]) {}
}

void main() {
final c = C();
void Function() x = runtimeTrue ? c.test : () {};
x();
}

0 comments on commit 9dc5754

Please sign in to comment.