From e5704eacfaeba826d6564b2367dd8e3e0c94b3d2 Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Fri, 16 Aug 2024 11:49:19 +0000 Subject: [PATCH] [dart2wasm] Fix missing type check in implicit field setters if field is covariant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For a class like this: class A { 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 Commit-Queue: Martin Kustermann --- pkg/dart2wasm/lib/code_generator.dart | 67 ++++++++++++------- pkg/dart2wasm/lib/functions.dart | 6 +- .../covariant_field_implicit_setter_test.dart | 26 +++++++ 3 files changed, 73 insertions(+), 26 deletions(-) create mode 100644 tests/language/covariant_field_implicit_setter_test.dart diff --git a/pkg/dart2wasm/lib/code_generator.dart b/pkg/dart2wasm/lib/code_generator.dart index 3fd20d690500..ea163669ec0d 100644 --- a/pkg/dart2wasm/lib/code_generator.dart +++ b/pkg/dart2wasm/lib/code_generator.dart @@ -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; @@ -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); @@ -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(); } diff --git a/pkg/dart2wasm/lib/functions.dart b/pkg/dart2wasm/lib/functions.dart index 498d3db99332..0c261b2c743e 100644 --- a/pkg/dart2wasm/lib/functions.dart +++ b/pkg/dart2wasm/lib/functions.dart @@ -471,9 +471,9 @@ w.FunctionType _makeFunctionType( final List 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) || diff --git a/tests/language/covariant_field_implicit_setter_test.dart b/tests/language/covariant_field_implicit_setter_test.dart new file mode 100644 index 000000000000..722796ea0aac --- /dev/null +++ b/tests/language/covariant_field_implicit_setter_test.dart @@ -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> l = [A(), A()]; + + 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? value; +}