Introduce code to generate summaries from an element model.

Introduce code to generate summaries from an element model.

R=brianwilkerson@google.com

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

[Paul Berry, 2015-11-10 23:50:22]

paulberry@google.com changed reviewers:
+ brianwilkerson@google.com, scheglov@google.com

[Paul Berry, 2015-11-10 23:50:22]

Sorry for the large CL.  This code is largely ported over from my experiments on
summaries earlier this year, so a lot of functionality is being added all at
once.

[Brian Wilkerson, 2015-11-11 15:26:10]

LGTM

https://codereview.chromium.org/1420053011/diff/40001/pkg/analyzer/lib/src/ge...
File pkg/analyzer/lib/src/generated/testing/test_type_provider.dart (right):

https://codereview.chromium.org/1420053011/diff/40001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/testing/test_type_provider.dart:214:
functionClassElement.constructors = <ConstructorElement>[];
I'm surprised that this is necessary. I thought we already initialized all of
the fields in elements. If not, I think we ought to. And if that isn't feasible,
wouldn't it be better to do the initialization in ElementFactory.classElement2?

(Also, we should use ConstructorElement.EMPTY_LIST rather than create a new list
for each element.)

https://codereview.chromium.org/1420053011/diff/40001/pkg/analyzer/lib/src/su...
File pkg/analyzer/lib/src/summary/summarize_elements.dart (right):

https://codereview.chromium.org/1420053011/diff/40001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/summarize_elements.dart:20: Object
serializeLibrary(
The class 'Object' is used a lot in this library. Is there any (reasonable) way
we could narrow the type any?

https://codereview.chromium.org/1420053011/diff/40001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/summarize_elements.dart:192: if (variable != null
&& !variable.isSynthetic) {
Under what conditions will the variable be non-synthetic when the getter is not
synthetic? (Do summaries support invalid code?)

[Paul Berry, 2015-11-11 19:12:22]

PTAL

https://codereview.chromium.org/1420053011/diff/40001/pkg/analyzer/lib/src/ge...
File pkg/analyzer/lib/src/generated/testing/test_type_provider.dart (right):

https://codereview.chromium.org/1420053011/diff/40001/pkg/analyzer/lib/src/ge...
pkg/analyzer/lib/src/generated/testing/test_type_provider.dart:214:
functionClassElement.constructors = <ConstructorElement>[];
On 2015/11/11 15:26:10, Brian Wilkerson wrote:
> I'm surprised that this is necessary. I thought we already initialized all of
> the fields in elements. If not, I think we ought to. And if that isn't
feasible,
> wouldn't it be better to do the initialization in
ElementFactory.classElement2?

What we currently do is: in the constructor for ClassElementImpl, we initialize
"_constructors" to null, and in the "constructors" getter, we assert that it's
not null, and then if we're running in unchecked mode, we replace null with
ConstructorElement.EMPTY_LIST.  I think that strikes a really good balance: it
means that if a bug is ever encountered during testing where we forget to set
_constructors, an assertion will fire, but if such a bug happens in production,
the code will defensively act like there are no constructors, avoiding an
exception.

So I'd rather not initialize _constructors to ConstructorElement.EMPTY_LIST in
the ClassElementImpl constructor, since that would make us lose the ability to
detect the bug during testing.

But I'm quite happy initializing _constructors in ElementFactory.classElement2.

> 
> (Also, we should use ConstructorElement.EMPTY_LIST rather than create a new
list
> for each element.)

As we discussed in person, I'd prefer to move toward using `const
<ConstructorElement>[]` rather than `ConstructorElement.EMPTY_LIST` just because
the former is more idiomatic Dart.  So that's what I'm doing for now.  If you
find that it really bothers you, we can discuss it more.

https://codereview.chromium.org/1420053011/diff/40001/pkg/analyzer/lib/src/su...
File pkg/analyzer/lib/src/summary/summarize_elements.dart (right):

https://codereview.chromium.org/1420053011/diff/40001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/summarize_elements.dart:20: Object
serializeLibrary(
On 2015/11/11 15:26:10, Brian Wilkerson wrote:
> The class 'Object' is used a lot in this library. Is there any (reasonable)
way
> we could narrow the type any?

Yeah, I'm not happy about that either.  I have an idea for a re-work that uses
the "Builder" classes in place of Object.  I'll try to do that in a follow-up
CL.

https://codereview.chromium.org/1420053011/diff/40001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/summarize_elements.dart:192: if (variable != null
&& !variable.isSynthetic) {
On 2015/11/11 15:26:10, Brian Wilkerson wrote:
> Under what conditions will the variable be non-synthetic when the getter is
not
> synthetic? (Do summaries support invalid code?)

Did you perhaps mean to ask "Under what conditions would both the variable and
the getter be synthetic"?  (After all, the function of `&&
!variable.isSynthetic` is to ensure that we don't summarize a variable if both
it and the getter are synthetic).

In which case, good catch.  It looks like this only could arise during tests,
due to a bug in ElementFactory.topLevelVariableElement3 (it was erroneously
marking both the variable and its getter as synthetic, and it wasn't populating
the variable's "type" field, causing my code to crash).  With
ElementFactory.topLevelVariableElement3 fixed, I don't need the `&&
!variable.isSynthetic` anymore.

I've left an assertion in place so that if we ever run across a bug causing both
the getter and the variable to be synthetic, we'll find out about it in testing,
but production code won't be slowed down.

[Brian Wilkerson, 2015-11-11 21:06:39]

LGTM

https://codereview.chromium.org/1420053011/diff/60001/pkg/analyzer/tool/summa...
File pkg/analyzer/tool/summary/idl_model.dart (right):

https://codereview.chromium.org/1420053011/diff/60001/pkg/analyzer/tool/summa...
pkg/analyzer/tool/summary/idl_model.dart:23: */
Misaligned comment. Needs formatting?

[Paul Berry, 2015-11-11 21:14:09]

https://codereview.chromium.org/1420053011/diff/60001/pkg/analyzer/tool/summa...
File pkg/analyzer/tool/summary/idl_model.dart (right):

https://codereview.chromium.org/1420053011/diff/60001/pkg/analyzer/tool/summa...
pkg/analyzer/tool/summary/idl_model.dart:23: */
On 2015/11/11 21:06:39, Brian Wilkerson wrote:
> Misaligned comment. Needs formatting?

Done.

[Paul Berry, 2015-11-11 21:18:08]

Committed patchset #4 (id:60001) manually as
74f09b99f130fc7ef8aac286b849e82f92d1f64d (presubmit successful).