Serialize static constant field references.

Serialize static constant field references.

R=paulberry@google.com
BUG=

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

[Paul Berry, 2016-01-26 20:52:50]

lgtm

https://codereview.chromium.org/1636113002/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/summarize_ast.dart (right):

https://codereview.chromium.org/1636113002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/summarize_ast.dart:67: throw new
StateError('Unexpected target type: ${target.runtimeType}');
Add a TODO comment here.  We're going to need to be able to handle this when it
comes time to summarize code outside the SDK, because target might not be an
identifier in a malformed constant.

https://codereview.chromium.org/1636113002/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/summarize_const_expr.dart (right):

https://codereview.chromium.org/1636113002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/summarize_const_expr.dart:97:
serialize(expr.target);
There's two ambiguities with `length` that I think we need to be careful about:

const x = a.b.length;
const y = c.length;

Either of these could either be UnlinkedConstOperation.length, or a reference to
a static const field called `length`, depending on the definitions of `a`, `b`,
and `c`.  (`y` could also be a reference to a top level variable called
`length`, if `c` is an import prefix).  Since we want to be able to summarize an
AST without consulting resolution information, we need to provisionally resolve
the ambiguity one way, and then fix it up during resynthesis.

If I'm reading your code right, it looks like `a.b.length` will be encoded using
UnlinkedConstOperation.length, but `c.length` will be encoded using
UnlinkedConstOperation.pushReference (because it will be interpreted as a
prefixed identifier).

Personally, I'd prefer to consistently serialize these cases using
UnlinkedConstOperation.pushReference, because I think the fix up will be easier
in that direction.

If you want to just put in a TODO comment for now and return to this ambiguous
case later, that's fine with me.

[scheglov, 2016-01-26 21:01:44]

Description was changed from

==========
Serialize static constant field references.

R=paulberry@google.com
BUG=
==========

to

==========
Serialize static constant field references.

R=paulberry@google.com
BUG=

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

[scheglov, 2016-01-26 21:01:46]

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

[scheglov, 2016-01-26 21:02:29]

https://codereview.chromium.org/1636113002/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/summarize_ast.dart (right):

https://codereview.chromium.org/1636113002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/summarize_ast.dart:67: throw new
StateError('Unexpected target type: ${target.runtimeType}');
On 2016/01/26 20:52:49, Paul Berry wrote:
> Add a TODO comment here.  We're going to need to be able to handle this when
it
> comes time to summarize code outside the SDK, because target might not be an
> identifier in a malformed constant.

Done.

https://codereview.chromium.org/1636113002/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/summarize_const_expr.dart (right):

https://codereview.chromium.org/1636113002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/summarize_const_expr.dart:97:
serialize(expr.target);
On 2016/01/26 20:52:49, Paul Berry wrote:
> There's two ambiguities with `length` that I think we need to be careful
about:
> 
> const x = a.b.length;
> const y = c.length;
> 
> Either of these could either be UnlinkedConstOperation.length, or a reference
to
> a static const field called `length`, depending on the definitions of `a`,
`b`,
> and `c`.  (`y` could also be a reference to a top level variable called
> `length`, if `c` is an import prefix).  Since we want to be able to summarize
an
> AST without consulting resolution information, we need to provisionally
resolve
> the ambiguity one way, and then fix it up during resynthesis.
> 
> If I'm reading your code right, it looks like `a.b.length` will be encoded
using
> UnlinkedConstOperation.length, but `c.length` will be encoded using
> UnlinkedConstOperation.pushReference (because it will be interpreted as a
> prefixed identifier).
> 
> Personally, I'd prefer to consistently serialize these cases using
> UnlinkedConstOperation.pushReference, because I think the fix up will be
easier
> in that direction.
> 
> If you want to just put in a TODO comment for now and return to this ambiguous
> case later, that's fine with me.

I added TODO and will fix it soon.