[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);
+}
