Add generic types for strong mode

Add generic types for strong mode

FieldInfo, Extension, and PbList should always be created
with the correct reified type when in strong mode.

This is still experimental (not tested with DDC).

BUG=https://github.com/dart-lang/dart-protobuf/issues/54
R=leafp@google.com, sgjesse@google.com

Committed: https://github.com/dart-lang/protobuf/commit/6d131e36f4c4b4b6ada59ab55f307a847e86bdd7

[skybrian, 2016-04-01 01:52:42]

skybrian@google.com changed reviewers:
+ leafp@google.com, sgjesse@google.com, vsm@google.com

[Søren Gjesse, 2016-04-01 06:24:28]

lgtm

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
File lib/src/protobuf/field_set.dart (right):

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
lib/src/protobuf/field_set.dart:222: if (value != null) return value as
List/*<T>*/;
This is more a strong mode question.

Why is this explicit cast required? The return type is known, so it could just
be implicit.

[Leaf, 2016-04-01 17:09:56]

lgtm

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
File lib/src/protobuf/extension_field_set.dart (right):

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
lib/src/protobuf/extension_field_set.dart:46: if (list != null) return list as
List/*<T>*/;
Is this going to have the right dynamic type (and hence work in DDC), or is it
going to be List<dynamic>?

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
File lib/src/protobuf/field_set.dart (right):

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
lib/src/protobuf/field_set.dart:222: if (value != null) return value as
List/*<T>*/;
On 2016/04/01 06:24:28, Søren Gjesse wrote:
> This is more a strong mode question.
> 
> Why is this explicit cast required? The return type is known, so it could just
> be implicit.

We (strong mode) warn on categories of casts that are likely to fail in DDC but
succeed in regular checked mode.  Casting from List<dynamic> to List<NotDynamic>
is one of those categories.  We allow an explicit cast so that you can do the
conversion if you're sure that the runtime value will actually have the right
reified type.

[skybrian, 2016-04-01 18:01:40]

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
File lib/src/protobuf/extension_field_set.dart (right):

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
lib/src/protobuf/extension_field_set.dart:46: if (list != null) return list as
List/*<T>*/;
On 2016/04/01 17:09:56, Leaf wrote:
> Is this going to have the right dynamic type (and hence work in DDC), or is it
> going to be List<dynamic>?

It's not statically checkable but the intent is that it always has the right
type. (The list should always be created by fi._createRepeatedField, below, and
the FieldInfo object should have been created with the right dynamic type.)

I'd be happier if I had a way to test this, though.

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
File lib/src/protobuf/field_info.dart (right):

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
lib/src/protobuf/field_info.dart:49: this.makeDefault = (() => new
PbList<T>(check: check)) {
This is a bit inconsistent.

For a non-repeated field, T is the field's type.
For a repeated field, T is the *base type* and List<T> is the field type. (But
it's really a PbList or perhaps some other kind of list if a mixin overrides
it.)

Similarly for extensions. As a result, we can't really write generic methods
that work uniformly with non-repeated and repeated fields. (There are internal
methods that work that way, but they are currently untyped.)

I think I'm just going to live with it for now.

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
File lib/src/protobuf/field_set.dart (right):

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
lib/src/protobuf/field_set.dart:222: if (value != null) return value as
List/*<T>*/;
On 2016/04/01 17:09:56, Leaf wrote:
> On 2016/04/01 06:24:28, Søren Gjesse wrote:
> > This is more a strong mode question.
> > 
> > Why is this explicit cast required? The return type is known, so it could
just
> > be implicit.
> 
> We (strong mode) warn on categories of casts that are likely to fail in DDC
but
> succeed in regular checked mode.  Casting from List<dynamic> to
List<NotDynamic>
> is one of those categories.  We allow an explicit cast so that you can do the
> conversion if you're sure that the runtime value will actually have the right
> reified type.

The return type of _getFieldOrNull is dynamic. I tried turning it into a generic
method, but ran into complications, and it didn't seem to make the warning go
away.

[skybrian, 2016-04-01 18:04:35]

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
File lib/src/protobuf/extension_field_set.dart (right):

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
lib/src/protobuf/extension_field_set.dart:46: if (list != null) return list as
List/*<T>*/;
On 2016/04/01 18:01:40, skybrian wrote:
> On 2016/04/01 17:09:56, Leaf wrote:
> > Is this going to have the right dynamic type (and hence work in DDC), or is
it
> > going to be List<dynamic>?
> 
> It's not statically checkable but the intent is that it always has the right
> type. (The list should always be created by fi._createRepeatedField, below,
and
> the FieldInfo object should have been created with the right dynamic type.)
> 
> I'd be happier if I had a way to test this, though.

On second thought, maybe this doesn't work? I'm returning a PbList<T> in place
of a List<T>. Will the "as" check succeed?

[Leaf, 2016-04-01 18:15:59]

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
File lib/src/protobuf/extension_field_set.dart (right):

https://chromiumcodereview.appspot.com/1844293003/diff/20001/lib/src/protobuf...
lib/src/protobuf/extension_field_set.dart:46: if (list != null) return list as
List/*<T>*/;
On 2016/04/01 18:04:35, skybrian wrote:
> On 2016/04/01 18:01:40, skybrian wrote:
> > On 2016/04/01 17:09:56, Leaf wrote:
> > > Is this going to have the right dynamic type (and hence work in DDC), or
is
> it
> > > going to be List<dynamic>?
> > 
> > It's not statically checkable but the intent is that it always has the right
> > type. (The list should always be created by fi._createRepeatedField, below,
> and
> > the FieldInfo object should have been created with the right dynamic type.)
> > 
> > I'd be happier if I had a way to test this, though.
> 
> On second thought, maybe this doesn't work? I'm returning a PbList<T> in place
> of a List<T>. Will the "as" check succeed?

If PbList<T> implements or extends List<T>, then yes.

[skybrian, 2016-04-01 19:17:29]

Description was changed from

==========
Add generic types for strong mode

FieldInfo, Extension, and PbList should always be created
with the correct reified type when in strong mode.

This is still experimental (not tested with DDC).

BUG=https://github.com/dart-lang/dart-protobuf/issues/54
==========

to

==========
Add generic types for strong mode

FieldInfo, Extension, and PbList should always be created
with the correct reified type when in strong mode.

This is still experimental (not tested with DDC).

BUG=https://github.com/dart-lang/dart-protobuf/issues/54
R=leafp@google.com, sgjesse@google.com

Committed:
https://github.com/dart-lang/protobuf/commit/6d131e36f4c4b4b6ada59ab55f307a84...
==========

[skybrian, 2016-04-01 19:17:31]

Committed patchset #2 (id:20001) manually as
6d131e36f4c4b4b6ada59ab55f307a847e86bdd7 (presubmit successful).