"RuntimeError: illegal cast" when casting value to a type parameter whose bound is a nullable extension type

[srujzs]

Dart SDK version: 3.3.0-edge.8f23c4a91687f3eb59746f00fe75ca5f162bd0d3 (be) (Thu Nov 2 16:28:57 2023 -0700) on "macos_x64"
Browser: Both d8 and Chrome

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

extension type A(int a) {}

T returnA<T extends A?>() {
  return null as T;
}

void main() {
  returnA();
}

results in:

stdout:
wasm-function[593]:0x1d0e0: RuntimeError: illegal cast
RuntimeError: illegal cast
    at returnA (wasm://wasm/0008d21e:wasm-function[593]:0x1d0e0)
    at main (wasm://wasm/0008d21e:wasm-function[62]:0xfb3a)
    at main tear-off trampoline (wasm://wasm/0008d21e:wasm-function[64]:0xfb4b)
    at _invokeMain (wasm://wasm/0008d21e:wasm-function[67]:0xfd28)
    at Module.invoke (/Users/srujzs/dart-sdk/sdk/xcodebuild/ReleaseX64/generated_compilations/custom-configuration-1/tests_lib_js_static_interop_test_dummy_test/dummy_test.mjs:177:28)
    at main (pkg/dart2wasm/bin/run_wasm.js:351:21)
    at async action (pkg/dart2wasm/bin/run_wasm.js:296:37)
    at async eventLoop (pkg/dart2wasm/bin/run_wasm.js:279:9)

This seems to result specifically from a combination of the nullable extension type type parameter bound and a value of null. Doing any of the following does not reproduce this:

• casting 0 as T instead of null
• making the representation type of A nullable
• casting null as A instead of T

My best guess is that we're losing the nullability of the extension type somewhere.

This popped up when I was migrating JS types to extension types, specifically here (when the property get result is null):

sdk/sdk/lib/_internal/wasm/lib/js_interop_unsafe_patch.dart

Line 22 in 18c8a50

@patch

.

Neither DDC or dart2js reproduce this issue.

[osa1]

Fix in https://dart-review.googlesource.com/c/sdk/+/334460.

[eernstg]

Hi, just to make sure you aren't working on the semantics of extension types based on a rule which is being updated, I'd like to mention a thing:

In dart-lang/language#3434, the definition of 'being nullable' for extension types is changed: It used to be based on the nullability of the representation type. With the change proposed in that PR, it is based on explicit superinterface declarations (implements ... Object ... or implements ... T ... where T is a type, extension type or non-extension type, that has a path to Object in the superinterface graph).

So an extension type E is non-nullable if and only if there is a path in the superinterface graph to Object, and we go back to a situation where the superinterface graph is determined by the declaration (that is, E<T1> and E<T2> will never have structurally different superinterface graphs, they will just have different type arguments in some nodes).

The change has been accepted by the language team, so the essence of it is in place today. So dart-lang/language#3434 hasn't landed yet, but we are just debating the choice of words, not the meaning.

I'm sorry about introducing changes that might be substantial at this time, but I think we will be happy about the new rule because it's considerably simpler (and more in line with the rest of the language).

[Edit: Corrected 'nullable' to 'non-nullable' in paragraph 3.]

[osa1]

Thanks @eernstg. I haven't even looked at the issue here. I realized that the failure looks similar to failures in #53963 so I tried my fix on the repro here and realized it's also fixed.

[srujzs]

@eernstg Just so I'm understanding this clearly:

So an extension type E is nullable if and only if there is a path in the superinterface graph to Object

I'm guessing this is supposed to be "non-nullable"? In which case E? is a static error as it cannot exist? It's an interesting addition but I don't think it modifies the above example as A is implicitly a subtype of Object? iiuc.

cc @johnniwinther because this affects the type erasure algorithm in the AST

[lrhn]

The extension type rules should have no (direct) effect on the runtime behavior, since extension types should have been effectively erased at runtime.

Whether the extension type is itself nullable or not shouldn't change what, effectively, null as T with <T extends int?> does.
(But it can obviously change where a bug can happen on the way).

[eernstg]

I'm guessing this is supposed to be "non-nullable"?

Right, thanks! I corrected this in my comment above.

In which case E? is a static error as it cannot exist?

We have no rules that makes E? a static error: If E is an extension type whose superinterface graph has a path to Object then E is non-nullable and E? is a type that admits all values of type E as well as null. Nothing special about that. When E is an extension type that does not have such a path then E is a potentially nullable type, and E? can be used just like we can use X? when X is a type variable which is potentially nullable.

So I think it all makes sense, and E? never needs to be a static error. What would be the problem in allowing it?

I'd say that the example in the original post should be accepted at compile time, and it should succeed at run time. The actual type argument passed to the invocation of returnA would be A?, because there are no constraints at the call site, and the value of T would then be int? at run time when we evaluate null as T, and that should not throw.

[lrhn]

Extension types use precisely the same rules for being nullable or being non-nullerne as every other type:
A type is nullable if Null is a subtype, and it's non-nullable if Object is a supertype.
The change Erik mentioned is that no extension types will implement Object implicitly any longer, and have to do so explicitly, or implement a subtype of Object, if they want to be non-nullable.
(An extension type, E, cannot itself be nullable, same as every class type other than Null itself, but E? is nullable as normal.)

[srujzs]

Thank you both for the comments!

Maybe this comment is better suited in the proposal thread (and we can certainly move the discussion there if desired), but if we had a function f:

void f<X extends Object>() {}

f<E1>() where E1 implements Object is okay but f<E2>() where E2 implements Object? (this implements clause is redundant) is not okay with this change? And this is regardless of what the representation type is? This feels weirdly in contrast with Lasse's statement that "An extension type, E, cannot itself be nullable". This is also in contrast with classes, but I think that's reasonable because there are complexities that arise from having a nullable representation type that classes do not have to deal with.

If I understand correctly, if I want to get non-nullable semantics, I should have an implements Object clause on all of the JS types (e.g. extension type JSAny(backend_specific_implementation.JSAnyRepType) implements Object)? I'm also curious because I think we have some old JS interop helpers that do accept type parameters bound to Object and I wonder how these extension types play with those.

[lrhn]

You can't "implement Object?", but it's true that f<E2> is invalid if E2 doesn't implement Object, which is possible for an extension type, but not for a class type.

That hasn't changed, the only thing that was changed was whether some extension types implements Object or not. Now they only do if they are declared as such, regardless of their representation type. They can still only implement Object if their representation type does, but now only if they also ask for it.

[eernstg]

I should have an implements Object clause on all of the JS types

At least the ones that don't have a path to Object through some other superinterface. So it will suffice to add implements Object to JSAny (if I'm right in assuming that all the other JS... classes have an implements path to JSAny).

[johnniwinther]

The CFE change to require implements Object for non-nullability hasn't landed yet. When it does, it will change the .nullability property of some extension types.

cc @chloestefantsova

[chloestefantsova]

The CFE change to require implements Object for non-nullability hasn't landed yet. When it does, it will change the .nullability property of some extension types.

The CL tying extension type non-nullability to implementing Object has landed yesterday: https://dart-review.googlesource.com/c/sdk/+/333500.

[srujzs]

It seems like the fix in #53968 (comment) is irrelevant, so this needs a separate fix. @ the dart2wasm folks, is there anything I can help with to fix this? I think this is going to generally block us from moving to extension types.

[osa1]

Tentative fix: https://dart-review.googlesource.com/c/sdk/+/339400