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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2712473003: closure conversion: Support closures in initializers (Closed)

Created:
3 years, 10 months ago by karlklose
Modified:
3 years, 9 months ago
Reviewers:
ahe
CC:
reviews_dartlang.org
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Support closures in initializers Moved the code to modify the current block with context updates to its own class and added another class for context updates in initializer expressions. R=ahe@google.com Committed: https://github.com/dart-lang/sdk/commit/5a42bf8062b63839e69f75849ec1fc04f257cfdc

Patch Set 1 #

Patch Set 2 : Implement AST rewriter for initializers #

Patch Set 3 : Comments, TODOs, etc. #

Patch Set 4 : Remove parameter #

Total comments: 13

Patch Set 5 : Address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+431 lines, -86 lines) Patch
M pkg/kernel/lib/transformations/closure/context.dart View 1 5 chunks +9 lines, -19 lines 0 comments Download
M pkg/kernel/lib/transformations/closure/converter.dart View 1 2 3 4 11 chunks +73 lines, -67 lines 0 comments Download
A pkg/kernel/lib/transformations/closure/rewriter.dart View 1 2 3 4 1 chunk +125 lines, -0 lines 0 comments Download
A pkg/kernel/testcases/closures/closure_in_constructor.dart View 1 2 3 4 1 chunk +20 lines, -0 lines 0 comments Download
A pkg/kernel/testcases/closures/closure_in_constructor.dart.expect View 1 1 chunk +49 lines, -0 lines 0 comments Download
A pkg/kernel/testcases/closures/closure_in_initializer.dart View 1 2 3 4 1 chunk +18 lines, -0 lines 0 comments Download
A pkg/kernel/testcases/closures/closure_in_initializer.dart.expect View 1 1 chunk +46 lines, -0 lines 0 comments Download
A pkg/kernel/testcases/closures/closure_in_initializer_closure.dart View 1 2 3 4 1 chunk +25 lines, -0 lines 0 comments Download
A pkg/kernel/testcases/closures/closure_in_initializer_closure.dart.expect View 1 2 1 chunk +63 lines, -0 lines 0 comments Download
M pkg/kernel/testcases/closures/type_variables.dart.expect View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (6 generated)
karlklose
3 years, 10 months ago (2017-02-23 06:47:03 UTC) #5
ahe
lgtm https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/lib/transformations/closure/converter.dart File pkg/kernel/lib/transformations/closure/converter.dart (right): https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/lib/transformations/closure/converter.dart#newcode216 pkg/kernel/lib/transformations/closure/converter.dart:216: .forEach(extendContextWith); I'm curious, is this manual formatting or ...
3 years, 9 months ago (2017-02-27 07:36:06 UTC) #7
karlklose
Committed patchset #5 (id:140001) manually as 5a42bf8062b63839e69f75849ec1fc04f257cfdc (presubmit successful).
3 years, 9 months ago (2017-02-28 10:16:48 UTC) #9
karlklose
3 years, 9 months ago (2017-02-28 11:45:15 UTC) #10
Message was sent while issue was closed. 
https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/lib/transfo...
File pkg/kernel/lib/transformations/closure/converter.dart (right):

https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/lib/transfo...
pkg/kernel/lib/transformations/closure/converter.dart:216:
.forEach(extendContextWith);
On 2017/02/27 07:36:06, ahe wrote:
> I'm curious, is this manual formatting or dartfmt?
> I like it.

I formatted it manually but the formatter produces the same.

https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/lib/transfo...
pkg/kernel/lib/transformations/closure/converter.dart:226: throw 'unexpeced node
$parent';
On 2017/02/27 07:36:06, ahe wrote:
> Consider rewriting this message to be a complete sentence.

Done.

https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/lib/transfo...
File pkg/kernel/lib/transformations/closure/rewriter.dart (right):

https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/lib/transfo...
pkg/kernel/lib/transformations/closure/rewriter.dart:87: int _insertionIndex;
On 2017/02/27 07:36:06, ahe wrote:
> Move to beginning of class declaration.

Done.

https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/testcases/c...
File pkg/kernel/testcases/closures/closure_in_constructor.dart (right):

https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/testcases/c...
pkg/kernel/testcases/closures/closure_in_constructor.dart:1: class C1 {
On 2017/02/27 07:36:06, ahe wrote:
> Add copyright.

Done.

https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/testcases/c...
File pkg/kernel/testcases/closures/closure_in_initializer.dart (right):

https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/testcases/c...
pkg/kernel/testcases/closures/closure_in_initializer.dart:1: class C {
On 2017/02/27 07:36:06, ahe wrote:
> Add copyright.

Done.

https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/testcases/c...
File pkg/kernel/testcases/closures/closure_in_initializer_closure.dart (right):

https://codereview.chromium.org/2712473003/diff/120001/pkg/kernel/testcases/c...
pkg/kernel/testcases/closures/closure_in_initializer_closure.dart:1: class C {
On 2017/02/27 07:36:06, ahe wrote:
> Add copyright.

Done.

Powered by Google App Engine
This is Rietveld 408576698