[dart2js] Clean up --experimental-trust-js-interop-type-annotations
flag.
Change-Id: I8622e88afdd1fc3f65f36814eebc8e124de2bc25
Fixes: https://github.com/dart-lang/sdk/issues/43470
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/164422
Commit-Queue: Mayank Patke <fishythefish@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
diff --git a/pkg/compiler/lib/src/dart2js.dart b/pkg/compiler/lib/src/dart2js.dart
index 5eeb420..98f2f79 100644
--- a/pkg/compiler/lib/src/dart2js.dart
+++ b/pkg/compiler/lib/src/dart2js.dart
@@ -447,7 +447,7 @@
new OptionHandler(Flags.enableNativeNullAssertions, passThrough),
new OptionHandler(Flags.trustTypeAnnotations, setTrustTypeAnnotations),
new OptionHandler(Flags.trustPrimitives, passThrough),
- new OptionHandler(Flags.trustJSInteropTypeAnnotations, passThrough),
+ new OptionHandler(Flags.trustJSInteropTypeAnnotations, ignoreOption),
new OptionHandler(r'--help|/\?|/h', (_) => wantHelp = true),
new OptionHandler('--packages=.+', setPackageConfig),
new OptionHandler(Flags.noSourceMaps, passThrough),
diff --git a/pkg/compiler/lib/src/kernel/element_map_impl.dart b/pkg/compiler/lib/src/kernel/element_map_impl.dart
index a574666..ab22c77 100644
--- a/pkg/compiler/lib/src/kernel/element_map_impl.dart
+++ b/pkg/compiler/lib/src/kernel/element_map_impl.dart
@@ -1939,10 +1939,6 @@
KernelBehaviorBuilder(this.elementEnvironment, this.commonElements,
this.nativeBasicData, this.reporter, this.options);
-
- @override
- bool get trustJSInteropTypeAnnotations =>
- options.trustJSInteropTypeAnnotations;
}
class KernelNativeMemberResolver implements NativeMemberResolver {
diff --git a/pkg/compiler/lib/src/native/behavior.dart b/pkg/compiler/lib/src/native/behavior.dart
index a01fac5..0457090 100644
--- a/pkg/compiler/lib/src/native/behavior.dart
+++ b/pkg/compiler/lib/src/native/behavior.dart
@@ -736,7 +736,6 @@
CommonElements get commonElements;
DiagnosticReporter get reporter;
NativeBasicData get nativeBasicData;
- bool get trustJSInteropTypeAnnotations;
ElementEnvironment get elementEnvironment;
CompilerOptions get options;
DartTypes get dartTypes => commonElements.dartTypes;
@@ -817,21 +816,13 @@
_behavior.typesInstantiated.add(type);
}
- if (!trustJSInteropTypeAnnotations || dartTypes.isTopType(type)) {
- // By saying that only JS-interop types can be created, we prevent
- // pulling in every other native type (e.g. all of dart:html) when a
- // JS interop API returns dynamic or when we don't trust the type
- // annotations. This means that to some degree we still use the return
- // type to decide whether to include native types, even if we don't
- // trust the type annotation.
- ClassEntity cls = commonElements.jsJavaScriptObjectClass;
- _behavior.typesInstantiated.add(elementEnvironment.getThisType(cls));
- } else {
- // Otherwise, when the declared type is a Dart type, we do not
- // register an allocation because we assume it cannot be instantiated
- // from within the JS-interop code. It must have escaped from another
- // API.
- }
+ // By saying that only JS-interop types can be created, we prevent
+ // pulling in every other native type (e.g. all of dart:html) when a
+ // JS interop API returns dynamic. This means that to some degree we
+ // still use the return type to decide whether to include native types,
+ // even though we don't trust the type annotation.
+ ClassEntity cls = commonElements.jsJavaScriptObjectClass;
+ _behavior.typesInstantiated.add(elementEnvironment.getThisType(cls));
}
}
}
@@ -868,9 +859,7 @@
{bool isJsInterop}) {
_behavior = new NativeBehavior();
// TODO(sigmund,sra): consider doing something better for numeric types.
- _addReturnType(!isJsInterop || trustJSInteropTypeAnnotations
- ? type
- : commonElements.dynamicType);
+ _addReturnType(!isJsInterop ? type : commonElements.dynamicType);
_capture(type, isJsInterop);
_overrideWithAnnotations(
createsAnnotations, returnsAnnotations, lookupType);
@@ -898,15 +887,12 @@
// Note: For dart:html and other internal libraries we maintain, we can
// trust the return type and use it to limit what we enqueue. We have to
// be more conservative about JS interop types and assume they can return
- // anything (unless the user provides the experimental flag to trust the
- // type of js-interop APIs). We do restrict the allocation effects and say
- // that interop calls create only interop types (which may be unsound if
- // an interop call returns a DOM type and declares a dynamic return type,
- // but otherwise we would include a lot of code by default).
+ // anything. We do restrict the allocation effects and say that interop
+ // calls create only interop types (which may be unsound if an interop call
+ // returns a DOM type and declares a dynamic return type, but otherwise we
+ // would include a lot of code by default).
// TODO(sigmund,sra): consider doing something better for numeric types.
- _addReturnType(!isJsInterop || trustJSInteropTypeAnnotations
- ? returnType
- : commonElements.dynamicType);
+ _addReturnType(!isJsInterop ? returnType : commonElements.dynamicType);
_capture(type, isJsInterop);
for (DartType type in type.optionalParameterTypes) {
diff --git a/pkg/compiler/lib/src/options.dart b/pkg/compiler/lib/src/options.dart
index f9d0f30..f8c7901 100644
--- a/pkg/compiler/lib/src/options.dart
+++ b/pkg/compiler/lib/src/options.dart
@@ -281,9 +281,6 @@
/// The compiler is run from the build bot.
bool testMode = false;
- /// Whether to trust JS-interop annotations. (experimental)
- bool trustJSInteropTypeAnnotations = false;
-
/// Whether to trust primitive types during inference and optimizations.
bool trustPrimitives = false;
@@ -506,8 +503,6 @@
.._legacyJavaScript = _hasOption(options, Flags.legacyJavaScript)
.._noLegacyJavaScript = _hasOption(options, Flags.noLegacyJavaScript)
..testMode = _hasOption(options, Flags.testMode)
- ..trustJSInteropTypeAnnotations =
- _hasOption(options, Flags.trustJSInteropTypeAnnotations)
..trustPrimitives = _hasOption(options, Flags.trustPrimitives)
..useContentSecurityPolicy =
_hasOption(options, Flags.useContentSecurityPolicy)
diff --git a/pkg/compiler/lib/src/ssa/builder_kernel.dart b/pkg/compiler/lib/src/ssa/builder_kernel.dart
index e093833..b3b3e6f 100644
--- a/pkg/compiler/lib/src/ssa/builder_kernel.dart
+++ b/pkg/compiler/lib/src/ssa/builder_kernel.dart
@@ -4967,11 +4967,6 @@
new js.Template(null, js.objectLiteral(parameterNameMap));
var nativeBehavior = new NativeBehavior()..codeTemplate = codeTemplate;
- if (options.trustJSInteropTypeAnnotations) {
- InterfaceType thisType =
- _elementEnvironment.getThisType(constructor.enclosingClass);
- nativeBehavior.typesReturned.add(thisType);
- }
registry.registerNativeMethod(element);
// TODO(efortuna): Source information.
return new HForeignCode(
@@ -4991,10 +4986,9 @@
? _elementEnvironment.getThisType(element.enclosingClass)
: _elementEnvironment.getFunctionType(element).returnType;
// Native behavior effects here are similar to native/behavior.dart.
- // The return type is dynamic if we don't trust js-interop type
+ // The return type is dynamic because we don't trust js-interop type
// declarations.
- nativeBehavior.typesReturned.add(
- options.trustJSInteropTypeAnnotations ? type : dartTypes.dynamicType());
+ nativeBehavior.typesReturned.add(dartTypes.dynamicType());
// The allocation effects include the declared type if it is native (which
// includes js interop types).
@@ -5003,14 +4997,13 @@
nativeBehavior.typesInstantiated.add(type);
}
- // It also includes any other JS interop type if we don't trust the
- // annotation or if is declared too broad.
- if (!options.trustJSInteropTypeAnnotations ||
- type == _commonElements.objectType ||
- type is DynamicType) {
- nativeBehavior.typesInstantiated.add(_elementEnvironment
- .getThisType(_commonElements.jsJavaScriptObjectClass));
- }
+ // It also includes any other JS interop type. Technically, a JS interop API
+ // could return anything, so the sound thing to do would be to assume that
+ // anything that may come from JS as instantiated. In order to prevent the
+ // resulting code bloat (e.g. from `dart:html`), we unsoundly assume that
+ // only JS interop types are returned.
+ nativeBehavior.typesInstantiated.add(_elementEnvironment
+ .getThisType(_commonElements.jsJavaScriptObjectClass));
AbstractValue instructionType =
_typeInferenceMap.typeFromNativeBehavior(nativeBehavior, closedWorld);
diff --git a/pkg/compiler/test/helpers/compiler_helper.dart b/pkg/compiler/test/helpers/compiler_helper.dart
index 562741f..eb7046a 100644
--- a/pkg/compiler/test/helpers/compiler_helper.dart
+++ b/pkg/compiler/test/helpers/compiler_helper.dart
@@ -37,7 +37,6 @@
bool enableTypeAssertions: false,
bool minify: false,
bool disableInlining: true,
- bool trustJSInteropTypeAnnotations: false,
bool disableTypeInference: true,
bool omitImplicitChecks: true,
bool enableVariance: false,
@@ -58,9 +57,6 @@
if (minify) {
options.add(Flags.minify);
}
- if (trustJSInteropTypeAnnotations) {
- options.add(Flags.trustJSInteropTypeAnnotations);
- }
if (disableInlining) {
options.add(Flags.disableInlining);
}
diff --git a/pkg/compiler/test/jsinterop/interop_anonymous_unreachable_test.dart b/pkg/compiler/test/jsinterop/interop_anonymous_unreachable_test.dart
index ded9c90..2e4e586 100644
--- a/pkg/compiler/test/jsinterop/interop_anonymous_unreachable_test.dart
+++ b/pkg/compiler/test/jsinterop/interop_anonymous_unreachable_test.dart
@@ -80,16 +80,6 @@
Expect.isTrue(generated1.contains("UniqueLongNameForTesting_B"));
Expect.isTrue(generated1.contains("UniqueLongNameForTesting_C"));
Expect.isTrue(generated1.contains("UniqueLongNameForTesting_E"));
-
- print(' - tree-shake when using flag -');
- String generated2 = await compile(program,
- trustJSInteropTypeAnnotations: true, returnAll: true);
- Expect.isTrue(generated2.contains("UniqueLongNameForTesting_A"));
- Expect.isTrue(generated2.contains("UniqueLongNameForTesting_D"));
-
- Expect.isFalse(generated2.contains("UniqueLongNameForTesting_B"));
- Expect.isFalse(generated2.contains("UniqueLongNameForTesting_C"));
- Expect.isFalse(generated2.contains("UniqueLongNameForTesting_E"));
}
testTreeShakingNativeTypes() async {
@@ -122,14 +112,6 @@
// but we exclude other native types like HTMLAudioElement
Expect.isFalse(generated1.contains("HTMLAudioElement"));
- print(' - allocation effect of dynamic excludes native types [flag] -');
- // Trusting types doesn't make a difference.
- String generated2 = await compile(program,
- trustJSInteropTypeAnnotations: true, returnAll: true);
- Expect.isTrue(generated2.contains("UniqueLongNameForTesting_A"));
- Expect.isTrue(generated2.contains("UniqueLongNameForTesting_B"));
- Expect.isFalse(generated2.contains("HTMLAudioElement"));
-
print(' - declared native types are included in allocation effect -');
String program2 = """
import 'dart:html';
diff --git a/tests/lib/html/js_typed_interop_anonymous2_exp_test.dart b/tests/lib/html/js_typed_interop_anonymous2_exp_test.dart
deleted file mode 100644
index 2c827b8..0000000
--- a/tests/lib/html/js_typed_interop_anonymous2_exp_test.dart
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright (c) 2015, 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.
-
-// SharedOptions=--experimental-trust-js-interop-type-annotations
-
-// Same test as js_typed_interop_anonymous2, but using the
-// --experimental-trust-js-interop-type-annotations flag.
-@JS()
-library js_typed_interop_anonymous2_exp_test;
-
-import 'dart:html';
-import 'dart:js' as js;
-
-import 'package:js/js.dart';
-import 'package:expect/minitest.dart';
-
-@JS()
-@anonymous
-class A {
- external factory A({B? b});
-
- external B? get b;
-}
-
-@JS()
-@anonymous
-class B {
- external factory B({C? c});
-
- external C? get c;
-}
-
-@JS()
-@anonymous
-class C {
- external factory C();
-}
-
-// D is unreachable, and that is OK
-@JS()
-@anonymous
-class D {
- external factory D();
-}
-
-main() {
- test('simple', () {
- var b = new B();
- var a = new A(b: b);
- expect(a.b, equals(b));
- expect(b.c, isNull);
- });
-}
diff --git a/tests/lib/html/js_typed_interop_anonymous_exp_test.dart b/tests/lib/html/js_typed_interop_anonymous_exp_test.dart
deleted file mode 100644
index e160bdd..0000000
--- a/tests/lib/html/js_typed_interop_anonymous_exp_test.dart
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright (c) 2015, 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.
-
-// SharedOptions=--experimental-trust-js-interop-type-annotations
-
-// Same test as js_typed_interop_anonymous, but using the
-// --experimental-trust-js-interop-type-annotations flag.
-@JS()
-library js_typed_interop_anonymous_exp_test;
-
-import 'dart:html';
-import 'dart:js' as js;
-
-import 'package:js/js.dart';
-import 'package:expect/minitest.dart';
-
-@JS()
-@anonymous
-class Literal {
- external factory Literal({int? x, String? y, num? z});
-
- external int? get x;
- external String? get y;
- external num? get z;
-}
-
-main() {
- test('simple', () {
- var l = new Literal(x: 3, y: "foo");
- expect(l.x, equals(3));
- expect(l.y, equals("foo"));
- expect(l.z, isNull);
- });
-}
diff --git a/tests/lib/html/js_typed_interop_anonymous_unreachable_exp_test.dart b/tests/lib/html/js_typed_interop_anonymous_unreachable_exp_test.dart
deleted file mode 100644
index 046dcff..0000000
--- a/tests/lib/html/js_typed_interop_anonymous_unreachable_exp_test.dart
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright (c) 2015, 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.
-
-// SharedOptions=--experimental-trust-js-interop-type-annotations
-
-// Same test as js_typed_interop_anonymous_unreachable, but using the
-// --experimental-trust-js-interop-type-annotations flag.
-@JS()
-library js_typed_interop_anonymous_unreachable_exp_test;
-
-import 'dart:html';
-import 'dart:js' as js;
-
-import 'package:js/js.dart';
-import 'package:expect/minitest.dart';
-
-@JS()
-@anonymous
-class Literal {
- external factory Literal({required int x, required String y, required num z});
-
- external int get x;
- external String get y;
- external num get z;
-}
-
-main() {
- test('nothing to do', () {
- // This test is empty, but it is a regression for Issue# 24974: dart2js
- // would crash trying to compile code that used @anonymous and that was
- // not reachable from main.
- });
-}
diff --git a/tests/lib/html/js_typed_interop_side_cast_exp_test.dart b/tests/lib/html/js_typed_interop_side_cast_exp_test.dart
deleted file mode 100644
index be112c8..0000000
--- a/tests/lib/html/js_typed_interop_side_cast_exp_test.dart
+++ /dev/null
@@ -1,51 +0,0 @@
-// Copyright (c) 2015, 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.
-
-// SharedOptions=--experimental-trust-js-interop-type-annotations
-
-// Similar test to js_typed_interop_side_cast, but because we are using the
-// --experimental-trust-js-interop-type-annotations flag, we test a slightly
-// different behavior.
-@JS()
-library js_typed_interop_side_cast_exp_test;
-
-import 'package:js/js.dart';
-import 'package:expect/minitest.dart';
-
-@JS()
-@anonymous
-class A {
- external int get x;
- external factory A({required int x});
-}
-
-@JS()
-@anonymous
-class B {
- external int get x;
- external factory B({required int x});
-}
-
-@JS()
-@anonymous
-class C {
- external int get x;
- external factory C({required int x});
-}
-
-main() {
- test('side-casts work for reachable types', () {
- new C(x: 3); // make C reachable
- dynamic a = new A(x: 3);
- expect(a is C, isTrue);
- C c = a;
- expect(c.x, equals(3));
- });
-
- // Note: this test would fail without the experimental flag.
- test('side-casts do not work for unreachable types', () {
- dynamic a = new A(x: 3);
- expect(a is B, isFalse); //# 01: ok
- });
-}
diff --git a/tests/lib_2/html/js_typed_interop_anonymous2_exp_test.dart b/tests/lib_2/html/js_typed_interop_anonymous2_exp_test.dart
deleted file mode 100644
index ba4afc2..0000000
--- a/tests/lib_2/html/js_typed_interop_anonymous2_exp_test.dart
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright (c) 2015, 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.
-//
-// SharedOptions=--experimental-trust-js-interop-type-annotations
-
-// Same test as js_typed_interop_anonymous2, but using the
-// --experimental-trust-js-interop-type-annotations flag.
-@JS()
-library js_typed_interop_anonymous2_exp_test;
-
-import 'dart:html';
-import 'dart:js' as js;
-
-import 'package:js/js.dart';
-import 'package:expect/minitest.dart';
-
-@JS()
-@anonymous
-class A {
- external factory A({B b});
-
- external B get b;
-}
-
-@JS()
-@anonymous
-class B {
- external factory B({C c});
-
- external C get c;
-}
-
-@JS()
-@anonymous
-class C {
- external factory C();
-}
-
-// D is unreachable, and that is OK
-@JS()
-@anonymous
-class D {
- external factory D();
-}
-
-main() {
- test('simple', () {
- var b = new B();
- var a = new A(b: b);
- expect(a.b, equals(b));
- expect(b.c, isNull);
- });
-}
diff --git a/tests/lib_2/html/js_typed_interop_anonymous_exp_test.dart b/tests/lib_2/html/js_typed_interop_anonymous_exp_test.dart
deleted file mode 100644
index 362a41c..0000000
--- a/tests/lib_2/html/js_typed_interop_anonymous_exp_test.dart
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright (c) 2015, 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.
-//
-// SharedOptions=--experimental-trust-js-interop-type-annotations
-
-// Same test as js_typed_interop_anonymous, but using the
-// --experimental-trust-js-interop-type-annotations flag.
-@JS()
-library js_typed_interop_anonymous_exp_test;
-
-import 'dart:html';
-import 'dart:js' as js;
-
-import 'package:js/js.dart';
-import 'package:expect/minitest.dart';
-
-@JS()
-@anonymous
-class Literal {
- external factory Literal({int x, String y, num z});
-
- external int get x;
- external String get y;
- external num get z;
-}
-
-main() {
- test('simple', () {
- var l = new Literal(x: 3, y: "foo");
- expect(l.x, equals(3));
- expect(l.y, equals("foo"));
- expect(l.z, isNull);
- });
-}
diff --git a/tests/lib_2/html/js_typed_interop_anonymous_unreachable_exp_test.dart b/tests/lib_2/html/js_typed_interop_anonymous_unreachable_exp_test.dart
deleted file mode 100644
index 95c6141..0000000
--- a/tests/lib_2/html/js_typed_interop_anonymous_unreachable_exp_test.dart
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright (c) 2015, 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.
-//
-// SharedOptions=--experimental-trust-js-interop-type-annotations
-
-// Same test as js_typed_interop_anonymous_unreachable, but using the
-// --experimental-trust-js-interop-type-annotations flag.
-@JS()
-library js_typed_interop_anonymous_unreachable_exp_test;
-
-import 'dart:html';
-import 'dart:js' as js;
-
-import 'package:js/js.dart';
-import 'package:expect/minitest.dart';
-
-@JS()
-@anonymous
-class Literal {
- external factory Literal({int x, String y, num z});
-
- external int get x;
- external String get y;
- external num get z;
-}
-
-main() {
- test('nothing to do', () {
- // This test is empty, but it is a regression for Issue# 24974: dart2js
- // would crash trying to compile code that used @anonymous and that was
- // not reachable from main.
- });
-}
diff --git a/tests/lib_2/html/js_typed_interop_side_cast_exp_test.dart b/tests/lib_2/html/js_typed_interop_side_cast_exp_test.dart
deleted file mode 100644
index a5061fb..0000000
--- a/tests/lib_2/html/js_typed_interop_side_cast_exp_test.dart
+++ /dev/null
@@ -1,51 +0,0 @@
-// Copyright (c) 2015, 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.
-//
-// SharedOptions=--experimental-trust-js-interop-type-annotations
-
-// Similar test to js_typed_interop_side_cast, but because we are using the
-// --experimental-trust-js-interop-type-annotations flag, we test a slightly
-// different behavior.
-@JS()
-library js_typed_interop_side_cast_exp_test;
-
-import 'package:js/js.dart';
-import 'package:expect/minitest.dart';
-
-@JS()
-@anonymous
-class A {
- external int get x;
- external factory A({int x});
-}
-
-@JS()
-@anonymous
-class B {
- external int get x;
- external factory B({int x});
-}
-
-@JS()
-@anonymous
-class C {
- external int get x;
- external factory C({int x});
-}
-
-main() {
- test('side-casts work for reachable types', () {
- new C(x: 3); // make C reachable
- dynamic a = new A(x: 3);
- expect(a is C, isTrue);
- C c = a;
- expect(c.x, equals(3));
- });
-
- // Note: this test would fail without the experimental flag.
- test('side-casts do not work for unreachable types', () {
- dynamic a = new A(x: 3);
- expect(a is B, isFalse); //# 01: ok
- });
-}
