Do not apply optimizations based on an element if that element cannot be resolved.

Do not apply optimizations based on an element if that element cannot be resolved.
Committed: https://code.google.com/p/dart/source/detail?r=16906

[ahe, 2013-01-09 15:46:27]

Do you really need the new element and kind?

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:963: Element
findInScopeContainer(ScopeContainerElement scope, SourceString name) {
I really don't like the term "scope container", and you're definitely not
looking up in a scope.

So I'd prefer a different name for the method and the first argument.

Also, this method is a "lookup" not a "find" as it reports an error if the
element isn't found.

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:966:
internalError('Could not find ${name.slowToString()} in $scope');
Throw an exception instead?

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:124: static
const ElementKind UNRESOLVED =
Could you use ErroneousElement instead?

[Johnni Winther, 2013-01-10 07:09:55]

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:123: const
ElementKind('malformed', ElementCategory.NONE);
I don't think this kind is used anymore.

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:124: static
const ElementKind UNRESOLVED =
On 2013/01/09 15:46:27, ahe wrote:
> Could you use ErroneousElement instead?

ErroneousElement has a different semantics in the backend, namely that a runtime
error should be produced for it. Therefore we need something else.

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:443: * An
ambiguous element represents multiple elements accessible by the same name.
Long line.

[ahe, 2013-01-10 07:32:07]

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:124: static
const ElementKind UNRESOLVED =
On 2013/01/10 07:09:55, Johnni Winther wrote:
> On 2013/01/09 15:46:27, ahe wrote:
> > Could you use ErroneousElement instead?
> 
> ErroneousElement has a different semantics in the backend, namely that a
runtime
> error should be produced for it. Therefore we need something else.

I don't see why. The element is only used by the mock compiler to compare for
identity. We should not add new classes and categories that aren't actually used
in production code.

[Johnni Winther, 2013-01-10 08:19:47]

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:124: static
const ElementKind UNRESOLVED =
On 2013/01/10 07:32:07, ahe wrote:
> On 2013/01/10 07:09:55, Johnni Winther wrote:
> > On 2013/01/09 15:46:27, ahe wrote:
> > > Could you use ErroneousElement instead?
> > 
> > ErroneousElement has a different semantics in the backend, namely that a
> runtime
> > error should be produced for it. Therefore we need something else.
> 
> I don't see why. The element is only used by the mock compiler to compare for
> identity. We should not add new classes and categories that aren't actually
used
> in production code.

You are right. I thought it was used by production code.

[ngeoffray, 2013-01-10 09:41:48]

PTAL

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:963: Element
findInScopeContainer(ScopeContainerElement scope, SourceString name) {
On 2013/01/09 15:46:27, ahe wrote:
> I really don't like the term "scope container", and you're definitely not
> looking up in a scope.
> 
> So I'd prefer a different name for the method and the first argument.
> 
> Also, this method is a "lookup" not a "find" as it reports an error if the
> element isn't found.

Renamed to findElementIn.

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:966:
internalError('Could not find ${name.slowToString()} in $scope');
On 2013/01/09 15:46:27, ahe wrote:
> Throw an exception instead?

Done.

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:123: const
ElementKind('malformed', ElementCategory.NONE);
On 2013/01/10 07:09:55, Johnni Winther wrote:
> I don't think this kind is used anymore.

As discussed, I'll leave it to you or another CL.

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:124: static
const ElementKind UNRESOLVED =
On 2013/01/09 15:46:27, ahe wrote:
> Could you use ErroneousElement instead?

Yes, Done.

https://codereview.chromium.org/11817013/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:443: * An
ambiguous element represents multiple elements accessible by the same name.
On 2013/01/10 07:09:55, Johnni Winther wrote:
> Long line.

Done.

[ahe, 2013-01-10 09:44:58]

LGTM!

[Johnni Winther, 2013-01-10 09:46:11]

lgtm