Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2131)

Issue 2893893002: Use generic functions in zones. (Closed)

Created:
3 years, 7 months ago by floitsch
Modified:
3 years, 2 months ago
Reviewers:
vsm, regis
CC:
reviews_dartlang.org
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Use generic functions in zones. Migrated to Gerrit: https://dart-review.googlesource.com/c/sdk/+/4942

Patch Set 1 #

Patch Set 2 : more fixes. #

Patch Set 3 : Fix places where we passed in too many generic arguments. #

Patch Set 4 : Rebase #

Patch Set 5 : Remove spurious `return null`. #

Patch Set 6 : A few fixes and cleanups. #

Patch Set 7 : Add missing returns. #

Patch Set 8 : Change synchronous error handling of `runZoned`. #

Total comments: 2

Patch Set 9 : Fix bad assert. #

Patch Set 10 : Run go.sh. #

Patch Set 11 : Update tests and status files. #

Patch Set 12 : Fix tests and update status expectations. #

Patch Set 13 : Use `dynamic` as required return type. #

Total comments: 3

Patch Set 14 : Don't use generic function types as type arguments. #

Total comments: 2

Patch Set 15 : Use `_ZoneFunction<Function>` also for the _RootZone. #

Patch Set 16 : Fix more tests. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+452 lines, -6667 lines) Patch
M pkg/compiler/lib/src/common/tasks.dart View 1 2 3 4 3 chunks +5 lines, -3 lines 0 comments Download
M pkg/dev_compiler/test/browser/language_tests.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +1 line, -2 lines 0 comments Download
M pkg/dev_compiler/tool/input_sdk/lib/html/dart2js/html_dart2js.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +11 lines, -17 lines 0 comments Download
M sdk/lib/async/future.dart View 1 2 3 4 5 6 7 8 9 10 12 13 2 chunks +2 lines, -2 lines 0 comments Download
M sdk/lib/async/schedule_microtask.dart View 12 13 1 chunk +1 line, -2 lines 0 comments Download
M sdk/lib/async/stream_impl.dart View 1 2 3 4 5 6 7 8 9 10 12 13 2 chunks +4 lines, -5 lines 0 comments Download
M sdk/lib/async/timer.dart View 12 13 2 chunks +3 lines, -6 lines 0 comments Download
M sdk/lib/async/zone.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 29 chunks +247 lines, -215 lines 0 comments Download
M sdk/lib/html/dart2js/html_dart2js.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +11 lines, -17 lines 0 comments Download
M sdk/lib/html/dartium/html_dartium.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +10 lines, -16 lines 0 comments Download
M sdk/lib/js/dartium/cached_patches.dart View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +5 lines, -6259 lines 0 comments Download
M tests/co19/co19-co19.status View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +8 lines, -0 lines 0 comments Download
M tests/compiler/dart2js/dart2js.status View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +2 lines, -0 lines 0 comments Download
M tests/lib/async/async_await_zones_test.dart View 3 chunks +6 lines, -3 lines 0 comments Download
M tests/lib/async/future_timeout_test.dart View 1 chunk +2 lines, -1 line 0 comments Download
M tests/lib/async/run_zoned6_test.dart View 1 2 3 4 5 6 7 8 9 10 1 chunk +5 lines, -6 lines 0 comments Download
M tests/lib/async/run_zoned9_test.dart View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -4 lines 0 comments Download
M tests/lib/async/stream_listen_zone_test.dart View 1 chunk +8 lines, -7 lines 0 comments Download
M tests/lib/async/zone_bind_callback_test.dart View 2 chunks +3 lines, -2 lines 0 comments Download
M tests/lib/async/zone_bind_callback_unary_test.dart View 1 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -2 lines 0 comments Download
M tests/lib/async/zone_debug_test.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 5 chunks +10 lines, -8 lines 0 comments Download
M tests/lib/async/zone_error_callback_test.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +6 lines, -3 lines 0 comments Download
M tests/lib/async/zone_fork_test.dart View 2 chunks +3 lines, -3 lines 0 comments Download
M tests/lib/async/zone_register_callback_test.dart View 1 chunk +2 lines, -1 line 0 comments Download
M tests/lib/async/zone_register_callback_unary_test.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -1 line 0 comments Download
M tests/lib/async/zone_root_bind_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/lib/async/zone_run_guarded_test.dart View 4 chunks +9 lines, -6 lines 0 comments Download
M tests/lib/async/zone_run_test.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M tests/lib/async/zone_run_unary_test.dart View 1 chunk +7 lines, -4 lines 0 comments Download
M tests/lib/mirrors/invocation_fuzz_test.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M tests/lib_strong/async/async_await_zones_test.dart View 4 chunks +7 lines, -4 lines 0 comments Download
M tests/lib_strong/async/future_timeout_test.dart View 1 chunk +2 lines, -1 line 0 comments Download
M tests/lib_strong/async/run_zoned6_test.dart View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +5 lines, -6 lines 0 comments Download
M tests/lib_strong/async/run_zoned9_test.dart View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -4 lines 0 comments Download
M tests/lib_strong/async/stream_listen_zone_test.dart View 1 chunk +8 lines, -7 lines 0 comments Download
M tests/lib_strong/async/zone_bind_callback_test.dart View 2 chunks +3 lines, -2 lines 0 comments Download
M tests/lib_strong/async/zone_bind_callback_unary_test.dart View 1 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -2 lines 0 comments Download
M tests/lib_strong/async/zone_debug_test.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M tests/lib_strong/async/zone_fork_test.dart View 2 chunks +3 lines, -3 lines 0 comments Download
M tests/lib_strong/async/zone_register_callback_test.dart View 1 chunk +2 lines, -1 line 0 comments Download
M tests/lib_strong/async/zone_register_callback_unary_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/lib_strong/async/zone_root_bind_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/lib_strong/async/zone_run_guarded_test.dart View 3 chunks +10 lines, -6 lines 0 comments Download
M tests/lib_strong/async/zone_run_test.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M tests/lib_strong/async/zone_run_unary_test.dart View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +4 lines, -4 lines 0 comments Download
M tests/lib_strong/mirrors/invocation_fuzz_test.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M tools/dom/src/EventStreamProvider.dart View 1 2 3 4 3 chunks +5 lines, -5 lines 0 comments Download
M tools/dom/src/shared_html.dart View 1 chunk +5 lines, -11 lines 0 comments Download
M tools/dom/templates/html/impl/impl_Window.darttemplate View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -1 line 0 comments Download
M tools/testing/dart/test_runner.dart View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +3 lines, -2 lines 0 comments Download
M utils/compiler/create_snapshot_entry.dart View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 20 (3 generated)
vsm
https://codereview.chromium.org/2893893002/diff/140001/sdk/lib/async/zone.dart File sdk/lib/async/zone.dart (right): https://codereview.chromium.org/2893893002/diff/140001/sdk/lib/async/zone.dart#newcode1495 sdk/lib/async/zone.dart:1495: assert(onError is! ZoneUnaryCallback<R, Object>); This assert is failing in ...
3 years, 6 months ago (2017-06-16 13:05:59 UTC) #2
floitsch
https://codereview.chromium.org/2893893002/diff/140001/sdk/lib/async/zone.dart File sdk/lib/async/zone.dart (right): https://codereview.chromium.org/2893893002/diff/140001/sdk/lib/async/zone.dart#newcode1495 sdk/lib/async/zone.dart:1495: assert(onError is! ZoneUnaryCallback<R, Object>); On 2017/06/16 13:05:59, vsm wrote: ...
3 years, 6 months ago (2017-06-16 19:54:17 UTC) #3
ahe
As far as I can tell, this no longer breaks Fasta. But Zones don't work ...
3 years, 5 months ago (2017-07-05 13:43:54 UTC) #4
floitsch
On 2017/07/05 13:43:54, ahe wrote: > As far as I can tell, this no longer ...
3 years, 5 months ago (2017-07-05 14:00:41 UTC) #5
ahe
On 2017/07/05 14:00:41, floitsch wrote: > On 2017/07/05 13:43:54, ahe wrote: > > As far ...
3 years, 5 months ago (2017-07-05 14:23:50 UTC) #6
ahe
On 2017/07/05 14:23:50, ahe wrote: > On 2017/07/05 14:00:41, floitsch wrote: > > On 2017/07/05 ...
3 years, 5 months ago (2017-07-05 14:27:01 UTC) #7
regis
https://codereview.chromium.org/2893893002/diff/240001/sdk/lib/async/zone.dart File sdk/lib/async/zone.dart (right): https://codereview.chromium.org/2893893002/diff/240001/sdk/lib/async/zone.dart#newcode802 sdk/lib/async/zone.dart:802: _ZoneFunction<RunHandler> get _run; It was pointed to me that ...
3 years, 5 months ago (2017-07-24 18:21:10 UTC) #9
floitsch
https://codereview.chromium.org/2893893002/diff/240001/sdk/lib/async/zone.dart File sdk/lib/async/zone.dart (right): https://codereview.chromium.org/2893893002/diff/240001/sdk/lib/async/zone.dart#newcode802 sdk/lib/async/zone.dart:802: _ZoneFunction<RunHandler> get _run; On 2017/07/24 18:21:10, regis wrote: > ...
3 years, 4 months ago (2017-07-27 14:11:36 UTC) #10
regis
On 2017/07/27 14:11:36, floitsch wrote: > https://codereview.chromium.org/2893893002/diff/240001/sdk/lib/async/zone.dart > File sdk/lib/async/zone.dart (right): > > https://codereview.chromium.org/2893893002/diff/240001/sdk/lib/async/zone.dart#newcode802 > ...
3 years, 4 months ago (2017-07-27 16:30:21 UTC) #11
floitsch
https://codereview.chromium.org/2893893002/diff/240001/sdk/lib/async/zone.dart File sdk/lib/async/zone.dart (right): https://codereview.chromium.org/2893893002/diff/240001/sdk/lib/async/zone.dart#newcode802 sdk/lib/async/zone.dart:802: _ZoneFunction<RunHandler> get _run; On 2017/07/27 14:11:35, floitsch wrote: > ...
3 years, 4 months ago (2017-08-02 15:27:52 UTC) #12
regis
https://codereview.chromium.org/2893893002/diff/260001/sdk/lib/async/zone.dart File sdk/lib/async/zone.dart (right): https://codereview.chromium.org/2893893002/diff/260001/sdk/lib/async/zone.dart#newcode1242 sdk/lib/async/zone.dart:1242: _ZoneFunction<RunHandler> get _run => The same restriction applies here. ...
3 years, 4 months ago (2017-08-08 20:31:21 UTC) #13
regis
On 2017/08/08 20:31:21, regis wrote: > https://codereview.chromium.org/2893893002/diff/260001/sdk/lib/async/zone.dart > File sdk/lib/async/zone.dart (right): > > https://codereview.chromium.org/2893893002/diff/260001/sdk/lib/async/zone.dart#newcode1242 > ...
3 years, 4 months ago (2017-08-09 00:53:30 UTC) #14
floitsch
@Regis: thanks! These failures were very helpful. I updated the tests and filed a bug ...
3 years, 4 months ago (2017-08-09 10:34:32 UTC) #15
regis
On 2017/08/09 10:34:32, floitsch wrote: > @Regis: thanks! > These failures were very helpful. I ...
3 years, 4 months ago (2017-08-09 22:27:56 UTC) #16
regis
On 2017/08/09 22:27:56, regis wrote: > On 2017/08/09 10:34:32, floitsch wrote: > > @Regis: thanks! ...
3 years, 4 months ago (2017-08-15 22:42:43 UTC) #17
regis
On 2017/08/15 22:42:43, regis wrote: > On 2017/08/09 22:27:56, regis wrote: > > On 2017/08/09 ...
3 years, 2 months ago (2017-09-25 20:13:12 UTC) #18
floitsch
3 years, 2 months ago (2017-09-26 10:45:28 UTC) #19
On 2017/09/25 20:13:12, regis wrote:
> On 2017/08/15 22:42:43, regis wrote:
> > On 2017/08/09 22:27:56, regis wrote:
> > > On 2017/08/09 10:34:32, floitsch wrote:
> > > > @Regis: thanks!
> > > > These failures were very helpful. I updated the tests and filed a bug
> > against
> > > > co19: https://github.com/dart-lang/co19/issues/121
> > > > 
> > > > If you have more failures I would be happy to look at them.
> > > > 
> > > >
> >
https://codereview.chromium.org/2893893002/diff/260001/sdk/lib/async/zone.dart
> > > > File sdk/lib/async/zone.dart (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2893893002/diff/260001/sdk/lib/async/zone.dar...
> > > > sdk/lib/async/zone.dart:1242: _ZoneFunction<RunHandler> get _run =>
> > > > On 2017/08/08 20:31:21, regis wrote:
> > > > > The same restriction applies here. RunHandler is a generic function
type
> > not
> > > > > allowed as a type argument.
> > > > 
> > > > argh. Missed these ones.
> > > > done.
> > > 
> > > I do not see any more failures. Thanks!
> > 
> > Florian, Vijay, do you have any estimate on when this cl will be committed?
> Any
> > road blocks remaining? It has been pending for almost 3 months and it would
be
> > great if you can submit it. Note that reification of generic functions is
not
> > yet enabled by default in the VM, so this should not be a concern to you.
> > 
> > Thanks,
> > Regis
> 
> Do I understand correctly that this cl is now obsolete, because it was patched
> in as part of https://codereview.chromium.org/3014593002 ?

Yes: this CL is obsolete.
No: https://codereview.chromium.org/3014593002 was just a CL for dartium.
  The CL that made this one obsolete was:
https://dart-review.googlesource.com/c/sdk/+/4942

Will update the description.

Powered by Google App Engine
This is Rietveld 408576698