Skip to content

Commit

Permalink
[stable][dart2js] Fix late local lowering with record destructuring.
Browse files Browse the repository at this point in the history
The CFE's lowering for record patterns can sometimes make use of late local fields. Dart2JS's late lowerer assumes that all late local fields live in the scope of some function body. Record patterns allow late locals to be created in other contexts though. Some examples that break this assumption include field initializers and expressions in constructor initializer lists.

To fix this we add a late local scope in some extra contexts where these record patterns can show up.

Fixes: 53358

Bug: #53358
Change-Id: Ic730f41253782241b3394c0b0353d1731e122c85
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/323020
Cherry-pick-request: #53449
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324600
Reviewed-by: Sigmund Cherem <[email protected]>
Commit-Queue: Nate Biggs <[email protected]>
  • Loading branch information
natebiggs authored and Commit Queue committed Sep 6, 2023
1 parent efd81da commit d5a6568
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 5 deletions.
13 changes: 12 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
## 3.1.2

This is a patch release that:

- Fixes a bug in dart2js which crashed the compiler when a typed record pattern
was used outside the scope of a function body, such as in a field initializer.
For example `final x = { for (var (int a,) in someList) a: a };`
(issue [#53358])

[#53358]: https://github.com/dart-lang/sdk/issues/53358

## 3.1.1 - 2023-09-07

This is a patch release that:
Expand Down Expand Up @@ -8504,4 +8515,4 @@ documentation on the [Dart API site](http://api.dart.dev).
compression.

- `dart:isolate`: `Isolate.spawnUri` added the optional `packageRoot` argument,
which controls how it resolves `package:` URIs.
which controls how it resolves `package:` URIs.
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,11 @@ class LateLowering {
_backingInstanceFields.clear();
}

void enterFunction() {
void enterScope() {
_variableCells.add(null);
}

void exitFunction() {
void exitScope() {
_variableCells.removeLast();
}

Expand Down
26 changes: 24 additions & 2 deletions pkg/compiler/lib/src/kernel/transformations/lowering.dart
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ class _Lowering extends Transformer {

@override
TreeNode visitFunctionNode(FunctionNode node) {
_lateLowering.enterFunction();
_lateLowering.enterScope();
_asyncLowering?.enterFunction(node);
node.transformChildren(this);
_lateLowering.exitFunction();
_lateLowering.exitScope();
_asyncLowering?.transformFunctionNodeAndExit(node);
return node;
}
Expand All @@ -88,10 +88,32 @@ class _Lowering extends Transformer {
@override
TreeNode visitField(Field node) {
_currentMember = node;
// Field initializers can contain late variable reads via record
// destructuring. The following patten can result in the CFE using a late
// local outside the scope of a function:
// Object? foo = { for (final (String x,) in records) x: x };
_lateLowering.enterScope();
node.transformChildren(this);
_lateLowering.exitScope();
return _lateLowering.transformField(node, _currentMember!);
}

@override
TreeNode visitConstructor(Constructor node) {
// Constructor initializers can contain late variable reads via record
// destructuring. Any of these patterns can result in the CFE using a
// late local outside the scope of a function:
// Foo() : super({ for (final (String x,) in records) x: x });
// Foo() : foo = { for (final (String x,) in records) x: x };
//
// We share the scope between the various initializers since variables
// cannot leak between them.
_lateLowering.enterScope();
super.visitConstructor(node);
_lateLowering.exitScope();
return node;
}

@override
TreeNode visitAwaitExpression(AwaitExpression expression) {
_asyncLowering?.visitAwaitExpression(expression);
Expand Down
33 changes: 33 additions & 0 deletions tests/web/regress/issue/53358_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright (c) 2023, 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.

List<(int,)> records = [(1,), (2,)];

// Library field initializer
Map<int, int> topLevel = {for (final (int x,) in records) x: x};

class Bar {
final Map<int, int> bar;

Bar(this.bar);
}

class Foo extends Bar {
// Class field initializer
final Map<int, int> foo1 = {for (final (int x,) in records) x: x};
final Map<int, int> foo2;
Foo()
// Constructor field initializer
: foo2 = {for (final (int x,) in records) x: x},
// Super initializer
super({for (final (int x,) in records) x: x});
}

void main() {
print(topLevel);
final foo = Foo();
print(foo.foo1);
print(foo.foo2);
print(foo.bar);
}

0 comments on commit d5a6568

Please sign in to comment.