Resolve initializers in their proper scope.

Resolve initializers in their proper scope.

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

[ahe, 2012-03-23 00:28:23]

Uploaded a new version, turned out that my changes to closure were bogus.

[floitsch, 2012-03-23 06:46:16]

LGTM.
The compile-time constant test should soon not be necessary anymore, but I can
very easily merge it (which in this case would mean 'remove it') when I commit.

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/resolv...
File dart/frog/leg/resolver.dart (right):

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/resolv...
dart/frog/leg/resolver.dart:194: initializerDo(tree, visitor.visit);
I would prefer keeping this function locally here (and duplicate in other
classes if necessary).
Also I would prefer 'visitPotentialInitializer'.

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/resolv...
dart/frog/leg/resolver.dart:201: new ResolverVisitor(compiler,
element.enclosingElement);
4 chars indent.

[ngeoffray, 2012-03-23 07:50:22]

LGTM

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/compil...
File dart/frog/leg/compile_time_constants.dart (right):

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/compil...
dart/frog/leg/compile_time_constants.dart:1004:
compiler.withCurrentElement(currentElement, () {
Why is that necessary? The withCurrentElement line 1024 should be enough.

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/resolv...
File dart/frog/leg/resolver.dart (right):

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/resolv...
dart/frog/leg/resolver.dart:51: return resolveVariableElement(element);
Should that be renamed to resolveField then?

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/resolv...
dart/frog/leg/resolver.dart:194: initializerDo(tree, visitor.visit);
On 2012/03/23 06:46:17, floitsch wrote:
> I would prefer keeping this function locally here (and duplicate in other
> classes if necessary).

+1

> Also I would prefer 'visitPotentialInitializer'.

visitPotentialInitializer -> handlePotentialInitializer

[ahe, 2012-03-23 11:30:31]

Hi Florian and Nicolas,

Thank you for your thoughts.

Cheers,
Peter

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/compil...
File dart/frog/leg/compile_time_constants.dart (right):

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/compil...
dart/frog/leg/compile_time_constants.dart:1004:
compiler.withCurrentElement(currentElement, () {
On 2012/03/23 07:50:22, ngeoffray wrote:
> Why is that necessary? The withCurrentElement line 1024 should be enough.

That's the wrong element. It is the element for the constructor that is being
inlined, not the element of the node.

This will get more ugly the more we inline. We need to find a better solution
than withCurrentElement.

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/resolv...
File dart/frog/leg/resolver.dart (right):

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/resolv...
dart/frog/leg/resolver.dart:51: return resolveVariableElement(element);
On 2012/03/23 07:50:22, ngeoffray wrote:
> Should that be renamed to resolveField then?

Done.

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/resolv...
dart/frog/leg/resolver.dart:194: initializerDo(tree, visitor.visit);
On 2012/03/23 07:50:22, ngeoffray wrote:
> On 2012/03/23 06:46:17, floitsch wrote:
> > I would prefer keeping this function locally here (and duplicate in other
> > classes if necessary).
> 
> +1
> 
> > Also I would prefer 'visitPotentialInitializer'.
> 
> visitPotentialInitializer -> handlePotentialInitializer

I would like to discuss this in person. I'll keep the current version but add a
TODO that we need to consider if this pattern is acceptable.

Perhaps this should be a method on Node. I'm trying to make certain operations
easier as they are repeated in many places.

https://chromiumcodereview.appspot.com/9808006/diff/5001/dart/frog/leg/resolv...
dart/frog/leg/resolver.dart:201: new ResolverVisitor(compiler,
element.enclosingElement);
On 2012/03/23 06:46:17, floitsch wrote:
> 4 chars indent.

Done.