Insert checks before deferred calls and accesses.

Insert checks before deferred calls and accesses.
These checks will throw an error if the library was not loaded.

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

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

Was reverted by Issue 247863005 r35308 due to failing co19-test

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

[Johnni Winther, 2014-04-11 07:34:54]

lgtm

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

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:827: if (first is!
ast.Identifier) return null;
Change to:
astNode identifier = first.asIdentifier();
if (identifier == null) return null;
Element maybePrefix = elements[identifier];

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:829: if
(maybePrefix != null && maybePrefix.isPrefix() && (maybePrefix as
Change to:
if (maybePrefix != null && maybePrefix.isPrefix()) {
  PrefixElement prefixElement = maybePrefix
  if (prefixElement.isDeferred) {
    return prefixElement;
  }
}

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

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:836:
AbstractFieldElement abstractField;
This should be a getter.

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

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:545: void
addAccessor(FunctionElement accessor,
Change to [FunctionElementX].

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:1176:
.inSameOutputUnit(element,compiler.currentElement)) return false;
Add { } around `return false;`.

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:2932: if
(elements[node] != null) {
Is this needed or just an optimization?

https://codereview.chromium.org/232563006/diff/60001/tests/compiler/dart2js_e...
File tests/compiler/dart2js_extra/deferred/deferred_function_test.dart (right):

https://codereview.chromium.org/232563006/diff/60001/tests/compiler/dart2js_e...
tests/compiler/dart2js_extra/deferred/deferred_function_test.dart:21: //
function closures.  We think they are never null.
Is this still a problem?

https://codereview.chromium.org/232563006/diff/60001/tests/lib/async/deferred...
File tests/lib/async/deferred/deferred_api_test.dart (right):

https://codereview.chromium.org/232563006/diff/60001/tests/lib/async/deferred...
tests/lib/async/deferred/deferred_api_test.dart:23: lazy.load().then((_) {
Why this change?

[sigurdm, 2014-04-11 09:11:37]

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

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:827: if (first is!
ast.Identifier) return null;
On 2014/04/11 07:34:54, Johnni Winther wrote:
> Change to:
> astNode identifier = first.asIdentifier();
> if (identifier == null) return null;
> Element maybePrefix = elements[identifier];

Done.

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:829: if
(maybePrefix != null && maybePrefix.isPrefix() && (maybePrefix as
On 2014/04/11 07:34:54, Johnni Winther wrote:
> Change to:
> if (maybePrefix != null && maybePrefix.isPrefix()) {
>   PrefixElement prefixElement = maybePrefix
>   if (prefixElement.isDeferred) {
>     return prefixElement;
>   }
> }

Done.

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

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:836:
AbstractFieldElement abstractField;
On 2014/04/11 07:34:54, Johnni Winther wrote:
> This should be a getter.

Done.

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

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:545: void
addAccessor(FunctionElement accessor,
On 2014/04/11 07:34:54, Johnni Winther wrote:
> Change to [FunctionElementX].

Done.

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:1176:
.inSameOutputUnit(element,compiler.currentElement)) return false;
On 2014/04/11 07:34:54, Johnni Winther wrote:
> Add { } around `return false;`.

Done.

https://codereview.chromium.org/232563006/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:2932: if
(elements[node] != null) {
On 2014/04/11 07:34:54, Johnni Winther wrote:
> Is this needed or just an optimization?

Not needed - I think it was a temporary fix I forgot to remove.

https://codereview.chromium.org/232563006/diff/60001/tests/compiler/dart2js_e...
File tests/compiler/dart2js_extra/deferred/deferred_function_test.dart (right):

https://codereview.chromium.org/232563006/diff/60001/tests/compiler/dart2js_e...
tests/compiler/dart2js_extra/deferred/deferred_function_test.dart:21: //
function closures.  We think they are never null.
On 2014/04/11 07:34:54, Johnni Winther wrote:
> Is this still a problem?
I think not - I removed the hack, and the test still runs.

https://codereview.chromium.org/232563006/diff/60001/tests/lib/async/deferred...
File tests/lib/async/deferred/deferred_api_test.dart (right):

https://codereview.chromium.org/232563006/diff/60001/tests/lib/async/deferred...
tests/lib/async/deferred/deferred_api_test.dart:23: lazy.load().then((_) {
On 2014/04/11 07:34:54, Johnni Winther wrote:
> Why this change?

I removed the bool from the future in a previous CL. Why it has not been
triggered before is a mystery.

[floitsch, 2014-04-14 12:04:08]

LGTM.

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

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:21: import
'js_backend/js_backend.dart' show
We should eventually split this file into a front-end and back-end specific
part. Different CL...

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:836: if (element !=
null && element.isDeferredLoaderGetter()) return null;
What happens if I write `lib.loadLibrary.toString()`
Does that return `null`? Should it?

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:837: ast.Node
firstNode(ast.Node node) {
New line before nested function.

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

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:2933: void
generateIsDeferredLoadedCheckIfNeeded(ast.Send node){
space before the trailing "{".

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:2934: var
deferredTask = compiler.deferredLoadTask;
use type.

https://codereview.chromium.org/232563006/diff/100001/tests/compiler/dart2js_...
File tests/compiler/dart2js_extra/deferred/deferred_function_test.dart (right):

https://codereview.chromium.org/232563006/diff/100001/tests/compiler/dart2js_...
tests/compiler/dart2js_extra/deferred/deferred_function_test.dart:17: isError(e)
=> e is Error;
Can we be more specific?

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

https://codereview.chromium.org/232563006/diff/100001/tests/language/deferred...
tests/language/deferred_constraints_type_annotation_old_syntax_test.dart:21: },
(e) => e is Error); /// new_before_load: continued
can we be more specific?

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

https://codereview.chromium.org/232563006/diff/100001/tests/language/deferred...
tests/language/deferred_not_loaded_check_test.dart:33: print("Calling toplevel
${lib.foo(sideEffect())}");
Remove the prints.
Don't hide the code that is actually tested in a string-literal.

https://codereview.chromium.org/232563006/diff/100001/tests/language/language...
File tests/language/language.status (right):

https://codereview.chromium.org/232563006/diff/100001/tests/language/language...
tests/language/language.status:36: deferred_load_library_wrong_args_test/none:
Fail
Move issue number to the right of the test.

https://codereview.chromium.org/232563006/diff/100001/tests/language/language...
tests/language/language.status:38: deferred_not_loaded_check_test: Fail
issue number.

[floitsch, 2014-04-14 13:14:38]

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

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:831: /// Returns
the prefixElement associated with the first prefix of [send] in
Please improve the documentation. Hard to understand what this really does.

[sigurdm, 2014-04-22 11:22:48]

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

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:21: import
'js_backend/js_backend.dart' show
On 2014/04/14 12:04:08, floitsch wrote:
> We should eventually split this file into a front-end and back-end specific
> part. Different CL...

Good idea.

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:831: /// Returns
the prefixElement associated with the first prefix of [send] in
On 2014/04/14 13:14:39, floitsch wrote:
> Please improve the documentation. Hard to understand what this really does.

You are right - it is not really describing the function well.
I have tried to make it more clear.

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:836: if (element !=
null && element.isDeferredLoaderGetter()) return null;
On 2014/04/14 12:04:08, floitsch wrote:
> What happens if I write `lib.loadLibrary.toString()`
> Does that return `null`? Should it?

I think it should return null: lib.loadLibrary is not deferred and therefore
lib.loadLibrary.toString() is not deferred.

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:837: ast.Node
firstNode(ast.Node node) {
On 2014/04/14 12:04:08, floitsch wrote:
> New line before nested function.

Done.

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

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:2933: void
generateIsDeferredLoadedCheckIfNeeded(ast.Send node){
On 2014/04/14 12:04:08, floitsch wrote:
> space before the trailing "{".

Done.

https://codereview.chromium.org/232563006/diff/100001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:2934: var
deferredTask = compiler.deferredLoadTask;
On 2014/04/14 12:04:08, floitsch wrote:
> use type.

Done.

https://codereview.chromium.org/232563006/diff/100001/tests/compiler/dart2js_...
File tests/compiler/dart2js_extra/deferred/deferred_function_test.dart (right):

https://codereview.chromium.org/232563006/diff/100001/tests/compiler/dart2js_...
tests/compiler/dart2js_extra/deferred/deferred_function_test.dart:17: isError(e)
=> e is Error;
On 2014/04/14 12:04:08, floitsch wrote:
> Can we be more specific?

I leave it for now in this CL - and then we try to ask Gilad if we can agree on
which error it should be.

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

https://codereview.chromium.org/232563006/diff/100001/tests/language/deferred...
tests/language/deferred_constraints_type_annotation_old_syntax_test.dart:21: },
(e) => e is Error); /// new_before_load: continued
On 2014/04/14 12:04:08, floitsch wrote:
> can we be more specific?

Ditto.

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

https://codereview.chromium.org/232563006/diff/100001/tests/language/deferred...
tests/language/deferred_not_loaded_check_test.dart:33: print("Calling toplevel
${lib.foo(sideEffect())}");
On 2014/04/14 12:04:08, floitsch wrote:
> Remove the prints.
> Don't hide the code that is actually tested in a string-literal.

Done.

https://codereview.chromium.org/232563006/diff/100001/tests/language/language...
File tests/language/language.status (right):

https://codereview.chromium.org/232563006/diff/100001/tests/language/language...
tests/language/language.status:36: deferred_load_library_wrong_args_test/none:
Fail
On 2014/04/14 12:04:08, floitsch wrote:
> Move issue number to the right of the test.

Done.

https://codereview.chromium.org/232563006/diff/100001/tests/language/language...
tests/language/language.status:38: deferred_not_loaded_check_test: Fail
On 2014/04/14 12:04:08, floitsch wrote:
> issue number.

Done.

[floitsch, 2014-04-22 12:33:44]

LGTM.

https://codereview.chromium.org/232563006/diff/140001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/deferred_load.dart (right):

https://codereview.chromium.org/232563006/diff/140001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:834: ///
Precondition: send must be static.
New line before Precondition.

[sigurdm, 2014-04-23 07:56:16]

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

[sigurdm, 2014-04-23 14:01:45]

https://codereview.chromium.org/232563006/diff/140001/sdk/lib/_internal/compi...
File sdk/lib/_internal/compiler/implementation/deferred_load.dart (right):

https://codereview.chromium.org/232563006/diff/140001/sdk/lib/_internal/compi...
sdk/lib/_internal/compiler/implementation/deferred_load.dart:834: ///
Precondition: send must be static.
On 2014/04/22 12:33:44, floitsch wrote:
> New line before Precondition.

Done.

[sigurdm, 2014-04-24 07:51:54]

Committed patchset #7 manually as r35355 (presubmit successful).