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

Specify error for await e where the type of e has something to do with an extension type #3473

Closed
wants to merge 2 commits into from

Conversation

eernstg
Copy link
Member

@eernstg eernstg commented Nov 17, 2023

This PR adds several missing cases to the error about await e where the static type of e is an extension type, or it is one of several other types where an extension type occurs.

The basic idea is that we wish to make it an error to await e in the case where type static type of e is an extension type that does not declare that it implements Future<U> for any U (that is, Future is not reachable, neither directly nor indirectly, in the superinterface graph). There are several other cases where the static type of e is a type that contains such an extension type, and which basically unions of the basic case and something else. For instance, when it's an error to await E then it should also be an error to await E? and FutureOr<E> and FutureOr<E?>? and so on.

The conceptual reason why we want to make it an error when the static type is an extension type that does not declare that it implements Future is that we wish to eliminate the support for awaiting anything which is not a future (except that we may wish to keep allowing await null or something like that in order to support an explicit suspend operation), and awaiting an extension type that isn't declared to be a Future is such a case.

We could also have eliminated the extension type entirely by taking the extension type erasure first, which means that we would have relied on the representation type, and we would have ignored that the type was specified to be an extension type. This would be possible (and sound), but it would eliminate the compile-time encapsulation that the extension type provides. For instance, it would make the code more susceptible to subtle breakage if the representation type is changed due to evolution of the code. Of course, it is always possible to break this kind of encapsulation, but there is no need to break it gratuitously and systematically.

@eernstg eernstg requested a review from lrhn November 17, 2023 14:53
@eernstg
Copy link
Member Author

eernstg commented Nov 17, 2023

This PR was created with an eye to earlier proposals involving functions like future_base_type. It is similar in several ways, but not identical. In particular, it does not make any changes to the notion of "a type derives a future type", or to the function flatten(), and it treats await e differently when, for example, the static type of e is of the form FutureOr<FutureOr<...>> or FutureOr<T?>.

`e` satisfiers at least one of the following criteria:

- an extension type which is unrelated to `Future`.
- a type of the form `T?` or `FutureOr<T>` where `T` is a type that matches
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We think we should not include FutureOr<T> here. The type FutureOr<T> is very much releated to Future, even if T isn't. Actually, especially if T isnt.

You should be allowed to await FutureOr<Ext> where Ext is an extension type unrelated to Future.
We want to allow you to await FutureOr<T> for any T, because otherwise you'd have to do an (v is Future<T>) ? await v : v, which would be allowed and have all the same issues as what we are trying to avoid by disallowing it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the point was that if you have an expression of type E which is an extension type that has no relation to Future then you can trivially type that same expression as FutureOr<E>, which we can think of as Future<E> | E, and then it seems contradictory if it's ok to await that, but it is not OK to await E` alone. So I classified all the unions including the "don't await me" type as "don't await me" as well.

We could drop this whole idea and say that (1) await just uses the extension type erasure, no matter what (also for computation of the type of await e), and (2) it's up to a lint to tell people that it's a violation of some notion of encapsulation to have this particular await expression.


- an extension type which is unrelated to `Future`.
- a type of the form `T?` or `FutureOr<T>` where `T` is a type that matches
one of the items in this list.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we give the property of being in the list a name, then we can refer to it recursively here.

Say:

We say that a type `S` is *incompatible with `await`* if:
* `S` is an extension type which is unrelated to `Future`,
* `S` is `T?` and `T` is incompatible with `await`, or
* `S` is `T` bounded, where `T` is not `S`, and `T` is incompatible with `await`.

An expression `await e` is a compile-time error if the static type of `e` is incompatible with `await`.

(IIRC, a type is trivially bounded by itself, which I guess is why you use the "one of the earlier items" phrase to avoid infinite regress. I guess calling it a coinductive definition would allow the infinite recursion.)

@lrhn
Copy link
Member

lrhn commented Nov 17, 2023

This does look like it solves the isolated problem of disallowing await "on an extension type" as generally as possible.

A little too general, in that it disallows FutureOr<Ext> too, which I don't want to do. It's true that if Ext is unrelated to Future, then await FutureOr<Ext> will need to do an is Future<Ext> which can reveal an unrelated future embedded in the extension type. But that's a risk of allowing await on something of type Object too.

I think not being able to await a FutureOr<Ext> type is more prolematic than any problem with doing so.

@eernstg
Copy link
Member Author

eernstg commented Nov 17, 2023

This does look like it solves the isolated problem of disallowing await "on an extension type" as generally as possible.

It's more like finding a single core case with a motivation: It is a violation of encapsulation to have await e when e is an extension type which is unrelated to Future (the representation type could be unrelated to Future or not, but it is a violation of encapsulation to use that distinction for anything). The remaining error cases are derived from that by being unions that include this core case. So it's not as general as possible, it's just intended to flag the core case, even when it's one among several operands to a union operation.

@lrhn lrhn mentioned this pull request Jan 10, 2024
@eernstg
Copy link
Member Author

eernstg commented Jan 17, 2024

Closing this PR: The same underlying issue was handled in #3560, and this PR is now obsolete.

@eernstg eernstg closed this Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants