Issue 16146008: Simplify AST by extending LetNode and replace CommaNode.

Issue 16146008: Simplify AST by extending LetNode and replace CommaNode. (Closed)

Created:
7 years, 6 months ago by Florian Schneider

Modified:
7 years, 6 months ago

Reviewers:
Kevin Millikin (Google)

CC:
reviews_dartlang.org, vm-dev_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Visibility:
Public.

More Reviews

Description

Simplify AST by extending LetNode and replace CommaNode. LetNode now supports a list of expressions as the body. The last expression is the result value of the let-expression. R=kmillikin@google.com Committed: https://code.google.com/p/dart/source/detail?r=23678

Patch Set 1 #

Total comments: 4

Created: 7 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+64 lines, -106 lines)			Patch
M	runtime/vm/ast.h	View	4 chunks	+5 lines, -34 lines	0 comments	Download
M	runtime/vm/ast.cc	View	2 chunks	+4 lines, -2 lines	0 comments	Download
M	runtime/vm/ast_printer.cc	View	1 chunk	+0 lines, -5 lines	0 comments	Download
M	runtime/vm/flow_graph_builder.h	View	1 chunk	+0 lines, -1 line	0 comments	Download
M	runtime/vm/flow_graph_builder.cc	View	2 chunks	+36 lines, -35 lines	4 comments	Download
M	runtime/vm/parser.cc	View	7 chunks	+19 lines, -29 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Kevin Millikin (Google)

Yes, LGTM. https://codereview.chromium.org/16146008/diff/1/runtime/vm/flow_graph_builder.cc File runtime/vm/flow_graph_builder.cc (right): https://codereview.chromium.org/16146008/diff/1/runtime/vm/flow_graph_builder.cc#newcode1896 runtime/vm/flow_graph_builder.cc:1896: node->nodes()[node->nodes().length() - 1]->Visit(&for_value); node->nodes()->Last() https://codereview.chromium.org/16146008/diff/1/runtime/vm/flow_graph_builder.cc#newcode1923 runtime/vm/flow_graph_builder.cc:1923: node->nodes()[node->nodes().length() ...

7 years, 6 months ago (2013-06-06 12:18:33 UTC) #2

Florian Schneider

Committed patchset #1 manually as r23678 (presubmit successful).

7 years, 6 months ago (2013-06-06 12:32:49 UTC) #3

Florian Schneider

7 years, 6 months ago (2013-06-06 12:33:39 UTC) #4

Message was sent while issue was closed.

https://codereview.chromium.org/16146008/diff/1/runtime/vm/flow_graph_builder.cc
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/16146008/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:1896: node->nodes()[node->nodes().length() -
1]->Visit(&for_value);
On 2013/06/06 12:18:33, kmillikin wrote:
> node->nodes()->Last()

Done.

https://codereview.chromium.org/16146008/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:1923: node->nodes()[node->nodes().length() -
1]->Visit(&for_value);
On 2013/06/06 12:18:33, kmillikin wrote:
> Also here.

Done.

Expand Messages | Collapse Messages