dart-lang / sdk

The Dart SDK, including the VM, dart2js, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
9.94k stars 1.53k forks source link

Erasure of extension types in DartObject is blocking adoption of dart:js_interop #56118

Open leonsenft opened 6 days ago

leonsenft commented 6 days ago

Issue

When computing a constant value, the analyzer performs extension type erasure on the resulting DartObject and TypeState.

https://github.com/dart-lang/sdk/blob/9f27bdeaf66ac8d84676a074ab0fc026564c3732/pkg/analyzer/lib/src/dart/constant/value.dart#L190

https://github.com/dart-lang/sdk/blob/9f27bdeaf66ac8d84676a074ab0fc026564c3732/pkg/analyzer/lib/src/dart/constant/value.dart#L3123

This is problematic for code generators when the analyzed type is a extension type on a dart:js_interop type.

Example

In ACX, we support annotating constructor parameters with a constant token instance used as a key for dependency injection:

const exampleToken = OpaqueToken<Example>();

class ExampleComponent {
  ExampleComponent(@exampleToken this.example);

  final Example example;
}

Since Dart's type system lacks any way for us to associate the token's type with the type of the parameter it annotates, we perform a check inside our code generator that the token's type argument matches the parameter's type:

/// Validates the type of the OpaqueToken, if present, is compatible with the
/// [parameter]'s type.
void validateOpaqueToken(ParameterElement parameter) {
  final opaqueTokenAnnotation = $OpaqueToken.firstAnnotationOf(parameter);
  if (opaqueTokenAnnotation == null) {
    return;
  }

  final opaqueToken = opaqueTokenAnnotation.type as InterfaceType?;
  final typeSystem = parameter.library!.typeSystem;
  final opaqueArgumentType = opaqueType.typeArguments.first;
  final parameterType = parameter.type;
  if (!typeSystem.isSubtypeOf(opaqueArgumentType, parameterType)) {
    final opaqueDisplayString = opaqueArgumentType.getDisplayString();
    final paramDisplayString = parameterType.getDisplayString();
    CompileContext.current.reportAndRecover(BuildError.forElement(
        parameter,
        'OpaqueTokens must have an explicit type argument which is '
        'assignable to the type of the parameter they annotate, but '
        '`$opaqueDisplayString` is not assignable to '
        '`$paramDisplayString`'));
  }
}

This check breaks down if Example happens to be an extension type on a JS type.

extension type Example._(JSObject _) implements JSFunction {}

Now, because of the aforementioned type erasure, the code generator sees exampleToken's type as OpaqueToken<JavaScriptFunction>, instead of OpaqueToken<Example>. However, the type of this.example–accessed through the element model–is still Example. This causes the isSubtypeOf() check to fail, since JavaScriptFunction is indeed not assignable to the narrower Example type.

One might suggest performing type erasure on the parameter's type as well:

  final parameterType = parameter.type.extensionTypeErasure;

This makes the check pass, however, we've now introduced a bug since we'd accept any extension type on JSFunction, not just Example.

Furthermore, the underlying type JavaScriptFunction is a private type from dart:_js_types, which we can't actually use in our generated code, so at the end of the day we still need to be able to get back to the user authored extension type. This problem isn't specific to JS interop types, as users could use extension types over private types in plain Dart code as well.

Solution

I'm not proposing any specific solution, however, some way to access an erased type's original type (extension) would be useful for code generators.

@mattrberry @srujzs @scheglov

dart-github-bot commented 6 days ago

Summary: Extension type erasure in DartObject prevents code generators from accurately validating types when working with extension types on dart:js_interop types. This leads to incorrect type checks and potential bugs in generated code.

leonsenft commented 6 days ago

Summary: Extension type erasure in DartObject prevents code generators from accurately validating types when working with extension types on dart:js_interop types. This leads to incorrect type checks and potential bugs in generated code.

Not so much potential bugs as it doesn't compile.

Error: Can't access platform private library.
import 'dart:_js_types' as import30;
       ^
Error: Compilation failed.

image

mattrberry commented 6 days ago

Related to https://github.com/dart-lang/sdk/pull/55897, which I'll close in favor of this bug. This is also a blocker for adopting extension types in our ongoing improvements to the Dart protobuf runtime

bwilkerson commented 6 days ago

As noted in elsewhere the problem arises because the element/type model is describing the static types in the code, while DartObject is describing the runtime-type of objects. The runtime-type of an object will never be an extension type.

We could potentially provide some notion of the "static type" of a runtime object, and it might even make sense in the limited world of compile time constants. I can't really think of any better option.

@scheglov

lrhn commented 6 days ago

Seems like annotations are not just values, but also expressions, and people want to inspect on the expression as well. This may become even more relevant with macros, where we want to inspect on annotations too.

mraleph commented 5 days ago

Note that erasure also implies that const OpaqueToken<A>() and const OpaqueToken<B>() where A and B are extension types on C will also collapse into a single const OpaqueToken<C>() - and consequently break injection if you try to provide both A and B.

This means OpaqueToken and type literals are not very suitable in their current form for injection purposes when extension types are involved, because today language gives you no ways to see them before erasure eliminates them.

leonsenft commented 5 days ago

Note that erasure also implies that const OpaqueToken<A>() and const OpaqueToken<B>() where A and B are extension types on C will also collapse into a single const OpaqueToken<C>() - and consequently break injection if you try to provide both A and B.

This means OpaqueToken and type literals are not very suitable in their current form for injection purposes when extension types are involved, because today language gives you no ways to see them before erasure eliminates them.

@mattrberry This is potentially concerning for us. Perhaps if an OpaqueToken is on a JS* type, the compiler should require a unique identifier to differentiate them?

// BAD
const signalToken = OpaqueToken<WritableSignal<int>>();
const computedToken = OpaqueToken<Signal<int>>();

// OKAY
const signalToken = OpaqueToken<WritableSignal<int>>('signal');
const computedToken = OpaqueToken<Signal<int>>('computed');

(For clarification for WritableSignal and Signal are extension types over JSFunction)

mattrberry commented 5 days ago

I think we should probably require an identifier for OpaqueTokens of any extension type, not just on JS* types. We may want to revisit doing the same for type aliases as well. I think I see that as a blocker, though