Change List/Set/Map/Queue.from constructrs to accept any iterable.

Change List/Set/Map/Queue.from constructrs to accept any iterable.

This allows using the .from constructor as a cast:

  new Set<int>.from(numSetContainingOnlyInts);

BUG= http://dartbug.com/21731
R=sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=42620

[Lasse Reichstein Nielsen, 2015-01-05 14:38:19]

lrn@google.com changed reviewers:
+ floitsch@google.com, sgjesse@google.com

[Lasse Reichstein Nielsen, 2015-01-05 14:38:19]

lrn@google.com changed required reviewers:
+ sgjesse@google.com

[floitsch, 2015-01-05 14:45:30]

DBC.
(looks good to me, but I didn't look very closely).

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
File sdk/lib/collection/hash_set.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
sdk/lib/collection/hash_set.dart:88: * Create a hash set containing all of
[elements].
all [elements]. ?

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/splay_tre...
File sdk/lib/collection/splay_tree.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/splay_tre...
sdk/lib/collection/splay_tree.dart:275: [ int compare(K key1, K key2),
remove space.

[Søren Gjesse, 2015-01-05 15:35:15]

lgtm

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_map....
File sdk/lib/collection/hash_map.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_map....
sdk/lib/collection/hash_map.dart:87: other.forEach((k, v) { result[k] = v; });
Add types K and V?

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
File sdk/lib/collection/hash_set.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
sdk/lib/collection/hash_set.dart:93: * All the [elements] should be assignable
to [E].
[E] -> `E`

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/linked_ha...
File sdk/lib/collection/linked_hash_set.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/linked_ha...
sdk/lib/collection/linked_hash_set.dart:84: for (final E element in elements) {
Any particular reason for using final here?

[Lasse Reichstein Nielsen, 2015-01-06 10:14:46]

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_map....
File sdk/lib/collection/hash_map.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_map....
sdk/lib/collection/hash_map.dart:87: other.forEach((k, v) { result[k] = v; });
The forEach method has type (K',V')->void where other:Map<K',V'>.
A (K,V)->void function might not be assignable to (K',V')->void, even if all the
keys and values of `other` happens to be K/V.

Example:
 class C {}
 class I {}
 class D extends C implements I {}
 Map<C,C> foo = ...;
 if (c.keys.every((v) => v is I) && c.values.every((v) => v is I)) {
   new Map<I,I>.from(foo);
 }

But it's tempting, so I'll add a test to catch it if someone decides to add K
and V later.

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
File sdk/lib/collection/hash_set.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
sdk/lib/collection/hash_set.dart:88: * Create a hash set containing all of
[elements].
On 2015/01/05 14:45:30, floitsch wrote:
> all [elements]. ?

Done.

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
sdk/lib/collection/hash_set.dart:93: * All the [elements] should be assignable
to [E].
Why? The [E] is in scope, so we might as well link to it.

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/linked_ha...
File sdk/lib/collection/linked_hash_set.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/linked_ha...
sdk/lib/collection/linked_hash_set.dart:84: for (final E element in elements) {
No. Copied it from somewhere else that had a final.
No reason not to make it final either.

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/splay_tre...
File sdk/lib/collection/splay_tree.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/splay_tre...
sdk/lib/collection/splay_tree.dart:275: [ int compare(K key1, K key2),
On 2015/01/05 14:45:30, floitsch wrote:
> remove space.

Done.

[Lasse Reichstein Nielsen, 2015-01-06 10:15:41]

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

[Søren Gjesse, 2015-01-06 12:09:05]

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_map....
File sdk/lib/collection/hash_map.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_map....
sdk/lib/collection/hash_map.dart:87: other.forEach((k, v) { result[k] = v; });
On 2015/01/06 10:14:45, Lasse Reichstein Nielsen wrote:
> The forEach method has type (K',V')->void where other:Map<K',V'>.
> A (K,V)->void function might not be assignable to (K',V')->void, even if all
the
> keys and values of `other` happens to be K/V.
> 
> Example:
>  class C {}
>  class I {}
>  class D extends C implements I {}
>  Map<C,C> foo = ...;
>  if (c.keys.every((v) => v is I) && c.values.every((v) => v is I)) {
>    new Map<I,I>.from(foo);
>  }
> 
> But it's tempting, so I'll add a test to catch it if someone decides to add K
> and V later.

Of cause.

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
File sdk/lib/collection/hash_set.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
sdk/lib/collection/hash_set.dart:93: * All the [elements] should be assignable
to [E].
On 2015/01/06 10:14:45, Lasse Reichstein Nielsen wrote:
> Why? The [E] is in scope, so we might as well link to it.

Good - I am not exactly clear on the dartdoc scoping rules thought they where
method specific and that [E] would not link to anything.

[Lasse Reichstein Nielsen, 2015-01-07 07:56:45]

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
File sdk/lib/collection/hash_set.dart (right):

https://codereview.chromium.org/838463002/diff/1/sdk/lib/collection/hash_set....
sdk/lib/collection/hash_set.dart:93: * All the [elements] should be assignable
to [E].
The spec tries to specify how references in DartDoc are resolved, but it's
somewhat vague.

In practice it *should* work so that any name that can be referenced from
anywhere inside the documented declaration, is in scope.
The type parameter of a class should be in scope for DartDoc in the class
documentation and in any DartDoc inside the class (I don't think we make
exceptions for static functions).

A type parameter can be used in a generative constructor, so it should be in
scope here. I haven't checked if it actually works with the DartDoc tool,
though, but if not, I'll have to file a bug report :)