Skip to content

Commit

Permalink
[dart2js] Handle object literal constructors with no library @js anno…
Browse files Browse the repository at this point in the history
…tation

Fixes #54801

Object literal constructors need to be explicitly handled when
determining a member is JS interop or not in dart2js as it does
not require any @js annotations.

Change-Id: Iee99375439057844485aa3f5cd88f85f5d03ae06
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349840
Reviewed-by: Stephen Adams <[email protected]>
Commit-Queue: Srujan Gaddam <[email protected]>
  • Loading branch information
srujzs authored and Commit Queue committed Feb 20, 2024
1 parent 9fe6571 commit e9678f7
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 33 deletions.
14 changes: 13 additions & 1 deletion pkg/compiler/lib/src/kernel/native_basic_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,19 @@ class KernelAnnotationProcessor {
}
} else {
FunctionEntity function = member as FunctionEntity;
if (function.isExternal && isExplicitlyJsLibrary) {
// We need this explicit check as object literal constructors in
// extension types do not need an `@JS()` annotation on them, their
// extension type, or their library. JS interop checks assert that the
// only extension type interop member that has named parameters is an
// object literal constructor.
// TODO(54968): We should handle the lowering for object literal
// constructors in the interop transformer somehow instead and avoid
// assuming all such members are object literal constructors or
// otherwise paying the cost to verify by indexing extension types.
bool isObjectLiteralConstructor = (memberNode.isExtensionTypeMember &&
memberNode.function?.namedParameters.isNotEmpty == true);
if (function.isExternal &&
(isExplicitlyJsLibrary || isObjectLiteralConstructor)) {
// External members of explicit js-interop library are implicitly
// js-interop members.
memberName ??= function.name;
Expand Down
12 changes: 7 additions & 5 deletions pkg/compiler/lib/src/ssa/builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4151,18 +4151,20 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
List.from(_visitPositionalArguments(arguments));

if (target.namedParameters.isNotEmpty) {
// Only anonymous factory or inline class literal constructors involving
// Only anonymous factory or extension type literal constructors involving
// JS interop are allowed to have named parameters. Otherwise, throw an
// error.
final member = target.parent as ir.Member;
final function = _elementMap.getMember(member) as FunctionEntity;
bool isAnonymousFactory = function is ConstructorEntity &&
function.isFactoryConstructor &&
_nativeData.isAnonymousJsInteropClass(function.enclosingClass);
// JS interop checks assert that the only inline class interop member that
// has named parameters is an object literal constructor. We could do a
// more robust check by visiting all inline classes and recording
// descriptors, but that's expensive.
// JS interop checks assert that the only extension type interop member
// that has named parameters is an object literal constructor.
// TODO(54968): We should handle the lowering for object literal
// constructors in the interop transformer somehow instead and avoid
// assuming all such members are object literal constructors or
// otherwise paying the cost to verify by indexing extension types.
bool isObjectLiteralConstructor = member.isExtensionTypeMember;
if (isAnonymousFactory || isObjectLiteralConstructor) {
// TODO(sra): Have a "CompiledArguments" structure to just update with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,45 +2,38 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

// SharedOptions=--enable-experiment=inline-class

@JS()
library object_literal_constructor_test;

import 'dart:js_interop';
import 'dart:js_util';
import 'dart:js_interop_unsafe';

import 'package:expect/minitest.dart';

@JS()
extension type Literal._(JSObject _) implements Object {
extension type Literal._(JSObject _) implements JSObject {
external Literal({double? a, String b, bool? c});
external factory Literal.fact({double? a, String b, bool? c});
}

@JS('Object.keys')
external JSArray objectKeys(Literal literal);

void main() {
// Test that the properties we assumed to exist in `literal` actually exist
// and that their values are as expected. Note that we don't check the order
// of the keys in the literal. This is not guaranteed to be the same across
// different backends.
void testProperties(Literal literal, {double? a, String? b, bool? c}) {
if (a != null) {
expect(hasProperty(literal, 'a'), true);
expect(getProperty<double>(literal, 'a'), a);
}
if (b != null) {
expect(hasProperty(literal, 'b'), true);
expect(getProperty<String>(literal, 'b'), b);
}
if (c != null) {
expect(hasProperty(literal, 'c'), true);
expect(getProperty<bool>(literal, 'c'), c);
}
// Test that the properties we assumed to exist in `literal` actually exist and
// that their values are as expected. Note that we don't check the order of the
// keys in the literal. This is not guaranteed to be the same across different
// backends.
void testProperties(JSObject literal, {double? a, String? b, bool? c}) {
if (a != null) {
expect(literal.has('a'), true);
expect((literal['a'] as JSNumber).toDartDouble, a);
}
if (b != null) {
expect(literal.has('b'), true);
expect((literal['b'] as JSString).toDart, b);
}
if (c != null) {
expect(literal.has('c'), true);
expect((literal['c'] as JSBoolean).toDart, c);
}
}

void main() {
testProperties(Literal());
testProperties(Literal.fact(a: 0.0), a: 0.0);
testProperties(Literal(b: ''), b: '');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

// Validate that the library adding an annotation doesn't change the backend's
// lowerings.

@JS()
library object_literal_constructor_with_library_annotation_test;

import 'dart:js_interop';

import 'object_literal_constructor_test.dart' show testProperties;

extension type Literal._(JSObject _) implements JSObject {
external Literal({double? a});
external factory Literal.fact({double? a});
}

void main() {
testProperties(Literal());
testProperties(Literal.fact(a: 0.0), a: 0.0);
}

0 comments on commit e9678f7

Please sign in to comment.