Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 7d6024b

Browse files
MarkzipanMarkzipan
committed
[dartdevc] Fix for DDC overzealously expanding nested constants
Fixes dart-lang/sdk#37523 Change-Id: I140d7d8fdce836dc0f7f2961711d70ec965e46d4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109640 Reviewed-by: Nicholas Shahan <[email protected]>
1 parent db25ef8 commit 7d6024b

File tree

1 file changed

+114
-34
lines changed

1 file changed

+114
-34
lines changed

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

Lines changed: 114 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,12 @@ import 'nullable_inference.dart';
3232
import 'property_model.dart';
3333
import 'type_table.dart';
3434

35-
class ProgramCompiler extends Object
35+
class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
3636
with SharedCompiler<Library, Class, InterfaceType, FunctionNode>
3737
implements
3838
StatementVisitor<js_ast.Statement>,
3939
ExpressionVisitor<js_ast.Expression>,
40-
DartTypeVisitor<js_ast.Expression>,
41-
ConstantVisitor<js_ast.Expression> {
40+
DartTypeVisitor<js_ast.Expression> {
4241
final SharedCompilerOptions _options;
4342

4443
/// Maps a library URI import, that is not in [_libraries], to the
@@ -62,6 +61,16 @@ class ProgramCompiler extends Object
6261
/// Let variables collected for the given function.
6362
List<js_ast.TemporaryId> _letVariables;
6463

64+
final _constTable = js_ast.TemporaryId("CT");
65+
66+
// Constant getters used to populate the constant table.
67+
final _constLazyAccessors = List<js_ast.Method>();
68+
69+
/// Tracks the index in [moduleItems] where the const table must be inserted.
70+
/// Required for SDK builds due to internal circular dependencies.
71+
/// E.g., dart.constList depends on JSArray.
72+
int _constTableInsertionIndex = 0;
73+
6574
/// The class that is emitting its base class or mixin references, otherwise
6675
/// null.
6776
///
@@ -185,6 +194,9 @@ class ProgramCompiler extends Object
185194
/// as targets for continue and break.
186195
final _switchLabelStates = HashMap<Statement, _SwitchLabelState>();
187196

197+
/// Maps Kernel constants to their JS aliases.
198+
final constAliasCache = HashMap<Constant, js_ast.Expression>();
199+
188200
final Class _jsArrayClass;
189201
final Class _privateSymbolClass;
190202
final Class _linkedHashMapImplClass;
@@ -287,6 +299,11 @@ class ProgramCompiler extends Object
287299
_pendingClasses.addAll(l.classes);
288300
}
289301

302+
// TODO(markzipan): Don't emit this when compiling the SDK.
303+
moduleItems
304+
.add(js.statement("const # = Object.create(null);", [_constTable]));
305+
_constTableInsertionIndex = moduleItems.length;
306+
290307
// Add implicit dart:core dependency so it is first.
291308
emitLibraryName(_coreTypes.coreLibrary);
292309

@@ -297,6 +314,15 @@ class ProgramCompiler extends Object
297314
// This is done by forward declaring items.
298315
libraries.forEach(_emitLibrary);
299316

317+
// This can cause problems if it's ever true during the SDK build, as it's
318+
// emitted before dart.defineLazy.
319+
if (_constLazyAccessors.isNotEmpty) {
320+
var constTableBody = runtimeStatement(
321+
'defineLazy(#, { # })', [_constTable, _constLazyAccessors]);
322+
moduleItems.insert(_constTableInsertionIndex, constTableBody);
323+
_constLazyAccessors.clear();
324+
}
325+
300326
moduleItems.addAll(afterClassDefItems);
301327
afterClassDefItems.clear();
302328

@@ -445,6 +471,11 @@ class ProgramCompiler extends Object
445471

446472
moduleItems.add(_emitClassDeclaration(c));
447473

474+
// The const table depends on dart.defineLazy, so emit it after the SDK.
475+
if (isSdkInternalRuntime(_currentLibrary)) {
476+
_constTableInsertionIndex = moduleItems.length;
477+
}
478+
448479
_currentClass = savedClass;
449480
_types.thisType = savedClass?.thisType;
450481
_currentLibrary = savedLibrary;
@@ -2083,12 +2114,11 @@ class ProgramCompiler extends Object
20832114
}
20842115

20852116
js_ast.Fun _emitStaticFieldInitializer(Field field) {
2086-
return js_ast.Fun(
2087-
[],
2088-
js_ast.Block(_withLetScope(() => [
2089-
js_ast.Return(
2090-
_visitInitializer(field.initializer, field.annotations))
2091-
])));
2117+
return js_ast.Fun([], js_ast.Block(_withLetScope(() {
2118+
return [
2119+
js_ast.Return(_visitInitializer(field.initializer, field.annotations))
2120+
];
2121+
})));
20922122
}
20932123

20942124
List<js_ast.Statement> _withLetScope(List<js_ast.Statement> visitBody()) {
@@ -3101,6 +3131,9 @@ class ProgramCompiler extends Object
31013131

31023132
js_ast.Expression _visitExpression(Expression e) {
31033133
if (e == null) return null;
3134+
if (e is ConstantExpression) {
3135+
return visitConstant(e.constant);
3136+
}
31043137
var result = e.accept(this) as js_ast.Expression;
31053138
result.sourceInformation ??= _nodeStart(e);
31063139
return result;
@@ -3780,7 +3813,15 @@ class ProgramCompiler extends Object
37803813

37813814
@override
37823815
js_ast.Expression visitConstantExpression(ConstantExpression node) =>
3783-
node.constant.accept(this) as js_ast.Expression;
3816+
visitConstant(node.constant);
3817+
3818+
@override
3819+
js_ast.Expression canonicalizeConstObject(js_ast.Expression expr) {
3820+
if (isSdkInternalRuntime(_currentLibrary)) {
3821+
return super.canonicalizeConstObject(expr);
3822+
}
3823+
return runtimeCall('const(#)', [expr]);
3824+
}
37843825

37853826
@override
37863827
js_ast.Expression visitVariableGet(VariableGet node) {
@@ -4725,7 +4766,7 @@ class ProgramCompiler extends Object
47254766
if (isFromEnvironmentInvocation(_coreTypes, node)) {
47264767
var value = _constants.evaluate(node);
47274768
if (value is PrimitiveConstant) {
4728-
return value.accept(this) as js_ast.Expression;
4769+
return visitConstant(value);
47294770
}
47304771
}
47314772

@@ -4864,7 +4905,7 @@ class ProgramCompiler extends Object
48644905
node.accept(this);
48654906
if (node is ConstantExpression) {
48664907
var list = node.constant as ListConstant;
4867-
entries.addAll(list.entries.map(_visitConstant));
4908+
entries.addAll(list.entries.map(visitConstant));
48684909
} else if (node is ListLiteral) {
48694910
entries.addAll(node.expressions.map(_visitExpression));
48704911
}
@@ -4886,7 +4927,7 @@ class ProgramCompiler extends Object
48864927
node.accept(this);
48874928
if (node is ConstantExpression) {
48884929
var set = node.constant as SetConstant;
4889-
entries.addAll(set.entries.map(_visitConstant));
4930+
entries.addAll(set.entries.map(visitConstant));
48904931
} else if (node is SetLiteral) {
48914932
entries.addAll(node.expressions.map(_visitExpression));
48924933
}
@@ -4909,8 +4950,8 @@ class ProgramCompiler extends Object
49094950
if (node is ConstantExpression) {
49104951
var map = node.constant as MapConstant;
49114952
for (var entry in map.entries) {
4912-
entries.add(_visitConstant(entry.key));
4913-
entries.add(_visitConstant(entry.value));
4953+
entries.add(visitConstant(entry.key));
4954+
entries.add(visitConstant(entry.value));
49144955
}
49154956
} else if (node is MapLiteral) {
49164957
for (var entry in node.entries) {
@@ -5032,11 +5073,7 @@ class ProgramCompiler extends Object
50325073
js_ast.Expression visitListLiteral(ListLiteral node) {
50335074
var elementType = node.typeArgument;
50345075
var elements = _visitExpressionList(node.expressions);
5035-
// TODO(markzipan): remove const check when we use front-end const eval
5036-
if (!node.isConst) {
5037-
return _emitList(elementType, elements);
5038-
}
5039-
return _emitConstList(elementType, elements);
5076+
return _emitList(elementType, elements);
50405077
}
50415078

50425079
js_ast.Expression _emitList(
@@ -5270,8 +5307,48 @@ class ProgramCompiler extends Object
52705307
findAnnotation(node, test), 'name') as String;
52715308
}
52725309

5273-
js_ast.Expression _visitConstant(Constant node) =>
5274-
node.accept(this) as js_ast.Expression;
5310+
@override
5311+
js_ast.Expression cacheConst(js_ast.Expression jsExpr) {
5312+
if (isSdkInternalRuntime(_currentLibrary)) {
5313+
return super.cacheConst(jsExpr);
5314+
}
5315+
return jsExpr;
5316+
}
5317+
5318+
@override
5319+
js_ast.Expression visitConstant(Constant node) {
5320+
if (node is TearOffConstant) {
5321+
// JS() or JS interop functions should not be lazily loaded.
5322+
if (node.procedure.isExternal || _isInForeignJS) {
5323+
return _emitStaticTarget(node.procedure);
5324+
}
5325+
}
5326+
if (isSdkInternalRuntime(_currentLibrary) || node is PrimitiveConstant) {
5327+
return super.visitConstant(node);
5328+
}
5329+
var constAlias = constAliasCache[node];
5330+
if (constAlias != null) {
5331+
return constAlias;
5332+
}
5333+
var constAliasString = "C${constAliasCache.length}";
5334+
var constAliasProperty = propertyName(constAliasString);
5335+
var constAliasId = js_ast.TemporaryId(constAliasString);
5336+
var constAccessor =
5337+
js.call("# || #.#", [constAliasId, _constTable, constAliasProperty]);
5338+
constAliasCache[node] = constAccessor;
5339+
var constJs = super.visitConstant(node);
5340+
moduleItems.add(js.statement('let #;', [constAliasId]));
5341+
5342+
var func = js_ast.Fun(
5343+
[],
5344+
js_ast.Block([
5345+
js.statement("return # = #;", [constAliasId, constJs])
5346+
]));
5347+
var accessor = js_ast.Method(constAliasProperty, func, isGetter: true);
5348+
_constLazyAccessors.add(accessor);
5349+
return constAccessor;
5350+
}
5351+
52755352
@override
52765353
js_ast.Expression visitNullConstant(NullConstant node) =>
52775354
js_ast.LiteralNull();
@@ -5324,25 +5401,26 @@ class ProgramCompiler extends Object
53245401
js_ast.Expression visitMapConstant(MapConstant node) {
53255402
var entries = [
53265403
for (var e in node.entries) ...[
5327-
_visitConstant(e.key),
5328-
_visitConstant(e.value),
5404+
visitConstant(e.key),
5405+
visitConstant(e.value),
53295406
],
53305407
];
53315408
return _emitConstMap(node.keyType, node.valueType, entries);
53325409
}
53335410

53345411
@override
53355412
js_ast.Expression visitListConstant(ListConstant node) => _emitConstList(
5336-
node.typeArgument, node.entries.map(_visitConstant).toList());
5413+
node.typeArgument, node.entries.map(visitConstant).toList());
53375414

53385415
@override
53395416
js_ast.Expression visitSetConstant(SetConstant node) => _emitConstSet(
5340-
node.typeArgument, node.entries.map(_visitConstant).toList());
5417+
node.typeArgument, node.entries.map(visitConstant).toList());
53415418

53425419
@override
53435420
js_ast.Expression visitInstanceConstant(InstanceConstant node) {
5421+
_declareBeforeUse(node.classNode);
53445422
entryToProperty(MapEntry<Reference, Constant> entry) {
5345-
var constant = entry.value.accept(this) as js_ast.Expression;
5423+
var constant = visitConstant(entry.value);
53465424
var member = entry.key.asField;
53475425
return js_ast.Property(
53485426
_emitMemberName(member.name.name, member: member), constant);
@@ -5359,20 +5437,22 @@ class ProgramCompiler extends Object
53595437
}
53605438

53615439
@override
5362-
js_ast.Expression visitTearOffConstant(TearOffConstant node) =>
5363-
_emitStaticGet(node.procedure);
5440+
js_ast.Expression visitTearOffConstant(TearOffConstant node) {
5441+
_declareBeforeUse(node.procedure.enclosingClass);
5442+
return _emitStaticGet(node.procedure);
5443+
}
53645444

53655445
@override
53665446
js_ast.Expression visitTypeLiteralConstant(TypeLiteralConstant node) =>
53675447
_emitTypeLiteral(node.type);
53685448

53695449
@override
53705450
js_ast.Expression visitPartialInstantiationConstant(
5371-
PartialInstantiationConstant node) =>
5372-
runtimeCall('gbind(#, #)', [
5373-
_visitConstant(node.tearOffConstant),
5374-
node.types.map(_emitType).toList()
5375-
]);
5451+
PartialInstantiationConstant node) =>
5452+
runtimeCall('gbind(#, #)', [
5453+
visitConstant(node.tearOffConstant),
5454+
node.types.map(_emitType).toList()
5455+
]);
53765456

53775457
@override
53785458
js_ast.Expression visitUnevaluatedConstant(UnevaluatedConstant node) =>

0 commit comments

Comments
 (0)