Skip to content

Commit

Permalink
[dart2wasm] Fix missing type check in implicit field setters if field…
Browse files Browse the repository at this point in the history
… is covariant

For a class like this:

   class A<T> {
     T? value;
   }

the implicit setter function (`A.value=`) has to check the argument
type.

TEST=language/covariant_field_implicit_setter_test

Change-Id: Ie990b79f631275fb0a7c88ec7d7dd3a82a784148
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381000
Reviewed-by: Ömer Ağacan <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>
  • Loading branch information
mkustermann authored and Commit Queue committed Aug 16, 2024
1 parent 534d3ff commit e5704ea
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 26 deletions.
67 changes: 44 additions & 23 deletions pkg/dart2wasm/lib/code_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2211,30 +2211,19 @@ abstract class AstCodeGenerator
w.ValueType _directSet(Member target, Expression receiver, Expression value,
{required bool preserved}) {
w.Local? temp;
if (target is Field) {
ClassInfo info = translator.classInfo[target.enclosingClass]!;
int fieldIndex = translator.fieldIndex[target]!;
w.ValueType receiverType = info.nonNullableType;
w.ValueType fieldType = info.struct.fields[fieldIndex].type.unpacked;
wrap(receiver, receiverType);
wrap(value, fieldType);
if (preserved) {
temp = addLocal(fieldType);
b.local_tee(temp);
}
b.struct_set(info.struct, fieldIndex);
} else {
w.FunctionType targetFunctionType =
translator.signatureForDirectCall(target.reference);
w.ValueType paramType = targetFunctionType.inputs.last;
wrap(receiver, targetFunctionType.inputs.first);
wrap(value, paramType);
if (preserved) {
temp = addLocal(paramType);
b.local_tee(temp);
}
call(target.reference);
final Reference reference = (target is Field)
? target.setterReference!
: (target as Procedure).reference;
final w.FunctionType targetFunctionType =
translator.signatureForDirectCall(reference);
final w.ValueType paramType = targetFunctionType.inputs.last;
wrap(receiver, targetFunctionType.inputs.first);
wrap(value, paramType);
if (preserved) {
temp = addLocal(paramType);
b.local_tee(temp);
}
call(reference);
if (preserved) {
b.local_get(temp!);
return temp.type;
Expand Down Expand Up @@ -3806,6 +3795,14 @@ class ImplicitFieldAccessorCodeGenerator extends AstCodeGenerator {

@override
void generateInternal() {
thisLocal = preciseThisLocal = paramLocals[0];

// Conceptually not needed for implicit accessors, but currently the code
// that instantiates types uses closure information to see whether a type
// parameter was captured (and loads it from context chain) or not (and
// loads it directly from `this`).
closures = Closures(translator, field);

final source = field.enclosingComponent!.uriToSource[field.fileUri]!;
setSourceMapSourceAndFileOffset(source, field.fileOffset);

Expand All @@ -3830,9 +3827,33 @@ class ImplicitFieldAccessorCodeGenerator extends AstCodeGenerator {
// Implicit setter
w.Local valueLocal = paramLocals[1];
getThis();

if (!translator.options.omitImplicitTypeChecks) {
if (field.isCovariantByClass || field.isCovariantByDeclaration) {
final boxedType = field.type.isPotentiallyNullable
? translator.topInfo.nullableType
: translator.topInfo.nonNullableType;
w.Local operand = valueLocal;
if (!operand.type.isSubtypeOf(boxedType)) {
final boxedOperand = addLocal(boxedType);
b.local_get(operand);
translator.convertType(b, operand.type, boxedOperand.type);
b.local_set(boxedOperand);
operand = boxedOperand;
}
b.local_get(operand);
_generateArgumentTypeCheck(
field.name.text,
operand.type as w.RefType,
field.type,
);
}
}

b.local_get(valueLocal);
translator.convertType(b, valueLocal.type, fieldType);
b.struct_set(struct, fieldIndex);
assert(functionType.outputs.isEmpty);
}
b.end();
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/dart2wasm/lib/functions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,9 @@ w.FunctionType _makeFunctionType(
final List<w.ValueType> inputs = _getInputTypes(
translator, target, receiverType, isImportOrExport, translateType);

// Mutable fields have initializer setters with a non-empty output list,
// so check that the member is a Procedure
final bool emptyOutputList = member is Procedure && member.isSetter;
final bool emptyOutputList =
(member is Field && member.setterReference == target) ||
(member is Procedure && member.isSetter);

bool isVoidType(DartType t) =>
(isImportOrExport && t is VoidType) ||
Expand Down
26 changes: 26 additions & 0 deletions tests/language/covariant_field_implicit_setter_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// 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.

import 'package:expect/expect.dart';
import 'package:expect/variations.dart';

main() {
final List<A<Object>> l = [A<int>(), A<String>()];

final aInt = l[int.parse('0')];
Expect.isNull(aInt.value);
aInt.value = 10;
Expect.equals(10, aInt.value);

final aString = l[int.parse('1')];
Expect.isNull(aString.value);
if (checkedParameters) {
Expect.throws(() => aString.value = 1);
Expect.isNull(aString.value);
}
}

class A<T> {
T? value;
}

0 comments on commit e5704ea

Please sign in to comment.