Allow closures inside lazy initializers.

Allow closures inside lazy initializers.

BUG:http://dartbug.com/3559.

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

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

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

[floitsch, 2012-09-07 15:51:31]

lazy_static2_test is not yet working. Some kind of resolution problem.

[kasperl, 2012-09-10 13:44:29]

Do you have any tests that show that this CL is a step in the right direction?
Are there parts of some existing tests that start working as a result of landing
this?

https://chromiumcodereview.appspot.com/10913133/diff/1/lib/compiler/implement...
File lib/compiler/implementation/closure.dart (right):

https://chromiumcodereview.appspot.com/10913133/diff/1/lib/compiler/implement...
lib/compiler/implementation/closure.dart:203: void translateFunction(Element
element, FunctionExpression node) {
So here you're ignoring the element you're passed completely? Maybe assert that
the element is the same as elements[node]?

https://chromiumcodereview.appspot.com/10913133/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10913133/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:184: // If there is an estimate of
the parameter types assume these types when
Long line. NYF.

[floitsch, 2012-09-10 19:16:07]

CL is not yet ready to go in. Other parts of the compiler need to be changed
first. Nicolas and I looked at the error and he will try to look at it.
Either Nicolas will then finish this CL or I will continue when I come back from
vacation.

[floitsch, 2012-10-09 16:06:44]

PTAL.

https://chromiumcodereview.appspot.com/10913133/diff/1/lib/compiler/implement...
File lib/compiler/implementation/closure.dart (right):

https://chromiumcodereview.appspot.com/10913133/diff/1/lib/compiler/implement...
lib/compiler/implementation/closure.dart:203: void translateFunction(Element
element, FunctionExpression node) {
On 2012/09/10 13:44:29, kasperl wrote:
> So here you're ignoring the element you're passed completely? Maybe assert
that
> the element is the same as elements[node]? 

Done.

https://chromiumcodereview.appspot.com/10913133/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10913133/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:184: // If there is an estimate of
the parameter types assume these types when
On 2012/09/10 13:44:29, kasperl wrote:
> Long line. NYF.
Has been fixed in the mean-time.

[kasperl, 2012-10-10 08:25:02]

LGTM.

https://chromiumcodereview.appspot.com/10913133/diff/12001/lib/compiler/imple...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10913133/diff/12001/lib/compiler/imple...
lib/compiler/implementation/ssa/builder.dart:385: builder.mapInferredType(
4 space indent.

[floitsch, 2012-10-10 09:15:27]

https://chromiumcodereview.appspot.com/10913133/diff/12001/lib/compiler/imple...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10913133/diff/12001/lib/compiler/imple...
lib/compiler/implementation/ssa/builder.dart:385: builder.mapInferredType(
On 2012/10/10 08:25:02, kasperl wrote:
> 4 space indent.

Done.

[floitsch, 2012-10-10 12:09:38]

Had to remove the assert in 'closure.dart', since it was not always true for
constructors. (That was also the reason we attach information to nodes and not
elements).
The closure-in-initializers work should clean this up.

[kasperl, 2012-10-10 12:15:34]

Still LGTM.

[floitsch, 2012-10-10 18:46:43]

PTAL.
I have added Peter since the latest changes (Patch-set 9) are touching the
resolver.

The problem: (field) initializers are resolved in the library-scope. However,
when we create a new closure we use the scope's element as enclosing element.
This would be the library-element. In almost all cases that's not a problem,
except when we try to find the compilation-unit (source-location) of the
closure. Then we would find the library-file and not the sourced file (as in the
test of Patch-set 10).

The easiest solution I found was to check if the scope.element is a library, and
if yes, use the enclosingElement instead of the scope.element. I believe that
this is correct, but I'm not sure if there is a better way.

[ahe, 2012-10-11 07:36:05]

Adding Johnni to take a look at the scope issues in the resolver.

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
File lib/compiler/implementation/elements/elements.dart (right):

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
lib/compiler/implementation/elements/elements.dart:249: if (this.isLibrary()) {
Perhaps this method should be overridden in LibraryElement.

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
lib/compiler/implementation/elements/elements.dart:254: while
(!element.isCompilationUnit()) {
This will ensure a null error if something is setup incorrectly. I like that.

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
File lib/compiler/implementation/resolver.dart (right):

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
lib/compiler/implementation/resolver.dart:1378: Element functionEnclosing =
scope.element;
How about moving this assignment above the if statement and use the variable in
the test.

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
lib/compiler/implementation/resolver.dart:1381: functionEnclosing =
enclosingElement;
This is not making any sense to me.

[floitsch, 2012-10-11 13:58:12]

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
File lib/compiler/implementation/elements/elements.dart (right):

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
lib/compiler/implementation/elements/elements.dart:249: if (this.isLibrary()) {
On 2012/10/11 07:36:05, ahe wrote:
> Perhaps this method should be overridden in LibraryElement.

Done.

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
File lib/compiler/implementation/resolver.dart (right):

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
lib/compiler/implementation/resolver.dart:1378: Element functionEnclosing =
scope.element;
On 2012/10/11 07:36:05, ahe wrote:
> How about moving this assignment above the if statement and use the variable
in
> the test.

Done.

http://codereview.chromium.org/10913133/diff/19004/lib/compiler/implementatio...
lib/compiler/implementation/resolver.dart:1381: functionEnclosing =
enclosingElement;
On 2012/10/11 07:36:05, ahe wrote:
> This is not making any sense to me.
Added comments and a TODO(ahe).