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

New Future constructor: Future.void #53364

Open
srawlins opened this issue Aug 28, 2023 · 4 comments
Open

New Future constructor: Future.void #53364

srawlins opened this issue Aug 28, 2023 · 4 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). library-async type-enhancement A request for a change that isn't a bug

Comments

@srawlins
Copy link
Member

If we want to change the signature of Future.value to feature a required parameter, then the shorthand, Future.value() (with zero args), used for Future<void>s or Future<Null> perhaps, would disappear. Future.void() would be a nice way to migrate code incrementally. Would there maybe be const / performance benefits to this as well?

Is there a similar incremental move for Completer.complete() (zero arguments)? Maybe a Completer.completeVoid() method? It would be cool to make it an extension method, in an extension on Completer<void>. Unfortunately, an extension on Completer<void> matches statically on any-and-all Completer types.

@srawlins srawlins added the area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). label Aug 28, 2023
@leafpetersen leafpetersen added the area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. label Nov 16, 2023
@leafpetersen
Copy link
Member

cc @jakemac53

@jakemac53
Copy link
Contributor

jakemac53 commented Nov 16, 2023

A related thing I was realizing today is that once we are in sound mode everywhere, I believe that we have a much stronger case for changing Future.value to have a required parameter, with a dart fix to auto pass null for anything that calls it with no arguments today (or use this new constructor for "void" futures).

In a sound world, this only changes a guaranteed runtime failure into a static failure, where in an unsound world the runtime failure was not guaranteed, so it was a harder sell.

@lrhn lrhn added library-async type-enhancement A request for a change that isn't a bug labels Nov 16, 2023
@lrhn
Copy link
Member

lrhn commented Nov 16, 2023

Would love to make that breaking change (and I'm sure there are a couple of other similar functions with an optional argument which is only really optional if the type parameter is nullable).

Adding Future.nullFuture or Future.voidFuture is possible, but it won't be const.

We have had enough trouble with out internal _Future._nullFuture which was created once in the root zone, and reused in other zones. Every fetch of Future.nullFuture would give you a future set to the current zone. It can be cached on the zone, so each zone only needs one, but it's equally likely that it will just be a new one each time. (Futures are not that big, and reusing a future is likely to make it end up in old-space and require old-to-new-space pointers when you add a new listener to it, which is bad for GC, according to my limited GC knowledge.)

(The internal "null future" was added as a hack to make some other code recognize it and avoid awaiting it, to be timing compatible with an earlier non-null-safe API where someone could return null and not be delayed. It wasn't done to save on allocations. Please don't write timing-dependent code.)

I'd probably make the migration just add the null to Future<T>.value(null), because that turns into an error where T isn't nullable, and retains precisely the current behavior when T is nullable.

@jakemac53
Copy link
Contributor

Would love to make that breaking change (and I'm sure there are a couple of other similar functions with an optional argument which is only really optional if the type parameter is nullable).

Completer.complete for sure, probably others too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). library-async type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants