Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding an extension type with a typedef representation type makes pkg/kernel/test/dart_scope_calculator_test throw during serialization #55986

Closed
srujzs opened this issue Jun 12, 2024 · 3 comments
Assignees
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. cfe-testing Issues related to testing of the CFE.

Comments

@srujzs
Copy link
Contributor

srujzs commented Jun 12, 2024

The issue was originally found in https://dart-review.googlesource.com/c/sdk/+/370663, with logs here. The simplest repro I can find is this patch:

Patch
diff --git a/sdk/lib/_internal/js_shared/lib/js_types.dart b/sdk/lib/_internal/js_shared/lib/js_types.dart
index cbc9acffcb0..6c01759b2da 100644
--- a/sdk/lib/_internal/js_shared/lib/js_types.dart
+++ b/sdk/lib/_internal/js_shared/lib/js_types.dart
@@ -68,5 +68,7 @@ typedef JSBigIntRepType = interceptors.JavaScriptBigInt;
 // to create a new shared library.
 typedef ExternalDartReferenceRepType = Object;
 
+typedef TD<T> = T;
+
 // JSVoid is just a typedef for void.
 typedef JSVoidRepType = void;
diff --git a/sdk/lib/_internal/wasm/lib/js_types.dart b/sdk/lib/_internal/wasm/lib/js_types.dart
index 1730dbc5213..35ed6fdcb3a 100644
--- a/sdk/lib/_internal/wasm/lib/js_types.dart
+++ b/sdk/lib/_internal/wasm/lib/js_types.dart
@@ -80,6 +80,8 @@ typedef JSBigIntRepType = js.JSValue;
 // to create a new shared library.
 typedef ExternalDartReferenceRepType = js.JSValue;
 
+typedef TD<T> = js.JSValue;
+
 // JSVoid is just a typedef for void. While we could just use JSUndefined, in
 // the future we may be able to use this to elide `return`s in JS trampolines.
 typedef JSVoidRepType = void;
diff --git a/sdk/lib/js_interop/js_interop.dart b/sdk/lib/js_interop/js_interop.dart
index dc572970e72..a0174047f27 100644
--- a/sdk/lib/js_interop/js_interop.dart
+++ b/sdk/lib/js_interop/js_interop.dart
@@ -549,6 +549,8 @@ extension ObjectToExternalDartReference on Object {
   external ExternalDartReference get toExternalReference;
 }
 
+extension type E<T>._(TD<T> _) {}
+
 /// Conversions from [JSPromise] to [Future].
 extension JSPromiseToFuture<T extends JSAny?> on JSPromise<T> {
   /// A [Future] that either completes with the result of the resolved

It essentially adds the extension type to dart:js_interop, and defines the typedef representation type in dart:_js_types. To repro, just patch that onto the SDK and run dart_scope_calculator_test.

Interestingly, if we define typedef TD<T> = T in the dart2wasm patch instead, this doesn't repro.

@srujzs srujzs added area-front-end Use area-front-end for front end / CFE / kernel format related issues. cfe-testing Issues related to testing of the CFE. labels Jun 12, 2024
@srujzs
Copy link
Contributor Author

srujzs commented Jun 25, 2024

@johnniwinther - I want to get the above CL in this week before the 3.5 cutoff, and so I believe this will be blocking. Is there anything I should do here to avoid this?

@jensjoha
Copy link
Contributor

Sorry, I forgot about this one. Looking at it now.

@srujzs
Copy link
Contributor Author

srujzs commented Jun 26, 2024

No worries, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. cfe-testing Issues related to testing of the CFE.
Projects
None yet
Development

No branches or pull requests

2 participants