Compute NewStructure in resolution.

Compute NewStructure in resolution.

Closes #21134

R=sigurdm@google.com

Committed: https://github.com/dart-lang/sdk/commit/9cfba28da17a951c7f2da5d810ff8be7f8004b80

[Johnni Winther, 2015-11-10 13:42:19]

johnniwinther@google.com changed reviewers:
+ sigurdm@google.com

[Johnni Winther, 2015-11-10 13:42:20]

Main changes in members.dart and constructors.dart

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/constructors.dart (right):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:772: final PrefixElement
prefix;
This prepares for handling of deferred access which itself didnt' make it into
this CL.

[sigurdm, 2015-11-11 08:24:52]

LGTM

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/el...
File pkg/compiler/lib/src/elements/elements.dart (right):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/el...
pkg/compiler/lib/src/elements/elements.dart:1266: /// Is `true` if this
constructor is malformed.
/// Is `true` if the effective target of this constructor is malformed.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/constructors.dart (right):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:768: NON_CONSTANT,
Add comment to explain what NON_CONSTANT means? Is it an error-case it seems to
be somehow orthogonal to the other kinds.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:771: class ConstructorResult {
Add dartdoc

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:772: final PrefixElement
prefix;
On 2015/11/10 13:42:20, Johnni Winther wrote:
> This prepares for handling of deferred access which itself didnt' make it into
> this CL.

Would it make sense to call it deferredPrefix and only pass it when the prefix
is deferred?

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:774: final Element element;
Could this be a `ConstructorElement`?

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:782:
ConstructorResult.forElement(this.element)
Add dartdoc

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:787:
ConstructorResult.forType(this.prefix, this.type)
add dartdoc

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:821: String constructorName) {
Not this CL.

This could take a Name instead of a String and a Library

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/members.dart (right):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/members.dart:3940: return new
ResolutionResult.forElement(constructor);
It is a bit confusing to have returns and breaks in the same switch. Especially
when there is a lot more processing following the switch.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/members.dart:3999: if (result.prefix != null &&
result.prefix.isDeferred) {
I think you can use `result.isDeferred` here.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/members.dart:4028: // Might be valid but we
don't know for sure.
Write where/when this is resolved.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/send_structure.dart (left):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/send_structure.dart:2678: case
ConstructorAccessKind.ERRONEOUS_REDIRECTING_FACTORY:
Where did this go?

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/send_structure.dart (right):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/send_structure.dart:2667: if
(callStructure.signatureApplies(effectiveTarget.functionSignature)) {
Long line

[Johnni Winther, 2015-11-11 09:56:30]

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/el...
File pkg/compiler/lib/src/elements/elements.dart (right):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/el...
pkg/compiler/lib/src/elements/elements.dart:1266: /// Is `true` if this
constructor is malformed.
On 2015/11/11 08:24:51, sigurdm wrote:
> /// Is `true` if the effective target of this constructor is malformed.

Done.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/constructors.dart (right):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:768: NON_CONSTANT,
On 2015/11/11 08:24:52, sigurdm wrote:
> Add comment to explain what NON_CONSTANT means? Is it an error-case it seems
to
> be somehow orthogonal to the other kinds.

Done.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:771: class ConstructorResult {
On 2015/11/11 08:24:52, sigurdm wrote:
> Add dartdoc

Done.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:772: final PrefixElement
prefix;
On 2015/11/11 08:24:51, sigurdm wrote:
> On 2015/11/10 13:42:20, Johnni Winther wrote:
> > This prepares for handling of deferred access which itself didnt' make it
into
> > this CL.
> 
> Would it make sense to call it deferredPrefix and only pass it when the prefix
> is deferred?

I'd rather have receivers test `result.isDeferred` than `result.deferredPrefix
!= null`.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:782:
ConstructorResult.forElement(this.element)
On 2015/11/11 08:24:52, sigurdm wrote:
> Add dartdoc

Done.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:787:
ConstructorResult.forType(this.prefix, this.type)
On 2015/11/11 08:24:52, sigurdm wrote:
> add dartdoc

Done.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/constructors.dart:821: String constructorName) {
On 2015/11/11 08:24:52, sigurdm wrote:
> Not this CL.
> 
> This could take a Name instead of a String and a Library

Acknowledged.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/members.dart (right):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/members.dart:3920:
registry.registerFeature(Feature.THROW_NO_SUCH_METHOD);
Why not 'isInvalid'? Added a comment.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/members.dart:3940: return new
ResolutionResult.forElement(constructor);
On 2015/11/11 08:24:52, sigurdm wrote:
> It is a bit confusing to have returns and breaks in the same switch.
Especially
> when there is a lot more processing following the switch.

Ack. Added a TODO.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/members.dart:3999: if (result.prefix != null &&
result.prefix.isDeferred) {
On 2015/11/11 08:24:52, sigurdm wrote:
> I think you can use `result.isDeferred` here.

Done.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/members.dart:4028: // Might be valid but we
don't know for sure.
On 2015/11/11 08:24:52, sigurdm wrote:
> Write where/when this is resolved.

Done.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/send_structure.dart (left):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/send_structure.dart:2678: case
ConstructorAccessKind.ERRONEOUS_REDIRECTING_FACTORY:
On 2015/11/11 08:24:52, sigurdm wrote:
> Where did this go?

Inside the FACTORY case.

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
File pkg/compiler/lib/src/resolution/send_structure.dart (right):

https://codereview.chromium.org/1437463005/diff/40001/pkg/compiler/lib/src/re...
pkg/compiler/lib/src/resolution/send_structure.dart:2667: if
(callStructure.signatureApplies(effectiveTarget.functionSignature)) {
On 2015/11/11 08:24:52, sigurdm wrote:
> Long line

Done.

[Johnni Winther, 2015-11-11 10:38:05]

Description was changed from

==========
Compute NewStructure in resolution.

BUG=
==========

to

==========
Compute NewStructure in resolution.

Closes #21134
==========

[Johnni Winther, 2015-11-11 10:46:17]

Committed patchset #4 (id:60001) manually as
9cfba28da17a951c7f2da5d810ff8be7f8004b80 (presubmit successful).