Add erroneous elements for function types and use them to allow unresolvable constructors to be han…

Add erroneous elements for function types and use them to allow unresolvable constructors to be handled in the backend.

This is the first step towards runtime error handling of unresolvable constructors, library prefixes and constructor names in new-expressions.

[ngeoffray, 2012-08-20 14:39:20]

LGTM

[ahe, 2012-08-20 15:06:47]

I really like where this is going, but it would be great if we can chat about
this approach. There is something fundamental in this, so it would be good to
make sure we all on the same page.

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

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:164: bool isValid() => true;
How about isErroneous instead (and reversed).

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:298: * [ErroneousElement]s
are used to mark something as unresolvable, but in
Try to keep the first line a summary of the class without referring to the
class' own name. Eventually, apidoc should be able to generate a summary of
classes that use the first paragraph like this:

ErroneousElement: Fist paragraph goes here.

With this documentation comment, the summary becomes:

ErroneousElement: [ErroneousElement]s are used to mark something as
unresolvable, but in contrast to using [null], there is additional information
about the error that caused the element to be unresolvable.

Notice how the name is repeated, and that the "summary" contains motivation.

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:303: * [isValid] will throw
an exception.
I don't think that is a good idea.

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:306: *  
[Element.isInvalid(element)]
[: Element.isInvalid(element) :]

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:308: *   [element == null].
[: element == null :]

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

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/resolver.dart:1463: * Try to resolve the constructor
that is referred to by this
"this" is not NewExpression, [node] is. So:

Try to resolve the constructor that is referred to by [node].

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/resolver.dart:1472: Type type =
resolveTypeRequired(annotation);
This looks bogus to me.

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/resolver.dart:2283: if (e !== null && e.isValid() &&
e.kind === ElementKind.CLASS) {
I don't like having to check for null and valid.

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

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:2409:
compiler.cancel('Unimplemented non-matching static call', node: node);
No really a non-matching static call.

[Johnni Winther, 2012-08-21 07:56:54]

lgtm

[karlklose, 2012-08-21 11:44:09]

Thanks for the reviews.

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

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:164: bool isValid() => true;
On 2012/08/20 15:06:47, ahe wrote:
> How about isErroneous instead (and reversed).

Done.

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:298: * [ErroneousElement]s
are used to mark something as unresolvable, but in
On 2012/08/20 15:06:47, ahe wrote:
> Try to keep the first line a summary of the class without referring to the
> class' own name. Eventually, apidoc should be able to generate a summary of
> classes that use the first paragraph like this:
> 
> ErroneousElement: Fist paragraph goes here.
> 
> With this documentation comment, the summary becomes:
> 
> ErroneousElement: [ErroneousElement]s are used to mark something as
> unresolvable, but in contrast to using [null], there is additional information
> about the error that caused the element to be unresolvable.
> 
> Notice how the name is repeated, and that the "summary" contains motivation.

Done.

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:303: * [isValid] will throw
an exception.
On 2012/08/20 15:06:47, ahe wrote:
I added a comment saying that we might some of these properties to also have
meaning for erroneous elements.

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:306: *  
[Element.isInvalid(element)]
On 2012/08/20 15:06:47, ahe wrote:
> [: Element.isInvalid(element) :]

Done.

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:308: *   [element == null].
On 2012/08/20 15:06:47, ahe wrote:
> [: element == null :]

Done.

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

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/resolver.dart:1463: * Try to resolve the constructor
that is referred to by this
On 2012/08/20 15:06:47, ahe wrote:
> "this" is not NewExpression, [node] is. So:
> 
> Try to resolve the constructor that is referred to by [node].

Done.

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/resolver.dart:1472: Type type =
resolveTypeRequired(annotation);
I added a TODO to clean this up.

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/resolver.dart:2283: if (e !== null && e.isValid() &&
e.kind === ElementKind.CLASS) {
On 2012/08/20 15:06:47, ahe wrote:
> I don't like having to check for null and valid.

Done, forgot to change that after introducing the static helper method on
Element.

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

https://chromiumcodereview.appspot.com/10829379/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:2409:
compiler.cancel('Unimplemented non-matching static call', node: node);
On 2012/08/20 15:06:47, ahe wrote:
> No really a non-matching static call.

Done.

[ahe, 2012-08-21 12:55:25]

LGTM!

https://chromiumcodereview.appspot.com/10829379/diff/1007/lib/compiler/implem...
File lib/compiler/implementation/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/10829379/diff/1007/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:295: * Represents an
unresolvable element.
I imagine this could also be used for duplicated elements.

https://chromiumcodereview.appspot.com/10829379/diff/1007/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:297: * [ErroneousElement]s
are used instead of [null] to provide additional
Perhaps more elegant:

An [ErroneousElement] is used ...

https://chromiumcodereview.appspot.com/10829379/diff/1007/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:305: * Code that does not
handle [ErroneousElement]s should use
Perhaps more elegant:

Code that cannot handle an [ErroneousElement] should ...

[karlklose, 2012-08-21 13:05:58]

Thanks, Peter.

https://chromiumcodereview.appspot.com/10829379/diff/1007/lib/compiler/implem...
File lib/compiler/implementation/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/10829379/diff/1007/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:295: * Represents an
unresolvable element.
On 2012/08/21 12:55:25, ahe wrote:
> I imagine this could also be used for duplicated elements.

Done.

https://chromiumcodereview.appspot.com/10829379/diff/1007/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:297: * [ErroneousElement]s
are used instead of [null] to provide additional
On 2012/08/21 12:55:25, ahe wrote:
> Perhaps more elegant:
> 
> An [ErroneousElement] is used ...

Done.

https://chromiumcodereview.appspot.com/10829379/diff/1007/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:305: * Code that does not
handle [ErroneousElement]s should use
On 2012/08/21 12:55:25, ahe wrote:
> Perhaps more elegant:
> 
> Code that cannot handle an [ErroneousElement] should ...

Done.