Refactor IrBuilder to use SemanticVisitor.

Refactor IrBuilder to use SemanticVisitor.

BUG=
R=floitsch@google.com, karlklose@google.com

Committed: https://code.google.com/p/dart/source/detail?r=44628

[Johnni Winther, 2015-03-12 13:47:51]

johnniwinther@google.com changed reviewers:
+ karlklose@google.com

[Johnni Winther, 2015-03-12 13:47:52]

Ready for review of the overall structure.

Still needs to fixs small bugs here-and-there.

[floitsch, 2015-03-12 18:04:01]

floitsch@google.com changed reviewers:
+ floitsch@google.com

[floitsch, 2015-03-12 18:04:02]

DBC.

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart (right):

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:14: import 'cps_ir_nodes.dart'
as ir;
why this change?

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:785: // TODO(karlklose,sigurdm):
build different nodes for getters.
I guess this comment should be one down.

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:796: // TODO(karlklose,sigurdm):
build different nodes for setters.
ditto.

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:128:
GroupStaticMixin<ir.Primitive, dynamic>,
Just looking at this, I have no idea what this (and the other) Mixin does.
we should discuss a better name.

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:603: // ==== Sends ====
nit (and not your code):
if we have sections in the sources, let's use Markdown sections.

[Johnni Winther, 2015-03-16 13:28:13]

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart (right):

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:14: import 'cps_ir_nodes.dart'
as ir;
On 2015/03/12 18:04:02, floitsch wrote:
> why this change?

Rebasing did it. Reverted.

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:785: // TODO(karlklose,sigurdm):
build different nodes for getters.
On 2015/03/12 18:04:02, floitsch wrote:
> I guess this comment should be one down.

Done.

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:796: // TODO(karlklose,sigurdm):
build different nodes for setters.
On 2015/03/12 18:04:02, floitsch wrote:
> ditto.

Done.

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:128:
GroupStaticMixin<ir.Primitive, dynamic>,
On 2015/03/12 18:04:02, floitsch wrote:
> Just looking at this, I have no idea what this (and the other) Mixin does.
> we should discuss a better name.

Is 'GroupStaticSemantics' better ?

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:603: // ==== Sends ====
On 2015/03/12 18:04:02, floitsch wrote:
> nit (and not your code):
> if we have sections in the sources, let's use Markdown sections.

Converted into 2nd level headers.

[floitsch, 2015-03-16 13:38:42]

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:128:
GroupStaticMixin<ir.Primitive, dynamic>,
On 2015/03/16 13:28:13, Johnni Winther wrote:
> On 2015/03/12 18:04:02, floitsch wrote:
> > Just looking at this, I have no idea what this (and the other) Mixin does.
> > we should discuss a better name.
> 
> Is 'GroupStaticSemantics' better ?

Maybe BaseImplementationOfStaticsMixin?

[karlklose, 2015-03-18 10:42:40]

On 2015/03/16 13:38:42, floitsch wrote:
>
https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
> File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):
> 
>
https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
> pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:128:
> GroupStaticMixin<ir.Primitive, dynamic>,
> On 2015/03/16 13:28:13, Johnni Winther wrote:
> > On 2015/03/12 18:04:02, floitsch wrote:
> > > Just looking at this, I have no idea what this (and the other) Mixin does.
> > > we should discuss a better name.
> > 
> > Is 'GroupStaticSemantics' better ?
> 
> Maybe BaseImplementationOfStaticsMixin?

Structure LGMT.

The formatting is wrong at a few places and I think we should find better names
for the mixins.

[karlklose, 2015-03-18 10:42:56]

https://codereview.chromium.org/1004683002/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1004683002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1043:
irBuilder.buildSuperGet(field),
Indentation is wrong.

https://codereview.chromium.org/1004683002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1165: List<ir.Primitive>
arguments = [indexValue];
Add type arguments to list literal.

https://codereview.chromium.org/1004683002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1200: List<ir.Primitive>
arguments = [visit(rhs)];
Type argument for list literal.

[karlklose, 2015-03-19 12:20:31]

LGTM.

[Johnni Winther, 2015-03-20 13:41:00]

PTAL @ the last patch

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1004683002/diff/1/pkg/compiler/lib/src/cps_ir...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:128:
GroupStaticMixin<ir.Primitive, dynamic>,
On 2015/03/16 13:38:41, floitsch wrote:
> On 2015/03/16 13:28:13, Johnni Winther wrote:
> > On 2015/03/12 18:04:02, floitsch wrote:
> > > Just looking at this, I have no idea what this (and the other) Mixin does.
> > > we should discuss a better name.
> > 
> > Is 'GroupStaticSemantics' better ?
> 
> Maybe BaseImplementationOfStaticsMixin?

Done.

https://codereview.chromium.org/1004683002/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1004683002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1043:
irBuilder.buildSuperGet(field),
On 2015/03/18 10:42:56, karlklose wrote:
> Indentation is wrong.

Done.

https://codereview.chromium.org/1004683002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1165: List<ir.Primitive>
arguments = [indexValue];
On 2015/03/18 10:42:56, karlklose wrote:
> Add type arguments to list literal.

Done.

https://codereview.chromium.org/1004683002/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1200: List<ir.Primitive>
arguments = [visit(rhs)];
On 2015/03/18 10:42:56, karlklose wrote:
> Type argument for list literal.

Done.

[floitsch, 2015-03-22 23:23:05]

LGTM.

[Johnni Winther, 2015-03-23 08:04:16]

Committed patchset #5 (id:80001) manually as 44628 (presubmit successful).

[sra1, 2015-03-23 20:54:46]

sra@google.com changed reviewers:
+ sra@google.com

[sra1, 2015-03-23 20:54:47]

This CL seems to break

python tools/test.py --host-checked -cdart2dart -mrelease --dart2js-batch
--builder-tag new_backend -t60
co19/Language/12_Expressions/18_Assignment_A04_t09

[sra1, 2015-03-23 20:59:01]

On 2015/03/23 20:54:47, sra1 wrote:
> This CL seems to break
> 
> python tools/test.py --host-checked -cdart2dart -mrelease --dart2js-batch
> --builder-tag new_backend -t60
> co19/Language/12_Expressions/18_Assignment_A04_t09

Sorry, maybe the break is earlier.