Throw NSM on unresolved factory redirection.

Throw NSM on unresolved factory redirection.

R=paulberry@google.com

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

[ahe, 2017-06-11 13:11:54]

ahe@google.com changed reviewers:
+ johnniwinther@google.com, paulberry@google.com

[Paul Berry, 2017-06-12 20:55:41]

lgtm.  Your call on whether to address the two minor nits below.

https://codereview.chromium.org/2933733002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart (right):

https://codereview.chromium.org/2933733002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart:23:
RedirectingFactoryBody.internal(Expression value)
Nit: I prefer to name internal constructors like this one with a leading "_". 
It avoids bugs by reducing the risk that someone will call it accidentally.  It
also reduces cognitive load because it lets a reviewer know that there are no
callers outside this file.

https://codereview.chromium.org/2933733002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart:61: new
RedirectingFactoryBody.internal(getValue(statement.expression))
Nit: I think we should report an internalError if the value returned by
getValue() is `null`, because that indicates that we've read corrupted data from
the Dill file.

[ahe, 2017-06-13 07:11:41]

Thank you, Paul!

https://codereview.chromium.org/2933733002/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart (right):

https://codereview.chromium.org/2933733002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart:23:
RedirectingFactoryBody.internal(Expression value)
On 2017/06/12 20:55:41, Paul Berry wrote:
> Nit: I prefer to name internal constructors like this one with a leading "_". 
> It avoids bugs by reducing the risk that someone will call it accidentally. 
It
> also reduces cognitive load because it lets a reviewer know that there are no
> callers outside this file.

Acknowledged.

https://codereview.chromium.org/2933733002/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/redirecting_factory_body.dart:61: new
RedirectingFactoryBody.internal(getValue(statement.expression))
On 2017/06/12 20:55:41, Paul Berry wrote:
> Nit: I think we should report an internalError if the value returned by
> getValue() is `null`, because that indicates that we've read corrupted data
from
> the Dill file.

I think I want to iterate more on this. I'll follow up with another CL.

[ahe, 2017-06-13 07:12:25]

Description was changed from

==========
Throw NSM on unresolved factory redirection.
==========

to

==========
Throw NSM on unresolved factory redirection.

R=paulberry@google.com

Committed:
https://github.com/dart-lang/sdk/commit/f559a6d1e99a1301ae507705e34caeb244f658cc
==========

[ahe, 2017-06-13 07:12:27]

Committed patchset #1 (id:1) manually as
f559a6d1e99a1301ae507705e34caeb244f658cc (presubmit successful).