Commit Graph

4 Commits

Author SHA1 Message Date
Lasse R.H. Nielsen f8086c81ae Collect all test-related files in package:expect.
Collects files from `package:async_helper` and `tests/language`
that are generally useful, so that all test-related helpers are
in `package:expect`.

Moves the two libraries from `package:async_helper` into `package:expect`,
and the `tests/language/static_type_helper.dart` file too.

Deprecates `async_minitest.dart`, to follow `minitest.dart`,
expecting the Flutter use of it to have been fixed to not break
on deprecation (I believe Flutter no longer breaks builds on deprecations at all).

Patch 1 is the actual change.
Patch 2+4+8 is changing all existing references to the files.
Patch 6 ignores deprecation in files still using `async_minitest.dart`.

3+5+7+9 are updating this text to make the numbers match.
Then it's just test-expectations and small tweaks from there.

Change-Id: I1b665135b5fef9b9a0c3b340ffe9daf874d0174c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373120
Reviewed-by: Nate Bosch <nbosch@google.com>
Reviewed-by: Devon Carew <devoncarew@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
2024-10-11 16:53:52 +00:00
Lasse R.H. Nielsen b06a34fc5a Don't allow completing a _Future with itself.
This never worked. It was silently accepted, at least if the future had no listeners, then any later attempt to use the future would cause a stack overflow or other impossible results.

Moves some `_Future._complete` call out of try-catch.
The `_complete` shouldn't throw (but before this fix it could).
Moving them out of the `try`/`catch` makes such errors be reported
as unhandled, instead of catching them and trying to complete the same
future again with an error, when it's possibly in an inconsistent state.

Fixes #43662.
Based on https://github.com/dart-lang/sdk/issues/43662#issuecomment-2058870247

CoreLibraryReviewExempt: No response.
Bug: http://dartbug.com/43662
Change-Id: I96a4f01bcd5b6cee93bba267299852569a9b905c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363060
Commit-Queue: Lasse Nielsen <lrn@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
2024-05-14 15:44:07 +00:00
Siva Annamalai 5cb3b37c74 Revert "Refactor _Future."
This reverts commit 69f32d6ad7.

Reason for revert: We seem to have a number of tests failing with timeouts in CBUILD after this change, please see logs at
https://dart-in-g3-qa-prod.corp.google.com/dg3/Home#/cbuild/find/69f32d6ad7e724e3148cb2eb6601e63165e76ad3

Original change's description:
> Refactor `_Future`.
>
> This is a major rewrite of the `_Future` class,
> which is the default implementation of the `Future` interface.
>
> The main goal was to reduce the number of expensive type checks
> in the internal passing around of data.
> Expensive type checks are things like
> * `is _Future<T>` (more expensive than just `is _Future`, the latter
>   can be a single class-ID check.
> * Covariant generic parameter checks (using `T` covariantly in a
>   parameter forces a run-time type check).
>
> Also removed some plain unnecessary casts and turned some
> implicit casts from `dynamic` into `unsafeCast`s.
>
> This seems to be an success, at least on very primitive benchmarks, according to Golem:
> FutureCatchErrorTest    41.22% (1.9 noise)
> FutureValueTest         46.51% (2.8 noise)
> EmptyFutureTest         59.15% (3.1 noise)
> FutureWhenCompleteTest  51.10% (3.2 noise)
>
> A secondary goal was to clean up a very old and messy class,
> and make it clearer for other `dart:async` how to interact
> with the future.
>
> The change has a memory cost: The `_FutureListener<S,T>` class,
> which represents a `then`, `catchError` or `whenComplete`
> call on a `_Future`, now contains a reference to its source future,
> the one which provides the inputs to the callbacks,
> as well as the result future returned by the call.
> That's one extra memory slot per listener.
>
> In return, the `_FutureListener` now does not need to
> get its source future as an argument, which needs a covariant
> generic type check, and the methods of `_Future` can be written
> in a way which ignores the type parameters of both `_Future`
> and `_FutureListener`, which reduces complex type checks
> significantly.
>
> In general, typed code is in `_FutureListener`, which knows both
> the source and target types of the listener callbacks, and which
> contains the futures already at that type, so no extra type checking
> is needed.
> The `_Future` class is mostly untyped, except for its "public"
> API, called by other classes, which checks inputs,
> and code interacting with non-native futures.
> Invariants ensure that only correctly typed values
> are stored in the untyped shared `_resultOrListeners` field
> on `_Future`, as determined by its `_state` integer.
> (This was already partially true, and has simply been made
> more consistent.)
>
> Further, we now throw an error in a situation that was previously
> unhandled: When a `_Future` is completed with *itself*.
> That would ensure that the future would never complete
> (it waits for itself to complete before it can complete),
> and may potentially have caused weird loops in the representation.
> In practice, it probably never happens. Now it makes the error
> fail with an error.
> Currently a private `_FutureCyclicDependencyError` which presents
> as an `UnsupportedError`.
> That avoids code like
> ```dart
> import "dart:async";
> void main() {
>   var c = Completer();
>   c.complete(c.future); // bad.
>   print("well!");
>   var d = Completer();
>   d.complete(c.future);
>   print("shucks!");
> }
> ```
> from hanging the runtime by busily searching for the end of a cycle.
>
> See https://github.com/dart-lang/sdk/issues/48225
> Fixes #48225
>
> TEST= refactoring covered by existing tests, few new tests.
>
> Change-Id: Id9fc5af5fe011deb0af3e1e8a4ea3a91799f9da4
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244241
> Reviewed-by: Martin Kustermann <kustermann@google.com>
> Commit-Queue: Lasse Nielsen <lrn@google.com>

TBR=lrn@google.com,kustermann@google.com,sra@google.com,sigmund@google.com,nshahan@google.com

Change-Id: I455be5a04b4c346df26d4ded0fa7388baccb0f8c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247762
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
2022-06-09 16:51:55 +00:00
Lasse R.H. Nielsen 69f32d6ad7 Refactor _Future.
This is a major rewrite of the `_Future` class,
which is the default implementation of the `Future` interface.

The main goal was to reduce the number of expensive type checks
in the internal passing around of data.
Expensive type checks are things like
* `is _Future<T>` (more expensive than just `is _Future`, the latter
  can be a single class-ID check.
* Covariant generic parameter checks (using `T` covariantly in a
  parameter forces a run-time type check).

Also removed some plain unnecessary casts and turned some
implicit casts from `dynamic` into `unsafeCast`s.

This seems to be an success, at least on very primitive benchmarks, according to Golem:
FutureCatchErrorTest    41.22% (1.9 noise)
FutureValueTest         46.51% (2.8 noise)
EmptyFutureTest         59.15% (3.1 noise)
FutureWhenCompleteTest  51.10% (3.2 noise)

A secondary goal was to clean up a very old and messy class,
and make it clearer for other `dart:async` how to interact
with the future.

The change has a memory cost: The `_FutureListener<S,T>` class,
which represents a `then`, `catchError` or `whenComplete`
call on a `_Future`, now contains a reference to its source future,
the one which provides the inputs to the callbacks,
as well as the result future returned by the call.
That's one extra memory slot per listener.

In return, the `_FutureListener` now does not need to
get its source future as an argument, which needs a covariant
generic type check, and the methods of `_Future` can be written
in a way which ignores the type parameters of both `_Future`
and `_FutureListener`, which reduces complex type checks
significantly.

In general, typed code is in `_FutureListener`, which knows both
the source and target types of the listener callbacks, and which
contains the futures already at that type, so no extra type checking
is needed.
The `_Future` class is mostly untyped, except for its "public"
API, called by other classes, which checks inputs,
and code interacting with non-native futures.
Invariants ensure that only correctly typed values
are stored in the untyped shared `_resultOrListeners` field
on `_Future`, as determined by its `_state` integer.
(This was already partially true, and has simply been made
more consistent.)

Further, we now throw an error in a situation that was previously
unhandled: When a `_Future` is completed with *itself*.
That would ensure that the future would never complete
(it waits for itself to complete before it can complete),
and may potentially have caused weird loops in the representation.
In practice, it probably never happens. Now it makes the error
fail with an error.
Currently a private `_FutureCyclicDependencyError` which presents
as an `UnsupportedError`.
That avoids code like
```dart
import "dart:async";
void main() {
  var c = Completer();
  c.complete(c.future); // bad.
  print("well!");
  var d = Completer();
  d.complete(c.future);
  print("shucks!");
}
```
from hanging the runtime by busily searching for the end of a cycle.

See https://github.com/dart-lang/sdk/issues/48225
Fixes #48225

TEST= refactoring covered by existing tests, few new tests.

Change-Id: Id9fc5af5fe011deb0af3e1e8a4ea3a91799f9da4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244241
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
2022-06-09 15:32:33 +00:00