fixes #427, static fields emitted outside the scope of their class

fixes #427, static fields emitted outside the scope of their class

mainly, this backs off of some readability optimizations around static and top-level fields that were too aggressive. On their own, they were okay, but they collide with the library-cycle issues. Once we can remove that issue, we could consider restoring some of this. In the meantime, simplicity is good.

The new operation in the declaration loader is to allow us to see if an initializer has all its dependencies satisfied, but without changing any ordering if they are not.

R=vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/8ff0b180027043a258d565643d87db1ff3c23b64

[Jennifer Messerly, 2016-01-27 01:37:10]

Description was changed from

==========
fixes #427, static fields emitted outside the scope of their class

mainly, this backs off of some readability optimizations around static and
top-level fields that were too aggressive. On their own, they were okay, but
they collide with the library-cycle issues. Once we can remove that issue, we
could consider restoring some of this.
==========

to

==========
fixes #427, static fields emitted outside the scope of their class

mainly, this backs off of some readability optimizations around static and
top-level fields that were too aggressive. On their own, they were okay, but
they collide with the library-cycle issues. Once we can remove that issue, we
could consider restoring some of this. In the meantime, simplicity is good.
==========

[Jennifer Messerly, 2016-01-27 01:39:18]

Description was changed from

==========
fixes #427, static fields emitted outside the scope of their class

mainly, this backs off of some readability optimizations around static and
top-level fields that were too aggressive. On their own, they were okay, but
they collide with the library-cycle issues. Once we can remove that issue, we
could consider restoring some of this. In the meantime, simplicity is good.
==========

to

==========
fixes #427, static fields emitted outside the scope of their class

mainly, this backs off of some readability optimizations around static and
top-level fields that were too aggressive. On their own, they were okay, but
they collide with the library-cycle issues. Once we can remove that issue, we
could consider restoring some of this. In the meantime, simplicity is good.

The new operation in the declaration loader is to allow us to see if an
initializer has all its dependencies satisfied, but without changing any
ordering if they are not.
==========

[Jennifer Messerly, 2016-01-27 01:39:18]

jmesserly@google.com changed reviewers:
+ vsm@google.com

[vsm, 2016-01-27 14:29:30]

lgtm

https://codereview.chromium.org/1636233002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1636233002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:844: // they are made static.
"made lazy"?

https://codereview.chromium.org/1636233002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:2213: isLoaded && (field.isConst ||
_constField.isFieldInitConstant(field));
Could this be an '||' instead of an '&&'?  I.e., if all references are loaded,
can we eagerly initialize even if the field isn't constant?

[Jennifer Messerly, 2016-01-27 17:26:28]

Thanks! I've tried to clarify some of the comments.

https://codereview.chromium.org/1636233002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1636233002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:844: // they are made static.
On 2016/01/27 14:29:29, vsm wrote:
> "made lazy"?

good catch, done!

https://codereview.chromium.org/1636233002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:2213: isLoaded && (field.isConst ||
_constField.isFieldInitConstant(field));
On 2016/01/27 14:29:29, vsm wrote:
> Could this be an '||' instead of an '&&'?  I.e., if all references are loaded,
> can we eagerly initialize even if the field isn't constant?

The initializer must not cause visible side effects if we are to execute it
eagerly. We don't have a way of knowing side effects, so "const" is used as a
proxy.

I had the same thought though, at first. Probably it needs a comment. :)

[Jennifer Messerly, 2016-01-27 17:35:43]

Description was changed from

==========
fixes #427, static fields emitted outside the scope of their class

mainly, this backs off of some readability optimizations around static and
top-level fields that were too aggressive. On their own, they were okay, but
they collide with the library-cycle issues. Once we can remove that issue, we
could consider restoring some of this. In the meantime, simplicity is good.

The new operation in the declaration loader is to allow us to see if an
initializer has all its dependencies satisfied, but without changing any
ordering if they are not.
==========

to

==========
fixes #427, static fields emitted outside the scope of their class

mainly, this backs off of some readability optimizations around static and
top-level fields that were too aggressive. On their own, they were okay, but
they collide with the library-cycle issues. Once we can remove that issue, we
could consider restoring some of this. In the meantime, simplicity is good.

The new operation in the declaration loader is to allow us to see if an
initializer has all its dependencies satisfied, but without changing any
ordering if they are not.

R=vsm@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/8ff0b180027043a258d565643d87...
==========

[Jennifer Messerly, 2016-01-27 17:35:45]

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