Resynthesize constructors lazily.

Resynthesize constructors lazily.

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

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

[Brian Wilkerson, 2016-06-03 16:06:34]

lgtm

https://codereview.chromium.org/2030403002/diff/1/pkg/analyzer/lib/src/dart/e...
File pkg/analyzer/lib/src/dart/element/element.dart (right):

https://codereview.chromium.org/2030403002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:549: return _constructors ??
const <ConstructorElement>[];
This doesn't make sense. If _constructors is not null why have the null guard?

[scheglov, 2016-06-03 16:10:38]

https://codereview.chromium.org/2030403002/diff/1/pkg/analyzer/lib/src/dart/e...
File pkg/analyzer/lib/src/dart/element/element.dart (right):

https://codereview.chromium.org/2030403002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:549: return _constructors ??
const <ConstructorElement>[];
On 2016/06/03 16:06:34, Brian Wilkerson wrote:
> This doesn't make sense. If _constructors is not null why have the null guard?

I agree, but this change just keeps the previous semantics.
I guess the idea was that at development time assert will fail.
But for clients we will silently return an empty list.
I don't have strong opinion whether we need this.

[Brian Wilkerson, 2016-06-03 16:18:15]

https://codereview.chromium.org/2030403002/diff/1/pkg/analyzer/lib/src/dart/e...
File pkg/analyzer/lib/src/dart/element/element.dart (right):

https://codereview.chromium.org/2030403002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:549: return _constructors ??
const <ConstructorElement>[];
Ok, I see. Are we doing the same everywhere that we're lazily populating a field
in the element model? Seems like we ought to be consistent.

[Paul Berry, 2016-06-03 16:21:50]

lgtm

https://codereview.chromium.org/2030403002/diff/1/pkg/analyzer/lib/src/dart/e...
File pkg/analyzer/lib/src/dart/element/element.dart (right):

https://codereview.chromium.org/2030403002/diff/1/pkg/analyzer/lib/src/dart/e...
pkg/analyzer/lib/src/dart/element/element.dart:549: return _constructors ??
const <ConstructorElement>[];
On 2016/06/03 16:10:38, scheglov wrote:
> On 2016/06/03 16:06:34, Brian Wilkerson wrote:
> > This doesn't make sense. If _constructors is not null why have the null
guard?
> 
> I agree, but this change just keeps the previous semantics.
> I guess the idea was that at development time assert will fail.
> But for clients we will silently return an empty list.
> I don't have strong opinion whether we need this.

Yes, that was my intention when I wrote this code, and I believe we should keep
it this way.  Consider the alternatives:

1. Defensive--hides bugs and makes it unclear to the reader whether
_constructors is intended to be nullable:

    return _constructors ?? const<ConstructorElement>[];

2. Fail fast--finds bugs, but in the event that a bug slips out to production,
is more likely to manifest in an unrecoverable crash:

    assert(_constructors != null);
    return _constructors;

To get both the defensiveness and the ability to find bugs quickly we need both
the assert and the null guard.

[scheglov, 2016-06-03 16:38:27]

Description was changed from

==========
Resynthesize constructors lazily.

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

to

==========
Resynthesize constructors lazily.

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

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

[scheglov, 2016-06-03 16:38:29]

Committed patchset #2 (id:20001) manually as
98553a49171b1dd9849ac4f23e670a35fe35586b (presubmit successful).