Handle constants that mix eager and deferred types.

Handle constants that mix eager and deferred types.

R=ngeoffray@google.com

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

[ngeoffray, 2013-05-14 07:17:38]

LGTM

https://codereview.chromium.org/15096006/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/deferred_load.dart (right):

https://codereview.chromium.org/15096006/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/deferred_load.dart:167: //
constructor, not the class itself.
Update comment.

https://codereview.chromium.org/15096006/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/deferred_load.dart:193: ||
element.isGenerativeConstructorBody()) {
Are you really seeing generative constructor bodies? These should be constructed
by the backend, and I think the deferred loading phase happens before codegen.

https://codereview.chromium.org/15096006/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/15096006/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3044:
return deferredBuffer;
So you return the deferred buffer if one of its dependency is deferred? That's
because you know that if eager code references the constant, it will be an error
if the deferred code is not loaded, right? Please add a comment.

https://codereview.chromium.org/15096006/diff/1/dart/tests/lib/async/deferred...
File dart/tests/lib/async/deferred/deferred_constant_test.dart (right):

https://codereview.chromium.org/15096006/diff/1/dart/tests/lib/async/deferred...
dart/tests/lib/async/deferred/deferred_constant_test.dart:1: // Copyright (c)
2013, the Dart project authors.  Please see the AUTHORS file
Could you provide a test that fails too, when the eager code tries to access
some constant that is not loaded yet.

[ahe, 2013-05-14 10:01:24]

PTAL

https://codereview.chromium.org/15096006/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/deferred_load.dart (right):

https://codereview.chromium.org/15096006/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/deferred_load.dart:167: //
constructor, not the class itself.
On 2013/05/14 07:17:38, ngeoffray wrote:
> Update comment.

I extended the comment.  It is not wrong.

https://codereview.chromium.org/15096006/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/deferred_load.dart:193: ||
element.isGenerativeConstructorBody()) {
On 2013/05/14 07:17:38, ngeoffray wrote:
> Are you really seeing generative constructor bodies? These should be
constructed
> by the backend, and I think the deferred loading phase happens before codegen.

I don't think I saw generative constructor bodies.

https://codereview.chromium.org/15096006/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/15096006/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3044:
return deferredBuffer;
On 2013/05/14 07:17:38, ngeoffray wrote:
> So you return the deferred buffer if one of its dependency is deferred? That's
> because you know that if eager code references the constant, it will be an
error
> if the deferred code is not loaded, right? Please add a comment.

Done.

https://codereview.chromium.org/15096006/diff/1/dart/tests/lib/async/deferred...
File dart/tests/lib/async/deferred/deferred_constant_test.dart (right):

https://codereview.chromium.org/15096006/diff/1/dart/tests/lib/async/deferred...
dart/tests/lib/async/deferred/deferred_constant_test.dart:1: // Copyright (c)
2013, the Dart project authors.  Please see the AUTHORS file
On 2013/05/14 07:17:38, ngeoffray wrote:
> Could you provide a test that fails too, when the eager code tries to access
> some constant that is not loaded yet.

http://dartbug.com/10640

[ngeoffray, 2013-05-14 10:08:48]

Still LGTM

https://codereview.chromium.org/15096006/diff/7001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/15096006/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3035: *
constant before the deferred type has been loaded has will not
remove second 'has'.

[ahe, 2013-05-15 08:22:55]

Thank you, Nicolas!

https://codereview.chromium.org/15096006/diff/7001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/15096006/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3035: *
constant before the deferred type has been loaded has will not
On 2013/05/14 10:08:49, ngeoffray wrote:
> remove second 'has'.

Done.

[ahe, 2013-05-15 09:27:24]

Committed patchset #3 manually as r22727 (presubmit successful).