[dart2wasm] Fix bug in seter type checker argument checking implementation The CL in [0] introduced a regression: By not performing the type checking code in -O4 mode it didn't have the side-effect of the type checking code anymore (setting the `argLocal`) which caused the setter to store `null` instead of the actual value. Patchset 1: Fix Patchset 2: Make type checking code not have side-effects [0] https://dart-review.googlesource.com/c/sdk/+/370280 Fixes https://github.com/flutter/flutter/issues/152029 TEST=web/wasm/flutter_regress_152029_test Change-Id: Ia678d922ba766aa885224e069f5f4e04a4bf457a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/378800 Reviewed-by: Srujan Gaddam <srujzs@google.com> Commit-Queue: Martin Kustermann <kustermann@google.com>
diff --git a/pkg/dart2wasm/lib/code_generator.dart b/pkg/dart2wasm/lib/code_generator.dart index b8c65b6..1488cfe 100644 --- a/pkg/dart2wasm/lib/code_generator.dart +++ b/pkg/dart2wasm/lib/code_generator.dart
@@ -293,11 +293,11 @@ b.local_set(boxedOperand); operand = boxedOperand; } + b.local_get(operand); _generateArgumentTypeCheck( variable.name!, - () => b.local_get(operand), + operand.type as w.RefType, variable.type, - operand, ); } } @@ -2909,32 +2909,23 @@ /// Generate code that checks type of an argument against an expected type /// and throws a `TypeError` on failure. /// - /// Does not expect any values on stack and does not leave any values on - /// stack. - /// - /// Locals [argLocal] are used to store values pushed by [pushArg] - /// and reuse the values. + /// Expects a boxed object (whose type is to be checked) on the stack. /// /// [argName] is used in the type error as the name of the argument that /// doesn't match the expected type. void _generateArgumentTypeCheck( String argName, - void Function() pushArg, + w.RefType argumentType, DartType testedAgainstType, - w.Local argLocal, ) { if (translator.options.minify) { // We don't need to include the name in the error message, so we can use // the optimized `as` checks. - pushArg(); - types.emitAsCheck( - this, - testedAgainstType, - translator.coreTypes.objectNullableRawType, - argLocal.type as w.RefType); + types.emitAsCheck(this, testedAgainstType, + translator.coreTypes.objectNullableRawType, argumentType); b.drop(); } else { - pushArg(); + final argLocal = b.addLocal(argumentType); b.local_tee(argLocal); types.emitIsTest( this, testedAgainstType, translator.coreTypes.objectNullableRawType); @@ -3288,24 +3279,15 @@ final List<VariableDeclaration> memberPositionalParams = procedure.function.positionalParameters; - // Local for the current argument being checked. Used to avoid indexing - // the positional parameters array again when throwing type error. - final argLocal = addLocal(translator.topInfo.nullableType); - for (int positionalParamIdx = 0; positionalParamIdx < memberPositionalParams.length; positionalParamIdx += 1) { final param = memberPositionalParams[positionalParamIdx]; + b.local_get(positionalArgsLocal); + b.i32_const(positionalParamIdx); + b.array_get(translator.nullableObjectArrayType); _generateArgumentTypeCheck( - param.name!, - () { - b.local_get(positionalArgsLocal); - b.i32_const(positionalParamIdx); - b.array_get(translator.nullableObjectArrayType); - }, - param.type, - argLocal, - ); + param.name!, translator.topInfo.nullableType, param.type); } // Check named argument types @@ -3328,16 +3310,11 @@ namedParamIdx < memberNamedParams.length; namedParamIdx += 1) { final param = memberNamedParams[namedParamIdx]; + b.local_get(namedArgsLocal); + b.i32_const(mapNamedParameterToArrayIndex(param.name!)); + b.array_get(translator.nullableObjectArrayType); _generateArgumentTypeCheck( - param.name!, - () { - b.local_get(namedArgsLocal); - b.i32_const(mapNamedParameterToArrayIndex(param.name!)); - b.array_get(translator.nullableObjectArrayType); - }, - param.type, - argLocal, - ); + param.name!, translator.topInfo.nullableType, param.type); } } @@ -3399,9 +3376,6 @@ _initializeThis(member.reference); - // Local for the argument. - final argLocal = addLocal(translator.topInfo.nullableType); - final member_ = member; DartType paramType; if (member_ is Field) { @@ -3411,11 +3385,11 @@ } if (!translator.options.omitImplicitTypeChecks) { + b.local_get(positionalArgLocal); _generateArgumentTypeCheck( member.name.text, - () => b.local_get(positionalArgLocal), + positionalArgLocal.type as w.RefType, paramType, - argLocal, ); } @@ -3424,9 +3398,9 @@ int fieldIndex = translator.fieldIndex[member_]!; b.local_get(receiverLocal); translator.convertType(b, receiverLocal.type, info.nonNullableType); - b.local_get(argLocal); - translator.convertType( - b, argLocal.type, info.struct.fields[fieldIndex].type.unpacked); + b.local_get(positionalArgLocal); + translator.convertType(b, positionalArgLocal.type, + info.struct.fields[fieldIndex].type.unpacked); b.struct_set(info.struct, fieldIndex); } else { final setterProcedure = member_ as Procedure; @@ -3436,12 +3410,12 @@ assert(setterWasmInputs.length == 2); b.local_get(receiverLocal); translator.convertType(b, receiverLocal.type, setterWasmInputs[0]); - b.local_get(argLocal); - translator.convertType(b, argLocal.type, setterWasmInputs[1]); + b.local_get(positionalArgLocal); + translator.convertType(b, positionalArgLocal.type, setterWasmInputs[1]); call(setterProcedure.reference); } - b.local_get(argLocal); + b.local_get(positionalArgLocal); b.end(); // end function } }
diff --git a/tests/web/wasm/flutter_regress_152029_test.dart b/tests/web/wasm/flutter_regress_152029_test.dart new file mode 100644 index 0000000..f218730 --- /dev/null +++ b/tests/web/wasm/flutter_regress_152029_test.dart
@@ -0,0 +1,20 @@ +// 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. + +// dart2wasmOptions=--extra-compiler-option=--omit-implicit-checks + +import 'package:expect/expect.dart'; + +class A<T> { + T? value = null; +} + +main() { + final list = [A<String>(), 1]; + final dynamic a = list[int.parse('0')]; + if (int.parse('0') == 0) { + a.value = '42'; + } + Expect.equals('42', a.value); +}