Introduces the new syntax for deferred loading

Introduces the new syntax for deferred loading

import "lib.dart" deferred as lib;

And then injecting a function of the name "loadLibrary" into the
namespace of the import prefix.

This FunctionElement is a triggers the builder to emit special code that causes _loadLibraryWrapper in js_helper to be called.

Dart2dart will not crash on deferred loading, but calls to
loadLibrary will become calls to a (not-existing) top-level function
loadLibrary.

The old syntax (using metadata) is still working

For now keeping the language tests with the old syntax in addition to
the updated tests.

R=floitsch@google.com, johnniwinther@google.com

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

[karlklose, 2014-03-17 14:05:00]

DBC:

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1664: class
DeferredLoaderFunctionElementX extends FunctionElementX {
This element should have source code information.

When, for example, calling it with too many arguments, we may print a hint and
then we would need the source code location to point to.

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1666:
DeferredLoaderFunctionElementX(prefix)
`prefix` -> `this.prefix`.

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:467: if
(isConstructor) {
Combine the conditions.

https://codereview.chromium.org/201613006/diff/40001/tests/language/deferred_...
File tests/language/deferred_constraints_constants_old_syntax_test.dart (right):

https://codereview.chromium.org/201613006/diff/40001/tests/language/deferred_...
tests/language/deferred_constraints_constants_old_syntax_test.dart:11: const
lazy = const DeferredLibrary("lib");
Why this change?

https://codereview.chromium.org/201613006/diff/40001/tests/language/language....
File tests/language/language.status (right):

https://codereview.chromium.org/201613006/diff/40001/tests/language/language....
tests/language/language.status:63: deferred_no_prefix_test/01: Fail, Pass
These tests are flaky?

[floitsch, 2014-03-17 14:15:41]

FYI.

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

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:541: // Returns
suggestedName if it is not in usedNames. Otherwise concatenate
concatenates

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1032: Import
_import;
I don't really like this.

Every prefix comes from imports. Having a field that only works in special
circumstances feels wrong.
Could we have a list of imports, and a `singleImport` that throws if there is
more than one?

[Johnni Winther, 2014-03-18 12:29:13]

LGTM

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1032: Import
_import;
On 2014/03/17 14:15:41, floitsch wrote:
> I don't really like this.
> 
> Every prefix comes from imports. Having a field that only works in special
> circumstances feels wrong.
> Could we have a list of imports, and a `singleImport` that throws if there is
> more than one?

Or rename to `deferredImport` ?

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/library_loader.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/library_loader.dart:563:
prefixElement.markAsDeferred(import);
Add a TODO to make (a deferred) [import] an argument to the constructor of
PrefixElementX.

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/tree/nodes.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/tree/nodes.dart:1809:
{this.isDeferred})
You need to update the Unparser and the PrettyPrinter to handle [isDeferred] +
add test in the unparser_test unittest.

https://codereview.chromium.org/201613006/diff/40001/tests/language/language....
File tests/language/language.status (right):

https://codereview.chromium.org/201613006/diff/40001/tests/language/language....
tests/language/language.status:24: deferred_constraints_constants_test/*: Fail,
Pass
Why the `Fail, Pass`/`Fail, Ok` status instead of `Fail` ?

[sigurdm, 2014-03-19 12:46:29]

Besides addressing comments, I have changed the deferredLoader element to be a
getter - so now it is handled as a closure.

Also I made sure that the loadLibrary function actually shadows as it is
supposed to.

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

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:541: // Returns
suggestedName if it is not in usedNames. Otherwise concatenate
On 2014/03/17 14:15:41, floitsch wrote:
> concatenates

Done.

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1032: Import
_import;
On 2014/03/18 12:29:14, Johnni Winther wrote:
> On 2014/03/17 14:15:41, floitsch wrote:
> > I don't really like this.
> > 
> > Every prefix comes from imports. Having a field that only works in special
> > circumstances feels wrong.
> > Could we have a list of imports, and a `singleImport` that throws if there
is
> > more than one?
> 
> Or rename to `deferredImport` ?

Done.

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1664: class
DeferredLoaderFunctionElementX extends FunctionElementX {
On 2014/03/17 14:05:00, karlklose wrote:
> This element should have source code information.
> 
> When, for example, calling it with too many arguments, we may print a hint and
> then we would need the source code location to point to.

Done.

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1666:
DeferredLoaderFunctionElementX(prefix)
On 2014/03/17 14:05:00, karlklose wrote:
> `prefix` -> `this.prefix`.

Not fixed - we still need to pass on the prefix to super.

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:467: if
(isConstructor) {
On 2014/03/17 14:05:00, karlklose wrote:
> Combine the conditions.

I think you look at the wrong }.

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/tree/nodes.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/tree/nodes.dart:1809:
{this.isDeferred})
On 2014/03/18 12:29:14, Johnni Winther wrote:
> You need to update the Unparser and the PrettyPrinter to handle [isDeferred] +
> add test in the unparser_test unittest.

Done.

https://codereview.chromium.org/201613006/diff/40001/tests/language/deferred_...
File tests/language/deferred_constraints_constants_old_syntax_test.dart (right):

https://codereview.chromium.org/201613006/diff/40001/tests/language/deferred_...
tests/language/deferred_constraints_constants_old_syntax_test.dart:11: const
lazy = const DeferredLibrary("lib");
On 2014/03/17 14:05:00, karlklose wrote:
> Why this change?

I must have hand-reverted this line at some point.

https://codereview.chromium.org/201613006/diff/40001/tests/language/language....
File tests/language/language.status (right):

https://codereview.chromium.org/201613006/diff/40001/tests/language/language....
tests/language/language.status:24: deferred_constraints_constants_test/*: Fail,
Pass
On 2014/03/18 12:29:14, Johnni Winther wrote:
> Why the `Fail, Pass`/`Fail, Ok` status instead of `Fail` ?

The fail, pass is because some of the tests pass - meaning they have a compile
time error when one was expected, but it is not the right error they stumble
upon.

The Fail, Ok, is because we do not expect the vm to support the annotation based
syntax

https://codereview.chromium.org/201613006/diff/40001/tests/language/language....
tests/language/language.status:63: deferred_no_prefix_test/01: Fail, Pass
On 2014/03/17 14:05:00, karlklose wrote:
> These tests are flaky?

No - they are passing negative tests, but for the wrong reason.
Per Johnni's advice I removed these lines.

[floitsch, 2014-03-19 21:04:51]

LGTM, but wait for Johnni's ok.

Add test where loadLibrary is used as a getter.

https://codereview.chromium.org/201613006/diff/100001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/201613006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1039: Import get
import => _deferredImport;
That's not what I meant. The PrefixElement still has an "import" getter that
only works in special circumstances.
It's the public accessor I was concerned about.

https://codereview.chromium.org/201613006/diff/100001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/201613006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:2971: if
(element.isDeferredLoaderGetter()) {
Can't you handle this one like all the other foreigns?

element.isForeign should be true and then it should be handled in the
visitForeignSend.

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
File tests/language/deferred_load_library_wrong_args_test.dart (right):

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
tests/language/deferred_load_library_wrong_args_test.dart:5:
lib.loadLibrary(10); /// 01: continued
make the error-area smaller.

lib.loadLibrary(
  10  /// 01: runtime error
);

Keep the import line.

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
File tests/language/deferred_no_prefix_test.dart (right):

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
tests/language/deferred_no_prefix_test.dart:8: import
"deferred_constraints_lib2.dart" deferred; /// 01: compile-time error
make error-area smaller

import "deferred_constraints_lib2.dart"
deferred  /// 01: compile-time error
;

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
tests/language/deferred_no_prefix_test.dart:13: lazy.load().then((_) { /// 01:
continued
Remove these lines.

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
File tests/language/deferred_shadow_load_library_test.dart (right):

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
tests/language/deferred_shadow_load_library_test.dart:18:
Expect.isTrue(lib.trueVar);
check that loadLibrary is still shadowed once the library is loaded.

[Johnni Winther, 2014-03-20 08:42:56]

Still LGTM

https://codereview.chromium.org/201613006/diff/100001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/201613006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1039: Import get
import => _deferredImport;
On 2014/03/19 21:04:51, floitsch wrote:
> That's not what I meant. The PrefixElement still has an "import" getter that
> only works in special circumstances.
> It's the public accessor I was concerned about.

+1

[sigurdm, 2014-03-20 12:26:32]

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/library_loader.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/library_loader.dart:563:
prefixElement.markAsDeferred(import);
On 2014/03/18 12:29:14, Johnni Winther wrote:
> Add a TODO to make (a deferred) [import] an argument to the constructor of
> PrefixElementX.

Done.

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/201613006/diff/40001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:467: if
(isConstructor) {
On 2014/03/17 14:05:00, karlklose wrote:
> Combine the conditions.

As you suggested I added an assertion if it is not a constructor

https://codereview.chromium.org/201613006/diff/100001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/201613006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1039: Import get
import => _deferredImport;
On 2014/03/19 21:04:51, floitsch wrote:
> That's not what I meant. The PrefixElement still has an "import" getter that
> only works in special circumstances.
> It's the public accessor I was concerned about.

Of course - I must have been sleeping.

https://codereview.chromium.org/201613006/diff/100001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/201613006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:2971: if
(element.isDeferredLoaderGetter()) {
On 2014/03/19 21:04:51, floitsch wrote:
> Can't you handle this one like all the other foreigns?
> 
> element.isForeign should be true and then it should be handled in the
> visitForeignSend.

Other foreigns are only handled as calls (not in a getter context).

I created visitForeignGetter that is called from generateGetter.

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
File tests/language/deferred_load_library_wrong_args_test.dart (right):

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
tests/language/deferred_load_library_wrong_args_test.dart:5:
lib.loadLibrary(10); /// 01: continued
On 2014/03/19 21:04:51, floitsch wrote:
> make the error-area smaller.
> 
> lib.loadLibrary(
>   10  /// 01: runtime error
> );
> 
> Keep the import line.

Done.

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
File tests/language/deferred_no_prefix_test.dart (right):

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
tests/language/deferred_no_prefix_test.dart:8: import
"deferred_constraints_lib2.dart" deferred; /// 01: compile-time error
On 2014/03/19 21:04:51, floitsch wrote:
> make error-area smaller
> 
> import "deferred_constraints_lib2.dart"
> deferred  /// 01: compile-time error
> ;

Done.

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
tests/language/deferred_no_prefix_test.dart:13: lazy.load().then((_) { /// 01:
continued
On 2014/03/19 21:04:51, floitsch wrote:
> Remove these lines.

Done.

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
File tests/language/deferred_shadow_load_library_test.dart (right):

https://codereview.chromium.org/201613006/diff/100001/tests/language/deferred...
tests/language/deferred_shadow_load_library_test.dart:18:
Expect.isTrue(lib.trueVar);
On 2014/03/19 21:04:51, floitsch wrote:
> check that loadLibrary is still shadowed once the library is loaded.

Done.

[floitsch, 2014-03-20 12:33:21]

Almost LGTM.

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:753: Import get
deferred_import;
Is this getter even used?

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1039: Import get
deferred_import => _deferredImport;
deferredImport

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3581:
assert(invariant(node, element.isDeferredLoaderGetter()));
Don't just make this an assert.

[sigurdm, 2014-03-20 12:45:54]

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:753: Import get
deferred_import;
On 2014/03/20 12:33:22, floitsch wrote:
> Is this getter even used?
Yes - it is used for giving the right argument to the loadLibraryWrapper call
generated in the ssa builder.

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:1039: Import get
deferred_import => _deferredImport;
On 2014/03/20 12:33:22, floitsch wrote:
> deferredImport

Done.

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/201613006/diff/120001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3581:
assert(invariant(node, element.isDeferredLoaderGetter()));
On 2014/03/20 12:33:22, floitsch wrote:
> Don't just make this an assert.

Done.

[sigurdm, 2014-03-21 07:50:40]

Committed patchset #5 manually as r34230 (presubmit successful).