From c552af0a9eb9b7d4d78331d5ea9b10e5d92bfd97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20A=C4=9Facan?= Date: Wed, 7 Jan 2026 03:38:06 -0800 Subject: [PATCH] [dart2wasm] Minor refactoring and comments in variable declaration compiler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactoring: - Update the checks when updating a local for a captured variable to check whether `local` is null, instead of whether it's not updated. If the `local` is available then we know that it's not updated. It's more direct to check whether we've created a local for the variable or not. - Add an assertion checking the the capture field and local for a variable can only differ in nullability. Documentation: - Document that context field for a captured local will always be nullable, to be able to allocate the context without dummy values. - Document in a few places that `!capture.written` means the variable is captured but not updated, so they can be held in a local (instead of getting them from the context on every read). Change-Id: I66048cb36f75e35ee3c479c41f2bbfca247a990f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/470981 Reviewed-by: Martin Kustermann Commit-Queue: Ömer Ağacan --- pkg/dart2wasm/lib/code_generator.dart | 31 ++++++++++++----- tests/language/local_null_initialization.dart | 34 +++++++++++++++++++ 2 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 tests/language/local_null_initialization.dart diff --git a/pkg/dart2wasm/lib/code_generator.dart b/pkg/dart2wasm/lib/code_generator.dart index 29523f9a14e..d6b1e7409e5 100644 --- a/pkg/dart2wasm/lib/code_generator.dart +++ b/pkg/dart2wasm/lib/code_generator.dart @@ -792,24 +792,34 @@ abstract class AstCodeGenerator void visitVariableDeclaration(VariableDeclaration node) { final w.ValueType type = translator.translateTypeOfLocalVariable(node); w.Local? local; - Capture? capture = closures.captures[node]; + final Capture? capture = closures.captures[node]; if (capture == null || !capture.written) { + // Variable is not captured, or never updated after initialization. Keep + // the value in a local. local = addLocal(type, name: node.name); locals[node] = local; } - // Handle variable initialization. Nullable variables have an implicit - // initializer. + // Handle variable initialization. Nullable variables don't get an + // initializer in kernel, but they still need to be initialized as `null`, + // to reset the variables in loops to the initial value, intead of reusing + // the last value from the previous iteration. This is tested in + // `tests/language/local_null_initialization.dart`. if (node.initializer != null || node.type.nullability == Nullability.nullable) { Expression initializer = node.initializer ?? ConstantExpression(NullConstant()); if (capture != null) { - w.ValueType expectedType = capture.written ? capture.type : local!.type; + // Type for the variable in context will always be nullable, to be able + // to allocate the context without creating dummy values. Nullability of + // the local's type will depend on the Dart type. + assert( + local == null || local.type.withNullability(true) == capture.type); + w.ValueType expectedType = local != null ? local.type : capture.type; b.local_get(capture.context.currentLocal); translateExpression(initializer, expectedType); - if (!capture.written) { - b.local_tee(local!); + if (local != null) { + b.local_tee(local); } b.struct_set(capture.context.struct, capture.fieldIndex); } else { @@ -817,7 +827,8 @@ abstract class AstCodeGenerator b.local_set(local); } } else if (local != null && !local.type.defaultable) { - // Uninitialized variable + // Uninitialized variable. We don't need to update the context when the + // variable is captured as the context is already initialized. translator .getDummyValuesCollectorForModule(b.moduleBuilder) .instantiateDummyValue(b, local.type); @@ -836,6 +847,8 @@ abstract class AstCodeGenerator w.Local? local; final Capture? capture = closures.captures[node]; if (capture == null || !capture.written) { + // Variable is not captured, or never updated after initialization. Keep + // the value in a local. local = addLocal(type, name: node.name); locals[node] = local; } @@ -843,8 +856,8 @@ abstract class AstCodeGenerator if (capture != null) { b.local_get(capture.context.currentLocal); pushInitialValue(); - if (!capture.written) { - b.local_tee(local!); + if (local != null) { + b.local_tee(local); } b.struct_set(capture.context.struct, capture.fieldIndex); } else { diff --git a/tests/language/local_null_initialization.dart b/tests/language/local_null_initialization.dart new file mode 100644 index 00000000000..47b009d072a --- /dev/null +++ b/tests/language/local_null_initialization.dart @@ -0,0 +1,34 @@ +// Copyright (c) 2026, 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. + +// This test checks that variable declarations in loops are reset in each +// iteration. (the `replacement` variable below) +// +// Before this test, getting this wrong in dart2wasm only caused one test +// failure in a large `dart:convert` test. This test is smaller and checks the +// same thing. + +import 'package:expect/expect.dart'; + +const _TEST_INPUT = ""; + +List _convert(String text) { + List result = []; + for (var i = 0; i < text.length; i++) { + var ch = text[i]; + int? replacement; + switch (ch) { + case '<': + replacement = 1; + case '>': + replacement = 2; + } + result.add(replacement); + } + return result; +} + +void main() { + Expect.listEquals(_convert(""), [1, null, 2]); +}