No globals.

No globals.

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

[floitsch, 2012-03-23 06:47:39]

LGTM.

[ngeoffray, 2012-03-23 07:57:02]

LGTM!

https://chromiumcodereview.appspot.com/9836022/diff/1/dart/frog/leg/ssa/build...
File dart/frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9836022/diff/1/dart/frog/leg/ssa/build...
dart/frog/leg/ssa/builder.dart:727: final Map<Node, ClosureData>
closureDataCache;
Since that guy is only used once in this file, I'd prefer avoiding a field.

[ahe, 2012-03-23 09:09:40]

Hi Florian and Nicolas,

Thank you for taking a look.

Cheers,
Peter

https://chromiumcodereview.appspot.com/9836022/diff/1/dart/frog/leg/ssa/build...
File dart/frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9836022/diff/1/dart/frog/leg/ssa/build...
dart/frog/leg/ssa/builder.dart:727: final Map<Node, ClosureData>
closureDataCache;
On 2012/03/23 07:57:02, ngeoffray wrote:
> Since that guy is only used once in this file, I'd prefer avoiding a field.

I don't understand your concern. I think the field makes it easier to see how
the closureDataCache is used.

I would refactor this code much more aggressively, but I don't want to interfere
with the rest of you.

[ngeoffray, 2012-03-23 09:16:28]

https://chromiumcodereview.appspot.com/9836022/diff/1/dart/frog/leg/ssa/build...
File dart/frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9836022/diff/1/dart/frog/leg/ssa/build...
dart/frog/leg/ssa/builder.dart:727: final Map<Node, ClosureData>
closureDataCache;
On 2012/03/23 09:09:40, ahe wrote:
> On 2012/03/23 07:57:02, ngeoffray wrote:
> > Since that guy is only used once in this file, I'd prefer avoiding a field.
> 
> I don't understand your concern. 

My concern is: I don't like introducing fields, IMO it makes reading the class
more difficult. And writing compiler.builder.closureDataCache looks as good to
me than closureDataCache, especially when it's only used once.

> I think the field makes it easier to see how
> the closureDataCache is used.
> 
> I would refactor this code much more aggressively, but I don't want to
interfere
> with the rest of you.

[ahe, 2012-03-23 09:35:48]

https://chromiumcodereview.appspot.com/9836022/diff/1/dart/frog/leg/ssa/build...
File dart/frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9836022/diff/1/dart/frog/leg/ssa/build...
dart/frog/leg/ssa/builder.dart:727: final Map<Node, ClosureData>
closureDataCache;
On 2012/03/23 09:16:28, ngeoffray wrote:
> On 2012/03/23 09:09:40, ahe wrote:
> > On 2012/03/23 07:57:02, ngeoffray wrote:
> > > Since that guy is only used once in this file, I'd prefer avoiding a
field.
> > 
> > I don't understand your concern. 
> 
> My concern is: I don't like introducing fields, IMO it makes reading the class
> more difficult. And writing compiler.builder.closureDataCache looks as good to
> me than closureDataCache, especially when it's only used once.

Perhaps I should have said: I assume you want me to write
compiler.builder.closureDataCache, I don't understand why you would want that as
it makes it less obvious that this class is a user of closureDataCache.

I think it would better if this class referenced its task directly, instead of
compiler, but I'm not going to make this "big" refactoring as I don't want to
create too many conflicts at this point.

I have bigger ideas for where we could take Task and Compiler, but I would
prefer to discuss this in person.

> > I think the field makes it easier to see how
> > the closureDataCache is used.
> > 
> > I would refactor this code much more aggressively, but I don't want to
> interfere
> > with the rest of you.
>

[ahe, 2012-03-23 10:43:26]

OMG my eyes bleed :-)

Hopefully we can clean this up later.

[floitsch, 2012-03-23 17:47:25]

LGTM.
yes we/I intend to clean this by (eventually) moving the closure translation
after the builder phase.