Skip to content

Commit d7fc9ca

Browse files
nshahanCommit Queue
authored andcommitted
[ddc] Add get and set proto inlined helpers
The helper methods make it easier to call the JS version of `Object.getPrototypeOf()` and `Object.getPrototypeOf()` from the SDK libraries. The body gets inlined directly to avoid extra method calls in potentially hot code paths. Start using the setter when setting base and extension classes. Issue:#52372 Change-Id: I6ca70cbf1936f76f24c8843e51c1c47e9bfe659c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303009 Reviewed-by: Sigmund Cherem <sigmund@google.com> Commit-Queue: Nicholas Shahan <nshahan@google.com>
1 parent fd5dd87 commit d7fc9ca

File tree

5 files changed

+57
-6
lines changed

5 files changed

+57
-6
lines changed

pkg/_js_interop_checks/lib/js_interop_checks.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ class JsInteropChecks extends RecursiveVisitor {
9090
'_interceptors', // for ddc JS string
9191
'_native_typed_data',
9292
'_runtime', // for ddc types at runtime
93+
'_js_helper', // for ddc inlined helper methods
9394
'async',
9495
'core', // for environment constructors
9596
'html',

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,9 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
594594
/// True when [library] is the sdk internal library 'dart:_internal'.
595595
bool _isDartInternal(Library library) => isDartLibrary(library, '_internal');
596596

597+
/// True when [library] is the sdk internal library 'dart:_js_helper'.
598+
bool _isDartJsHelper(Library library) => isDartLibrary(library, '_js_helper');
599+
597600
/// True when [library] is the sdk internal library 'dart:_internal'.
598601
bool _isDartForeignHelper(Library library) =>
599602
isDartLibrary(library, '_foreign_helper');
@@ -6069,6 +6072,19 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
60696072
}
60706073
}
60716074
}
6075+
if (_isDartJsHelper(enclosingLibrary)) {
6076+
var name = target.name.text;
6077+
if (name == 'jsObjectGetPrototypeOf') {
6078+
var obj = node.arguments.positional.single;
6079+
return js.call('Object.getPrototypeOf(#)', _visitExpression(obj));
6080+
}
6081+
if (name == 'jsObjectSetPrototypeOf') {
6082+
var obj = node.arguments.positional.first;
6083+
var prototype = node.arguments.positional.last;
6084+
return js.call('Object.setPrototypeOf(#, #)',
6085+
[_visitExpression(obj), _visitExpression(prototype)]);
6086+
}
6087+
}
60726088
if (target.isExternal &&
60736089
target.isInlineClassMember &&
60746090
hasObjectLiteralAnnotation(target)) {

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/classes.dart

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -598,18 +598,19 @@ definePrimitiveHashCode(proto) {
598598
}
599599

600600
/// Link the extension to the type it's extending as a base class.
601-
setBaseClass(derived, base) {
602-
JS('', '#.prototype.__proto__ = #.prototype', derived, base);
601+
void setBaseClass(@notNull Object derived, @notNull Object base) {
602+
jsObjectSetPrototypeOf(
603+
JS('', '#.prototype', derived), JS('', '#.prototype', base));
603604
// We use __proto__ to track the superclass hierarchy (see isSubtypeOf).
604-
JS('', '#.__proto__ = #', derived, base);
605+
jsObjectSetPrototypeOf(derived, base);
605606
}
606607

607608
/// Like [setBaseClass], but for generic extension types such as `JSArray<E>`.
608-
setExtensionBaseClass(dartType, jsType) {
609+
void setExtensionBaseClass(@notNull Object dartType, @notNull Object jsType) {
609610
// Mark the generic type as an extension type and link the prototype objects.
610-
var dartProto = JS('', '#.prototype', dartType);
611+
var dartProto = JS<Object>('!', '#.prototype', dartType);
611612
JS('', '#[#] = #', dartProto, _extensionType, dartType);
612-
JS('', '#.__proto__ = #.prototype', dartProto, jsType);
613+
jsObjectSetPrototypeOf(dartProto, JS('', '#.prototype', jsType));
613614
}
614615

615616
/// Adds type test predicates to a class/interface type [ctor], using the

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/runtime.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import 'dart:_js_helper'
3939
DeferredNotLoadedError,
4040
TypeErrorImpl,
4141
JsLinkedHashMap,
42+
jsObjectSetPrototypeOf,
4243
ImmutableMap,
4344
Primitives,
4445
PrivateSymbol,

sdk/lib/_internal/js_dev_runtime/private/js_helper.dart

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,3 +945,35 @@ rti.Rti getRtiForRecord(Object? record) {
945945
// TODO(48585): Move this class back to the dart:_rti library when old DDC
946946
// runtime type system has been removed.
947947
abstract class TrustedGetRuntimeType {}
948+
949+
/// Wraps the JavaScript `Object.getPrototypeOf()` method returning the
950+
/// `__proto__` of [obj].
951+
///
952+
/// This method is equivalent to:
953+
///
954+
/// JS<Object?>('', 'Object.getPrototypeOf(#)', obj);
955+
///
956+
/// but the code is generated by the compiler directly (a low-tech way of
957+
/// inlining).
958+
///
959+
/// This helper should always be used in place of `JS('', '#.__proto__', obj)`
960+
/// to avoid prototype pollution issues.
961+
/// See: https://github.com/tc39/proposal-symbol-proto
962+
external Object? jsObjectGetPrototypeOf(@notNull Object obj);
963+
964+
/// Wraps the JavaScript `Object.setPrototypeOf()` method setting the
965+
/// `__proto__` of [obj] to [prototype] and returning [obj].
966+
///
967+
/// This method is equivalent to:
968+
///
969+
/// JS<Object>('!', 'Object.setPrototypeOf(#, #)', obj, prototype);
970+
///
971+
/// but the code is generated by the compiler directly (a low-tech way of
972+
/// inlining).
973+
///
974+
/// This helper should always be used in place of
975+
/// `JS('', '#.__proto__ = #', obj, prototype)` to avoid prototype
976+
/// pollution issues.
977+
/// See: https://github.com/tc39/proposal-symbol-proto
978+
@notNull
979+
external Object jsObjectSetPrototypeOf(@notNull Object obj, Object? prototype);

0 commit comments

Comments
 (0)