From 6edbfd93ab9c128ee06c5ddec92f2dade44736a6 Mon Sep 17 00:00:00 2001 From: MarkZ Date: Thu, 23 Jan 2025 14:10:59 -0800 Subject: [PATCH] [ddc] Updating representation of enums to support hot reload semantics. Enums are now split between pre and post canonicalization members. A separate operation during link-time is emitted for every const enum field. These are required during link time since enhanced enums can have non-trivial type hierarchies. Example for snippet: ``` enum E { e1(1), ... const E(this.i); final int i; } ``` Used to emit enum fields as: ``` dart.defineLazy(CT, { get C1() { return C[1] = dart.const(Object.setPrototypeOf({ [_Enum__name]: "e1", [_Enum_index]: 0 i: 1 }, E.prototype)); }, } ``` Now emits them as: ``` // Declaration-time dart.defineLazy(CT, { get C1() { return C[1] = dart.const(Object.setPrototypeOf({ [_Enum__name]: "e1", }, E.prototype)); }, } // Link-time dart.extendEnum(dart.const(Object.setPrototypeOf({ [_Enum__name]: "e1" }, E.prototype)), { i: 1, get index() { return E.values.indexOf(this); } }); ``` Change-Id: Id5ce2ec117e59d8daa28df7fe6051e4a7e1a5bc1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404723 Commit-Queue: Mark Zhou Reviewed-by: Nicholas Shahan --- .../lib/src/kernel/compiler_new.dart | 86 +++++++++++++++++-- .../private/ddc_runtime/operations.dart | 13 ++- 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/pkg/dev_compiler/lib/src/kernel/compiler_new.dart b/pkg/dev_compiler/lib/src/kernel/compiler_new.dart index 24018c8f90e..9002e0af08c 100644 --- a/pkg/dev_compiler/lib/src/kernel/compiler_new.dart +++ b/pkg/dev_compiler/lib/src/kernel/compiler_new.dart @@ -348,6 +348,9 @@ class LibraryCompiler extends ComputeOnceConstantVisitor /// Library link method statements that create type rules. final List _typeRuleLinks = []; + /// Holds additional initialization logic for enum fields. + final List _enumExtensions = []; + /// Whether the current function needs to insert parameter checks. /// /// Used to avoid adding checks for formal parameters inside a synthetic @@ -1011,6 +1014,8 @@ class LibraryCompiler extends ComputeOnceConstantVisitor ..._defineExtensionMemberLinks, ..._nativeExtensionLinks, ..._typeRuleLinks, + // Enum extensions must be emitted after type hierachies have stabilized. + ..._enumExtensions ]); var function = js_ast.NamedFunction(functionName, js_ast.Fun(parameters, body)); @@ -7942,6 +7947,72 @@ class LibraryCompiler extends ComputeOnceConstantVisitor ]); } + js_ast.Expression visitEnum(InstanceConstant node) { + // Non-nullable is forced here because the type of an instance constant + // should never appear as legacy "*" at runtime but the library where the + // constant is defined can cause those types to appear here. + var type = node + .getType(_staticTypeContext) + .withDeclaredNullability(Nullability.nonNullable); + var classRef = _emitClassRef(type as InterfaceType); + var prototype = js.call('#.prototype', [classRef]); + var enumAccessor = _emitTopLevelName(node.classNode); + + // Enums are canonicalized based on their 'name' member alone. We + // append other members (such as 'index' and those introduced via enhanced + // enums) after canonicalization so they can be updated across hot reloads. + var constantProperties = []; + var additionalProperties = []; + if (type.typeArguments.isNotEmpty) { + // Generic interface type instances require a type information tag. + var property = js_ast.Property( + _propertyName(js_ast.FixedNames.rtiName), _emitType(type)); + constantProperties.add(property); + } + node.fieldValues.forEach((k, v) { + var constant = visitConstant(v); + var member = k.asField; + var memberClass = member.enclosingClass!; + if (!memberClass.isEnum) { + if (member.name.text == 'index') { + // We transform the 'index' field of Enum fields into a special + // getter so that their indices are consistent across hot reloads. + var value = js.call('#.values.indexOf(this)', enumAccessor); + var jsMember = _getSymbol(_emitClassPrivateNameSymbol( + memberClass.enclosingLibrary, + getLocalClassName(memberClass), + member)); + additionalProperties.add(js_ast.Method( + jsMember, js.fun('function() { return #; }', [value]), + isGetter: true)); + } else { + var jsMember = _getSymbol(_emitClassPrivateNameSymbol( + memberClass.enclosingLibrary, + getLocalClassName(memberClass), + member)); + constantProperties.add(js_ast.Property(jsMember, constant)); + } + } else { + additionalProperties.add(js_ast.Property( + _emitMemberName(member.name.text, member: member), constant)); + } + }); + + var canonicalizedEnum = _canonicalizeConstObject( + _emitJSObjectSetPrototypeOf( + js_ast.ObjectInitializer(constantProperties, multiline: true), + prototype, + fullyQualifiedName: false)); + + var enumExtension = _runtimeStatement('extendEnum(#, #)', [ + canonicalizedEnum, + js_ast.ObjectInitializer(additionalProperties, multiline: true) + ]); + _enumExtensions.add(enumExtension); + + return canonicalizedEnum; + } + @override js_ast.Expression visitInstanceConstant(InstanceConstant node) { var savedTypeEnvironment = _currentTypeEnvironment; @@ -7950,17 +8021,18 @@ class LibraryCompiler extends ComputeOnceConstantVisitor ClassTypeEnvironment(node.classNode.typeParameters); } + if (node.classNode.isEnum) { + var constant = visitEnum(node); + _currentTypeEnvironment = savedTypeEnvironment; + return constant; + } + js_ast.Property entryToProperty(MapEntry entry) { var constant = visitConstant(entry.value); var member = entry.key.asField; var cls = member.enclosingClass!; - // Enums cannot be overridden, so we can safely use the field name - // directly. Otherwise, use a private symbol in case the field - // was overridden. - var symbol = cls.isEnum - ? _emitMemberName(member.name.text, member: member) - : _getSymbol(_emitClassPrivateNameSymbol( - cls.enclosingLibrary, getLocalClassName(cls), member)); + var symbol = _getSymbol(_emitClassPrivateNameSymbol( + cls.enclosingLibrary, getLocalClassName(cls), member)); return js_ast.Property(symbol, constant); } diff --git a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart index cf552819126..bd7c9a7662a 100644 --- a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart +++ b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart @@ -1375,7 +1375,8 @@ bool isStateBearingSymbol(property) => JS( property, ); -/// Attempts to assign class [classDeclaration] as [classIdentifier] on [library]. +/// Attempts to assign class [classDeclaration] as [classIdentifier] on +/// [library]. /// /// During a hot reload, should [library.classIdentifier] already exist, this /// copies the members of [classDeclaration] and its prototype's properties to @@ -1428,3 +1429,13 @@ declareTopLevelProperties(topLevelContainer, propertiesObject) { copyProperties(topLevelContainer, propertiesObject, copyWhen: copyWhen); return topLevelContainer; } + +/// Appends const members in [additionalFieldsObject] to [canonicalizedEnum]. +/// +/// [additionalFieldsObject] is a JS object containing fields that should not +/// be considered for enum identity/equality but may be updated after a hot +/// reload. +extendEnum(canonicalizedEnum, additionalFieldsObject) { + copyProperties(canonicalizedEnum, additionalFieldsObject); + return canonicalizedEnum; +}