diff --git a/CHANGELOG.md b/CHANGELOG.md index aee53d35937..b1dcdb89c11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -264,6 +264,16 @@ Updated the Linter to `1.8.0`, which includes changes that [#46545]: https://github.com/dart-lang/sdk/issues/46545 [1]: https://dart.dev/faq#q-what-browsers-do-you-support-as-javascript-compilation-targets +#### Dart Dev Compiler (DDC) + +- **Breaking Change** [#44154][]: Subtyping relations of `package:js` classes + have been changed to be more correct and consistent with Dart2JS. + Like `anonymous` classes, non-`anonymous` classes will no longer check the + underlying type in DDC. The internal type representation of these objects have + changed as well, which will affect the `toString` value of these types. + +[#44154]: https://github.com/dart-lang/sdk/issues/44154 + ## 2.13.4 - 2021-06-28 This is a patch release that fixes: diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart index c5994740bcd..e777ddf537e 100644 --- a/pkg/dev_compiler/lib/src/kernel/compiler.dart +++ b/pkg/dev_compiler/lib/src/kernel/compiler.dart @@ -1677,7 +1677,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor ctorFields = ctor.initializers .map((c) => c is FieldInitializer ? c.field : null) .toSet() - ..remove(null); + ..remove(null); } var body = []; @@ -2884,27 +2884,15 @@ class ProgramCompiler extends ComputeOnceConstantVisitor js_ast.Expression typeRep; // Type parameters don't matter as JS interop types cannot be reified. - // We have to use lazy JS types because until we have proper module - // loading for JS libraries bundled with Dart libraries, we will sometimes - // need to load Dart libraries before the corresponding JS libraries are - // actually loaded. - // Given a JS type such as: - // @JS('google.maps.Location') - // class Location { ... } - // We can't emit a reference to MyType because the JS library that defines - // it may be loaded after our code. So for now, we use a special lazy type - // object to represent MyType. - // Anonymous JS types do not have a corresponding concrete JS type so we - // have to use a helper to define them. - if (isJSAnonymousType(c)) { - typeRep = runtimeCall( - 'anonymousJSType(#)', [js.escapedString(getLocalClassName(c))]); - } else { - var jsName = _emitJsNameWithoutGlobal(c); - if (jsName != null) { - typeRep = runtimeCall('lazyJSType(() => #, #)', - [_emitJSInteropForGlobal(jsName), js.escapedString(jsName)]); - } + // package:js types fall under either named or anonymous types. Named types + // are used to correspond to JS types that exist, but we do not use the + // underlying type for type checks, so they operate virtually the same as + // anonymous types. We represent package:js types with a corresponding type + // object. + var jsName = isJSAnonymousType(c) ? + getLocalClassName(c) : _emitJsNameWithoutGlobal(c); + if (jsName != null) { + typeRep = runtimeCall('packageJSType(#)', [js.escapedString(jsName)]); } if (typeRep != null) { diff --git a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/runtime.dart b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/runtime.dart index 2cd212ab3ca..6ff9de76fb5 100644 --- a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/runtime.dart +++ b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/runtime.dart @@ -181,8 +181,8 @@ final List _cacheMaps = JS('!', '[]'); /// A list of functions to reset static fields back to their uninitialized /// state. /// -/// This is populated by [defineLazyField] and [LazyJSType] and only contains -/// fields that have been initialized. +/// This is populated by [defineLazyField] and only contains fields that have +/// been initialized. @notNull final List _resetFields = JS('', '[]'); diff --git a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart index e48e530d6f5..3c0267db054 100644 --- a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart +++ b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart @@ -81,6 +81,15 @@ void nativeNonNullAsserts(bool enable) { final metadata = JS('', 'Symbol("metadata")'); +/// A javascript Symbol used to store a canonical version of T? on T. +final _cachedNullable = JS('', 'Symbol("cachedNullable")'); + +/// A javascript Symbol used to store a canonical version of T* on T. +final _cachedLegacy = JS('', 'Symbol("cachedLegacy")'); + +/// A javascript Symbol used to store prior subtype checks and their results. +final _subtypeCache = JS('', 'Symbol("_subtypeCache")'); + /// Types in dart are represented internally at runtime as follows. /// /// - Normal nominal types, produced from classes, are represented @@ -197,67 +206,17 @@ F tearoffInterop(F f) { return JS('', '#', ret); } -/// The Dart type that represents a JavaScript class(/constructor) type. -/// -/// The JavaScript type may not exist, either because it's not loaded yet, or -/// because it's not available (such as with mocks). To handle this gracefully, -/// we disable type checks for in these cases, and allow any JS object to work -/// as if it were an instance of this JS type. -class LazyJSType extends DartType { - Function() _getRawJSTypeFn; - @notNull - final String _dartName; - Object? _rawJSType; - - LazyJSType(this._getRawJSTypeFn, this._dartName); - - toString() { - var raw = _getRawJSType(); - return raw != null ? typeName(raw) : "JSObject<$_dartName>"; - } - - Object? _getRawJSType() { - var raw = _rawJSType; - if (raw != null) return raw; - - // Try to evaluate the JS type. If this fails for any reason, we'll try - // again next time. - // TODO(jmesserly): is it worth trying again? It may create unnecessary - // overhead, especially if exceptions are being thrown. Also it means the - // behavior of a given type check can change later on. - try { - raw = _getRawJSTypeFn(); - } catch (e) {} - - if (raw == null) { - _warn('Cannot find native JavaScript type ($_dartName) for type check'); - } else { - _rawJSType = raw; - JS('', '#.push(() => # = null)', _resetFields, _rawJSType); - } - return raw; - } - - Object rawJSTypeForCheck() => _getRawJSType() ?? typeRep(); - - @notNull - @JSExportName('is') - bool is_T(obj) => - obj != null && - (_isJsObject(obj) || isSubtypeOf(getReifiedType(obj), this)); - - @JSExportName('as') - as_T(obj) => is_T(obj) ? obj : castError(obj, this); -} - -/// An anonymous JS type +/// Dart type that represents a package:js class type (either anonymous or not). /// /// For the purposes of subtype checks, these match any JS type. -class AnonymousJSType extends DartType { +class PackageJSType extends DartType { final String _dartName; - AnonymousJSType(this._dartName); - toString() => _dartName; + PackageJSType(this._dartName); + @override + String toString() => _dartName; + + @notNull @JSExportName('is') bool is_T(obj) => obj != null && @@ -299,35 +258,25 @@ void _nullWarnOnType(type) { } } -var _lazyJSTypes = JS('', 'new Map()'); -var _anonymousJSTypes = JS('', 'new Map()'); +var _packageJSTypes = JS('', 'new Map()'); -lazyJSType(Function() getJSTypeCallback, String name) { - var ret = JS('', '#.get(#)', _lazyJSTypes, name); +packageJSType(String name) { + var ret = JS('', '#.get(#)', _packageJSTypes, name); if (ret == null) { - ret = LazyJSType(getJSTypeCallback, name); - JS('', '#.set(#, #)', _lazyJSTypes, name, ret); + ret = PackageJSType(name); + JS('', '#.set(#, #)', _packageJSTypes, name, ret); } return ret; } -anonymousJSType(String name) { - var ret = JS('', '#.get(#)', _anonymousJSTypes, name); - if (ret == null) { - ret = AnonymousJSType(name); - JS('', '#.set(#, #)', _anonymousJSTypes, name, ret); - } - return ret; -} - -/// A javascript Symbol used to store a canonical version of T? on T. -final _cachedNullable = JS('', 'Symbol("cachedNullable")'); - -/// A javascript Symbol used to store a canonical version of T* on T. -final _cachedLegacy = JS('', 'Symbol("cachedLegacy")'); - -/// A javascript Symbol used to store prior subtype checks and their results. -final _subtypeCache = JS('', 'Symbol("_subtypeCache")'); +/// Since package:js types are all subtypes of each other, we use this var to +/// denote *some* package:js type in our subtyping logic. +/// +/// Used only when a concrete PackageJSType is not available i.e. when neither +/// the object nor the target type is a PackageJSType. Avoids initializating a +/// new PackageJSType every time. Note that we don't add it to the set of JS +/// types, since it's not an actual JS class. +final _pkgJSTypeForSubtyping = PackageJSType(''); /// Returns a nullable (question, ?) version of [type]. /// @@ -1522,34 +1471,20 @@ bool _isSubtype(t1, t2, @notNull bool strictMode) => JS('!', '''(() => { // // JavaScriptObject <: package:js types // package:js types <: JavaScriptObject - // - // TODO: This doesn't allow package:js type <: another package:js type yet. if (${_isInterfaceSubtype(t1, JavaScriptObject, strictMode)} - && (${_jsInstanceOf(t2, LazyJSType)} - || ${_jsInstanceOf(t2, AnonymousJSType)} - // TODO: Since package:js types are instances of LazyJSType and - // AnonymousJSType, we don't have a mechanism to determine if *some* - // package:js type implements t2. This will possibly require keeping - // a map of these relationships for this subtyping check. For now, - // don't execute the following checks. - // || _isInterfaceSubtype(LazyJSType, t2, strictMode) - // || _isInterfaceSubtype(AnonymousJSType, t2, strictMode) - )) { + && + // TODO: Since package:js types are instances of PackageJSType and + // we don't have a mechanism to determine if *some* package:js type + // implements t2. This will possibly require keeping a map of these + // relationships for this subtyping check. For now, this will only + // work if t2 is also a PackageJSType. + ${_isInterfaceSubtype(_pkgJSTypeForSubtyping, t2, strictMode)}) { return true; } if (${_isInterfaceSubtype(JavaScriptObject, t2, strictMode)} - && (${_jsInstanceOf(t1, LazyJSType)} - || ${_jsInstanceOf(t1, AnonymousJSType)} - // TODO: We don't have a check in `isInterfaceSubtype` to check that - // a class is a subtype of *some* package:js class. This will likely - // require modifying `_isInterfaceSubtype` to check if the - // interfaces of t1 include a package:js type. For now, don't - // execute the following checks. - // || _isInterfaceSubtype(t1, LazyJSType, strictMode) - // || _isInterfaceSubtype(t1, AnonymousJSType, strictMode) - )) { + && ${_isInterfaceSubtype(t1, _pkgJSTypeForSubtyping, strictMode)}) { return true; } @@ -1617,10 +1552,11 @@ bool _isSubtype(t1, t2, @notNull bool strictMode) => JS('!', '''(() => { })()'''); bool _isInterfaceSubtype(t1, t2, @notNull bool strictMode) => JS('', '''(() => { - // If we have lazy JS types, unwrap them. This will effectively - // reduce to a prototype check below. - if (${_jsInstanceOf(t1, LazyJSType)}) $t1 = $t1.rawJSTypeForCheck(); - if (${_jsInstanceOf(t2, LazyJSType)}) $t2 = $t2.rawJSTypeForCheck(); + // Instances of PackageJSType are all subtypes of each other. + if (${_jsInstanceOf(t1, PackageJSType)} + && ${_jsInstanceOf(t2, PackageJSType)}) { + return true; + } if ($t1 === $t2) { return true; diff --git a/tests/dartdevc/debugger/debugger_test_golden.txt b/tests/dartdevc/debugger/debugger_test_golden.txt index 76772beaeee..a879c6e7db2 100644 --- a/tests/dartdevc/debugger/debugger_test_golden.txt +++ b/tests/dartdevc/debugger/debugger_test_golden.txt @@ -6519,7 +6519,7 @@ Value: { "style": "background-color: #d9edf7;color: black" }, - "Instance of 'TestGenericClass, int>'" + "Instance of 'TestGenericClass'" ] ----------------------------------- Test: TestGenericClassJSInterop instance body @@ -6616,7 +6616,7 @@ Value: { "style": "background-color: #d9edf7;color: black" }, - "TestGenericClass, int>" + "TestGenericClass" ] ----------------------------------- Test: TestGenericClassJSInterop definition formatting body diff --git a/tests/dartdevc_2/debugger/debugger_test_golden.txt b/tests/dartdevc_2/debugger/debugger_test_golden.txt index 76772beaeee..a879c6e7db2 100644 --- a/tests/dartdevc_2/debugger/debugger_test_golden.txt +++ b/tests/dartdevc_2/debugger/debugger_test_golden.txt @@ -6519,7 +6519,7 @@ Value: { "style": "background-color: #d9edf7;color: black" }, - "Instance of 'TestGenericClass, int>'" + "Instance of 'TestGenericClass'" ] ----------------------------------- Test: TestGenericClassJSInterop instance body @@ -6616,7 +6616,7 @@ Value: { "style": "background-color: #d9edf7;color: black" }, - "TestGenericClass, int>" + "TestGenericClass" ] ----------------------------------- Test: TestGenericClassJSInterop definition formatting body diff --git a/tests/lib/html/js_typed_interop_lazy_test.dart b/tests/lib/html/js_typed_interop_lazy_test.dart index 63c578d0e9c..1825174c592 100644 --- a/tests/lib/html/js_typed_interop_lazy_test.dart +++ b/tests/lib/html/js_typed_interop_lazy_test.dart @@ -142,10 +142,9 @@ baz.LazyClass = function LazyClass(a) { expect(new Object() is! AnonClass2, isTrue); expect([] is List, isTrue); - // TODO(jacobr): why doesn't this test pass? - // expect([] is List, isTrue); + expect([] is List, isTrue); expect([] is! List, isTrue); - expect([] is! List, isTrue); //# 01: ok + expect([] is List, isTrue); expect([] is! List, isTrue); expect([] is List, isTrue); @@ -234,10 +233,9 @@ baz.foo.NestedLazyClass = function NestedLazyClass(a) { expect(new Object() is! AnonClass2, isTrue); expect([] is List, isTrue); - // TODO(jacobr): why doesn't this test pass? - // expect([] is List, isTrue); + expect([] is List, isTrue); expect([] is! List, isTrue); - expect([] is! List, isTrue); //# 01: ok + expect([] is List, isTrue); expect([] is! List, isTrue); expect([] is List, isTrue); diff --git a/tests/lib_2/html/js_typed_interop_lazy_test.dart b/tests/lib_2/html/js_typed_interop_lazy_test.dart index a7b49a97766..918193db8b5 100644 --- a/tests/lib_2/html/js_typed_interop_lazy_test.dart +++ b/tests/lib_2/html/js_typed_interop_lazy_test.dart @@ -144,10 +144,9 @@ baz.LazyClass = function LazyClass(a) { expect(new Object() is! AnonClass2, isTrue); expect([] is List, isTrue); - // TODO(jacobr): why doesn't this test pass? - // expect([] is List, isTrue); + expect([] is List, isTrue); expect([] is! List, isTrue); - expect([] is! List, isTrue); //# 01: ok + expect([] is List, isTrue); expect([] is! List, isTrue); expect([] is List, isTrue); @@ -236,10 +235,9 @@ baz.foo.NestedLazyClass = function NestedLazyClass(a) { expect(new Object() is! AnonClass2, isTrue); expect([] is List, isTrue); - // TODO(jacobr): why doesn't this test pass? - // expect([] is List, isTrue); + expect([] is List, isTrue); expect([] is! List, isTrue); - expect([] is! List, isTrue); //# 01: ok + expect([] is List, isTrue); expect([] is! List, isTrue); expect([] is List, isTrue);