Put IR builder visitors in a different library than IrBuilder.

Put IR builder visitors in a different library than IrBuilder.

BUG=
R=asgerf@google.com, kmillikin@google.com

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

[Johnni Winther, 2015-02-11 10:24:21]

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

[asgerf, 2015-02-11 10:51:14]

LGTM but CC'ing kevin in case he wants to have a look.

I suspect this will cause rebase conflicts for anyone about to land anything in
cps_ir_visitor?

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

https://codereview.chromium.org/917663003/diff/1/pkg/compiler/lib/src/cps_ir/...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:1804: //
[JsIrBuilderVisitor.loadArguments].
I'd say ditch the TODO and just document what the method does. Here is a doc
comment you can use:

/// Creates a box for [scope.box] and binds the captured variables to
/// that box.
///
/// The captured variables can subsequently be manipulated with
/// [declareLocalVariable], [buildLocalGet], and [buildLocalSet].

https://codereview.chromium.org/917663003/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/917663003/diff/1/pkg/compiler/lib/src/cps_ir/...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:5: library
dart2js.ir_builder_visitor;
Why rename the file to cps_ir_builder_task and then call the library _visitor?

What was wrong with the _visitor suffix?

[Johnni Winther, 2015-02-11 11:41:58]

https://codereview.chromium.org/917663003/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/917663003/diff/1/pkg/compiler/lib/src/cps_ir/...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:5: library
dart2js.ir_builder_visitor;
On 2015/02/11 10:51:14, asgerf wrote:
> Why rename the file to cps_ir_builder_task and then call the library _visitor?
> 
> What was wrong with the _visitor suffix?

I started with the name _visitor but realized that IrBuilderTask is what is used
from the outside. The library name should be in alignment with that.

[Johnni Winther, 2015-03-05 11:05:53]

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

[Johnni Winther, 2015-03-05 11:05:54]

@kmillikin: PTAL

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

https://codereview.chromium.org/917663003/diff/1/pkg/compiler/lib/src/cps_ir/...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:1804: //
[JsIrBuilderVisitor.loadArguments].
On 2015/02/11 10:51:14, asgerf wrote:
> I'd say ditch the TODO and just document what the method does. Here is a doc
> comment you can use:
> 
> /// Creates a box for [scope.box] and binds the captured variables to
> /// that box.
> ///
> /// The captured variables can subsequently be manipulated with
> /// [declareLocalVariable], [buildLocalGet], and [buildLocalSet].

Done.

https://codereview.chromium.org/917663003/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/917663003/diff/1/pkg/compiler/lib/src/cps_ir/...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:5: library
dart2js.ir_builder_visitor;
On 2015/02/11 11:41:58, Johnni Winther wrote:
> On 2015/02/11 10:51:14, asgerf wrote:
> > Why rename the file to cps_ir_builder_task and then call the library
_visitor?
> > 
> > What was wrong with the _visitor suffix?
> 
> I started with the name _visitor but realized that IrBuilderTask is what is
used
> from the outside. The library name should be in alignment with that.

Done.

[Kevin Millikin (Google), 2015-03-05 11:15:12]

LGTM.

[Johnni Winther, 2015-03-05 11:33:00]

Committed patchset #4 (id:60001) manually as 44258 (presubmit successful).