Skip to content

Commit

Permalink
[ddc] Resolving link-time class members via the embedder.
Browse files Browse the repository at this point in the history 
Note: this may lead to memory leaks if all class declarations are persisted across hot reloads.

Fixes #59628

Change-Id: Iae82d6166602d6e4a005748e64e84b3bbbc81894
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403389
Reviewed-by: Nicholas Shahan <[email protected]>
Commit-Queue: Mark Zhou <[email protected]>
  • Loading branch information
@Markzipan
Markzipan authored and Commit Queue committed Jan 14, 2025
1 parent 330cc91 commit b5232ac
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 32 deletions.
101 changes: 69 additions & 32 deletions pkg/dev_compiler/lib/src/kernel/compiler_new.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1373,10 +1373,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
if (nonExternalProperties.isNotEmpty) {
// Note that this class has no heritage. This class should never be used
// as a type. It's merely a placeholder for static members.
var body = <js_ast.Statement>[
_emitClassStatement(c, className, null, nonExternalProperties)
.toStatement()
];
var body = _emitClassStatement(c, className, null, nonExternalProperties);
var typeFormals = c.typeParameters;
if (typeFormals.isNotEmpty) {
var genericClassStmts =
Expand All @@ -1398,19 +1395,40 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
];
}

js_ast.Statement _emitClassStatement(Class c, js_ast.Expression className,
js_ast.Expression? heritage, List<js_ast.Property> properties) {
var classIdentifier = _emitScopedId(getLocalClassName(c));
List<js_ast.Statement> _emitClassStatement(
Class c,
js_ast.Expression className,
js_ast.Expression? heritage,
List<js_ast.Property> properties) {
var localClassName = getLocalClassName(c);
var classIdentifier = _emitScopedId(localClassName);
var classDefIdentifier = _emitScopedId('_$localClassName');
// In order to ensure that 'super' binds to the correct target across hot
// reloads, we rebind its pre-assigned class definition's prototype during
// link phase (alongside the embedder-resolved class definition's).
var referencesSuperKeyword = hasSuper(c);
if (_options.emitDebugSymbols) classIdentifiers[c] = classIdentifier;
if (heritage != null) {
if (referencesSuperKeyword) {
_classExtendsLinks.add(_runtimeStatement(
'classExtends(#, #)', [classDefIdentifier, heritage]));
}
_classExtendsLinks
.add(_runtimeStatement('classExtends(#, #)', [className, heritage]));
}
var classExpr = js_ast.ClassExpression(classIdentifier, null, properties);
var libraryExpr = (className as js_ast.PropertyAccess).receiver;
var propertyExpr = className.selector;
return _runtimeStatement(
'declareClass(#, #, #)', [libraryExpr, propertyExpr, classExpr]);
return referencesSuperKeyword
? [
js.statement('let # = #;', [classDefIdentifier, classExpr]),
_runtimeStatement('declareClass(#, #, #)',
[libraryExpr, propertyExpr, classDefIdentifier])
]
: [
_runtimeStatement(
'declareClass(#, #, #)', [libraryExpr, propertyExpr, classExpr])
];
}

/// Like [_emitClassStatement] but emits a Dart 2.1 mixin represented by
Expand Down Expand Up @@ -1449,7 +1467,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
var staticProperties = properties.where((m) => m.isStatic).toList();
var instanceProperties = properties.where((m) => !m.isStatic).toList();

body.add(_emitClassStatement(c, className, heritage, staticProperties));
body.addAll(_emitClassStatement(c, className, heritage, staticProperties));
var superclassId = _emitScopedId(getLocalClassName(c.superclass!));
var classId = className is js_ast.Identifier
? className
Expand Down Expand Up @@ -1484,14 +1502,14 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
void _defineClass(Class c, js_ast.Expression className,
List<js_ast.Property> properties, List<js_ast.Statement> body) {
if (c == _coreTypes.objectClass) {
body.add(_emitClassStatement(c, className, null, properties));
body.addAll(_emitClassStatement(c, className, null, properties));
return;
}

js_ast.Expression emitClassRef(InterfaceType t) {
// TODO(jmesserly): investigate this. It seems like `lazyJSType` is
// invalid for use in an `extends` clause, hence this workaround.
return _emitJSInterop(t.classNode) ?? _emitClassRef(t);
js_ast.Expression emitClassRef(InterfaceType t,
{bool resolvedFromEmbedder = false}) {
return _emitJSInterop(t.classNode) ??
_emitClassRef(t, resolvedFromEmbedder: resolvedFromEmbedder);
}

// Find the real (user declared) superclass and the list of mixins.
Expand Down Expand Up @@ -1563,6 +1581,10 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>

// Unroll mixins.
var baseClass = emitClassRef(supertype);
// The SDK is never hot reloaded, so we can avoid the overhead of
// resolving their classes through the embedder.
var embedderResolvedBaseClass =
emitClassRef(supertype, resolvedFromEmbedder: !_isBuildingSdk);

// TODO(jmesserly): we need to unroll kernel mixins because the synthetic
// classes lack required synthetic members, such as constructors.
Expand Down Expand Up @@ -1622,19 +1644,25 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
js_ast.ClassExpression(
_emitScopedId(mixinName), null, forwardingMethodStubs)
]));
_classExtendsLinks
.add(_runtimeStatement('classExtends(#, #)', [mixinId, baseClass]));
_classExtendsLinks.add(_runtimeStatement(
'classExtends(#, #)', [mixinId, embedderResolvedBaseClass]));
emitMixinConstructors(mixinId, superclass, mixinClass, mixinType);
hasUnnamedSuper = hasUnnamedSuper || _hasUnnamedConstructor(mixinClass);
_mixinApplicationLinks.add(_runtimeStatement(
'applyMixin(#, #)', [mixinId, emitClassRef(mixinType)]));
// The SDK is never hot reloaded, so we can avoid the overhead of
// resolving their classes through the embedder.
_mixinApplicationLinks.add(_runtimeStatement('applyMixin(#, #)', [
mixinId,
emitClassRef(mixinType, resolvedFromEmbedder: !_isBuildingSdk)
]));
baseClass = mixinId;
embedderResolvedBaseClass = mixinId;
}

if (c.isMixinDeclaration && !c.isMixinClass) {
_emitMixinStatement(c, className, baseClass, properties, body);
} else {
body.add(_emitClassStatement(c, className, baseClass, properties));
body.addAll(_emitClassStatement(
c, className, embedderResolvedBaseClass, properties));
}

_classEmittingExtends = savedTopLevelClass;
Expand Down Expand Up @@ -3368,16 +3396,27 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
/// Emit the top-level name associated with [n], which should not be an
/// external interop member.
js_ast.PropertyAccess _emitTopLevelNameNoExternalInterop(NamedNode n,
{String suffix = ''}) {
{String suffix = '', bool resolvedFromEmbedder = false}) {
// Some native tests use top-level native methods.
var isTopLevelNative = n is Member && isNative(n);
return js_ast.PropertyAccess(
isTopLevelNative
? _runtimeCall('global.self')
: _emitLibraryName(getLibrary(n)),
: (resolvedFromEmbedder
? _emitEmbedderResolvedLibrary(getLibrary(n))
: _emitLibraryName(getLibrary(n))),
_emitTopLevelMemberName(n, suffix: suffix));
}

/// Emits [library] fully resolved via the Dart Dev Embedder.
///
/// Used when the 'current' hot reload's generation of a library needs to be
/// resolved.
js_ast.Expression _emitEmbedderResolvedLibrary(Library library) {
var libraryName = js.string('${library.importUri}');
return js.call('dartDevEmbedder.importLibrary(#)', [libraryName]);
}

/// Emits the member name portion of a top-level member.
///
/// NOTE: usually you should use [_emitTopLevelName] instead of this. This
Expand Down Expand Up @@ -3817,21 +3856,19 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
/// Note that for `package:js` types, this will emit the class we emitted
/// using `_emitJSInteropClassNonExternalMembers`, and not the runtime type
/// that we synthesize for `package:js` types.
js_ast.Expression _emitClassRef(InterfaceType type) {
if (!_emittingClassExtends && type.typeArguments.isNotEmpty) {
var genericName = _emitTopLevelNameNoExternalInterop(type.classNode);
return js.call('#', [genericName]);
}
return _emitTopLevelNameNoExternalInterop(type.classNode);
}
///
/// [resolvedFromEmbedder] looks up [type] via the embedder, which retrieves
/// the correct library in the context of hot reload. This should not be set
/// for external classes.
js_ast.Expression _emitClassRef(InterfaceType type,
{bool resolvedFromEmbedder = false}) =>
_emitTopLevelNameNoExternalInterop(type.classNode,
resolvedFromEmbedder: resolvedFromEmbedder);

Never _typeCompilationError(DartType type, String description) =>
throw UnsupportedError('$description Encountered while compiling '
'${_currentLibrary!.fileUri}, which contains the type: $type.');

bool get _emittingClassExtends =>
_currentClass != null && identical(_currentClass, _classEmittingExtends);

/// Emits an expression that lets you access statics on a [type] from code.
js_ast.Expression _emitConstructorName(InterfaceType type, Member c) {
var isSyntheticDefault =
Expand Down
36 changes: 36 additions & 0 deletions pkg/dev_compiler/lib/src/kernel/kernel_helpers.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,42 @@ class LabelContinueFinder extends RecursiveVisitor {
found = true;
}

/// Returns true if a class contains any 'super' calls.
bool hasSuper(Class node) {
var visitor = SuperFinder();
node.accept(visitor);
return visitor.found;
}

class SuperFinder extends RecursiveVisitor {
var found = false;

void visit(Statement? s) {
if (!found && s != null) s.accept(this);
}

@override
void visitAbstractSuperPropertyGet(AbstractSuperPropertyGet node) =>
found = true;

@override
void visitAbstractSuperPropertySet(AbstractSuperPropertySet node) =>
found = true;

@override
void visitSuperPropertyGet(SuperPropertyGet node) => found = true;

@override
void visitSuperPropertySet(SuperPropertySet node) => found = true;

@override
void visitAbstractSuperMethodInvocation(AbstractSuperMethodInvocation node) =>
found = true;

@override
void visitSuperMethodInvocation(SuperMethodInvocation node) => found = true;
}

/// Returns `true` if any of [n]s children are a [FunctionExpression] node.
bool containsFunctionExpression(Node n) {
var visitor = _FunctionExpressionFinder.instance;
Expand Down

0 comments on commit b5232ac

Please sign in to comment.