Skip to content

Commit 2f46eaa

Browse files
natebiggsCommit Queue
authored andcommitted
[ddc] Fix async rewrite ignoring new ScopedId variables.
The async rewriter was ignoring these variables since they were only used for non-user variables that didn't need to be captured. Now that we use these ScopedIds for user variables we need to capture some of them. "userDefined" specifies if the variable should be captured. Change-Id: I2dbb86e1834982b59883a3d107612ee8f8e2284a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400662 Commit-Queue: Nate Biggs <[email protected]> Reviewed-by: Nicholas Shahan <[email protected]>
1 parent 5c1510d commit 2f46eaa

File tree

5 files changed

+47
-9
lines changed

5 files changed

+47
-9
lines changed

pkg/dev_compiler/lib/src/compiler/js_names.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,20 @@ abstract class FixedNames {
3333
class ScopedId extends Identifier {
3434
final int _id;
3535

36+
/// If true, this variable can be used across different async scopes and must
37+
/// therefore be specially captured.
38+
final bool needsCapture;
39+
3640
@override
3741
ScopedId withSourceInformation(dynamic sourceInformation) =>
3842
ScopedId.from(this)..sourceInformation = sourceInformation;
3943

4044
static int _idCounter = 0;
4145

42-
ScopedId(super.name) : _id = _idCounter++;
46+
ScopedId(super.name, {this.needsCapture = false}) : _id = _idCounter++;
4347
ScopedId.from(ScopedId other)
4448
: _id = other._id,
49+
needsCapture = other.needsCapture,
4550
super(other.name);
4651

4752
@override

pkg/dev_compiler/lib/src/compiler/rewrite_async.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3258,14 +3258,14 @@ class _ScopeCollector extends js_ast.VariableDeclarationVisitor {
32583258

32593259
@override
32603260
void declare(js_ast.Identifier node) {
3261-
if (node is ScopedId) return;
3261+
if (node is ScopedId && !node.needsCapture) return;
32623262
_currentScope.declare(node, inClosure || skipHoisting);
32633263
registerUsed(node);
32643264
}
32653265

32663266
@override
32673267
void visitIdentifier(js_ast.Identifier node) {
3268-
if (node is ScopedId) return;
3268+
if (node is ScopedId && !node.needsCapture) return;
32693269
registerUsed(node);
32703270
}
32713271

pkg/dev_compiler/lib/src/kernel/compiler.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,8 +1035,9 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
10351035
static js_ast.Identifier _emitIdentifier(String name) =>
10361036
js_ast.Identifier(js_ast.toJSIdentifier(name));
10371037

1038-
static js_ast.ScopedId _emitScopedId(String name) =>
1039-
js_ast.ScopedId(js_ast.toJSIdentifier(name));
1038+
static js_ast.ScopedId _emitScopedId(String name,
1039+
{bool needsCapture = false}) =>
1040+
js_ast.ScopedId(js_ast.toJSIdentifier(name), needsCapture: needsCapture);
10401041

10411042
js_ast.Statement _emitClassDeclaration(Class c) {
10421043
var className = _emitTopLevelNameNoExternalInterop(c);
@@ -4988,7 +4989,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
49884989
// See https://github.com/dart-lang/sdk/issues/55918
49894990
name = extractLocalNameFromLateLoweredLocal(name);
49904991
}
4991-
return js_ast.ScopedId.from(_variableTempIds[v] ??= _emitScopedId(name));
4992+
return js_ast.ScopedId.from(
4993+
_variableTempIds[v] ??= _emitScopedId(name, needsCapture: true));
49924994
}
49934995

49944996
/// Emits the declaration of a variable.

pkg/dev_compiler/lib/src/kernel/compiler_new.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,8 +1204,9 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
12041204
static js_ast.Identifier _emitIdentifier(String name) =>
12051205
js_ast.Identifier(js_ast.toJSIdentifier(name));
12061206

1207-
static js_ast.ScopedId _emitScopedId(String name) =>
1208-
js_ast.ScopedId(js_ast.toJSIdentifier(name));
1207+
static js_ast.ScopedId _emitScopedId(String name,
1208+
{bool needsCapture = false}) =>
1209+
js_ast.ScopedId(js_ast.toJSIdentifier(name), needsCapture: needsCapture);
12091210

12101211
js_ast.Statement _emitClassDeclaration(Class c) {
12111212
var className = _emitTopLevelNameNoExternalInterop(c);
@@ -5308,7 +5309,8 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
53085309
// See https://github.com/dart-lang/sdk/issues/55918
53095310
name = extractLocalNameFromLateLoweredLocal(name);
53105311
}
5311-
return js_ast.ScopedId.from(_variableTempIds[v] ??= _emitScopedId(name));
5312+
return js_ast.ScopedId.from(
5313+
_variableTempIds[v] ??= _emitScopedId(name, needsCapture: true));
53125314
}
53135315

53145316
/// Emits the declaration of a variable.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:expect/async_helper.dart';
6+
import 'package:expect/expect.dart';
7+
8+
int global = -1;
9+
10+
Future<void> staticAsyncLoopFunction(String value) async {
11+
Function? f;
12+
for (var i in [1, 2, 3]) {
13+
print(value);
14+
final myLocal = await 'my local value';
15+
f ??= () {
16+
// This should capture the value from the first loop iteration.
17+
global = i;
18+
return myLocal;
19+
};
20+
}
21+
f!();
22+
}
23+
24+
void main() async {
25+
asyncStart();
26+
await staticAsyncLoopFunction('foo');
27+
Expect.equals(global, 1);
28+
asyncEnd();
29+
}

0 commit comments

Comments
 (0)