Move Arrays, Collections and Maps into a new library, dart:collections.

Move Arrays, Collections and Maps into a new library, dart:collections.


BUG=

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

[floitsch, 2012-10-25 12:11:28]

https://codereview.chromium.org/11274043/diff/1/lib/collections/collections.dart
File lib/collections/collections.dart (right):

https://codereview.chromium.org/11274043/diff/1/lib/collections/collections.d...
lib/collections/collections.dart:5: #library("collections");
use new syntax.

https://codereview.chromium.org/11274043/diff/1/lib/collections/helpers.dart
File lib/collections/helpers.dart (right):

https://codereview.chromium.org/11274043/diff/1/lib/collections/helpers.dart#...
lib/collections/helpers.dart:103: static void _emitCollection(Collection c,
StringBuffer result, List visiting) {
nyc: 80chars.

[Anders Johnsen, 2012-10-25 12:24:29]

Hi,

This create a new library 'dart:collections'. I've updated the VM, dart2js and
create_sdk to match the change.

Ivan: Can you review my VM changes?
Anton: Do I need to change anything for dartium to work this this? Also, do I
have to change any html files? I tried to grep, but found nothing.
Peter: Can you review my internal dart2js library changes?

Thank you all,

- Anders

https://codereview.chromium.org/11274043/diff/1/lib/collections/collections.dart
File lib/collections/collections.dart (right):

https://codereview.chromium.org/11274043/diff/1/lib/collections/collections.d...
lib/collections/collections.dart:5: #library("collections");
On 2012/10/25 12:11:28, floitsch wrote:
> use new syntax.

Done.

https://codereview.chromium.org/11274043/diff/1/lib/collections/helpers.dart
File lib/collections/helpers.dart (right):

https://codereview.chromium.org/11274043/diff/1/lib/collections/helpers.dart#...
lib/collections/helpers.dart:103: static void _emitCollection(Collection c,
StringBuffer result, List visiting) {
On 2012/10/25 12:11:28, floitsch wrote:
> nyc: 80chars.

Done.

[Anton Muhin, 2012-10-25 12:52:51]

Anders,

There are two things we may want to do:

1) most probably we want to add this lib into a set of libs available in Dartium
(like json, uri, utf)
1) currently dart:html has its own copy of _Collections, we should migrate to
this.

But I don't think your change should break Dartium, so we can address that
later.

I'd be happy to patch and check or just be around when you're going to land it
to clean the stuff up if need be.

https://codereview.chromium.org/11274043/diff/6001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/11274043/diff/6001/runtime/vm/object.cc#newco...
runtime/vm/object.cc:937: /*
commented out code

[ahe, 2012-10-25 13:37:34]

https://codereview.chromium.org/11274043/diff/6001/lib/_internal/libraries.dart
File lib/_internal/libraries.dart (right):

https://codereview.chromium.org/11274043/diff/6001/lib/_internal/libraries.da...
lib/_internal/libraries.dart:1: // Copyright (c) 2012, the Dart project authors.
 Please see the AUTHORS file
This file LGTM

https://codereview.chromium.org/11274043/diff/6001/lib/compiler/implementatio...
File lib/compiler/implementation/lib/interceptors.dart (right):

https://codereview.chromium.org/11274043/diff/6001/lib/compiler/implementatio...
lib/compiler/implementation/lib/interceptors.dart:1: // Copyright (c) 2012, the
Dart project authors.  Please see the AUTHORS file
This file LGTM

https://codereview.chromium.org/11274043/diff/6001/lib/compiler/implementatio...
File lib/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11274043/diff/6001/lib/compiler/implementatio...
lib/compiler/implementation/lib/js_helper.dart:1: // Copyright (c) 2012, the
Dart project authors.  Please see the AUTHORS file
This file LGTM

https://codereview.chromium.org/11274043/diff/6001/tools/create_sdk.py
File tools/create_sdk.py (right):

https://codereview.chromium.org/11274043/diff/6001/tools/create_sdk.py#newcode1
tools/create_sdk.py:1: #!/usr/bin/env python
This file LGTM

[kasperl, 2012-10-25 13:41:27]

There's some amount of inconsistency in the library names in the dart:
"namespace". Some of them are plural (mirrors, collections) but others are
singular (isolate, scalarlist). Have you guys already decide on which scheme
you're going to follow?

[Anders Johnsen, 2012-10-25 15:07:14]

Agreed Kasper, renamed to dart:collection.

[Ivan Posva, 2012-10-26 04:48:17]

NMW.

-Ivan

https://codereview.chromium.org/11274043/diff/24/lib/collection/arrays.dart
File lib/collection/arrays.dart (right):

https://codereview.chromium.org/11274043/diff/24/lib/collection/arrays.dart#n...
lib/collection/arrays.dart:6: class Arrays {
In general I expected that we are going away from these kinds of all static
helper classes and instead we were going to put this functionality on the
abstract class directly.

https://codereview.chromium.org/11274043/diff/24/runtime/lib/collection_sourc...
File runtime/lib/collection_sources.gypi (right):

https://codereview.chromium.org/11274043/diff/24/runtime/lib/collection_sourc...
runtime/lib/collection_sources.gypi:1: # Copyright (c) 2012, the Dart project
authors.  Please see the AUTHORS file
This file should reside next to the actual sources in lib/collection. There is
nothing runtime specific here.

https://codereview.chromium.org/11274043/diff/24/runtime/vm/bootstrap.cc
File runtime/vm/bootstrap.cc (right):

https://codereview.chromium.org/11274043/diff/24/runtime/vm/bootstrap.cc#newc...
runtime/vm/bootstrap.cc:45: RawScript* Bootstrap::LoadCollectionScript(bool
patch) {
I thought that the dart:collection library was not being patched, in which case
we do not need this extra support here.

[dgrove, 2012-10-26 04:51:23]

https://codereview.chromium.org/11274043/diff/24/tools/create_sdk.py
File tools/create_sdk.py (right):

https://codereview.chromium.org/11274043/diff/24/tools/create_sdk.py#newcode26
tools/create_sdk.py:26: # ......collections/
DBC:collection

[Mads Ager (google), 2012-10-26 06:05:53]

https://codereview.chromium.org/11274043/diff/24/runtime/lib/collection_sourc...
File runtime/lib/collection_sources.gypi (right):

https://codereview.chromium.org/11274043/diff/24/runtime/lib/collection_sourc...
runtime/lib/collection_sources.gypi:1: # Copyright (c) 2012, the Dart project
authors.  Please see the AUTHORS file
On 2012/10/26 04:48:17, Ivan Posva wrote:
> This file should reside next to the actual sources in lib/collection. There is
> nothing runtime specific here.

I think it would be fine to move this to lib/collection. Most of this should go
away and be replaced by the one main library file which the VM should be able to
load. Any progress on issue 5099?

[Anders Johnsen, 2012-10-26 09:23:15]

Thanks for the feedback all.

https://codereview.chromium.org/11274043/diff/24/lib/collection/arrays.dart
File lib/collection/arrays.dart (right):

https://codereview.chromium.org/11274043/diff/24/lib/collection/arrays.dart#n...
lib/collection/arrays.dart:6: class Arrays {
On 2012/10/26 04:48:17, Ivan Posva wrote:
> In general I expected that we are going away from these kinds of all static
> helper classes and instead we were going to put this functionality on the
> abstract class directly.

Exactly, but first thing we do is eliminating coreimpl.

https://codereview.chromium.org/11274043/diff/24/runtime/lib/collection_sourc...
File runtime/lib/collection_sources.gypi (right):

https://codereview.chromium.org/11274043/diff/24/runtime/lib/collection_sourc...
runtime/lib/collection_sources.gypi:1: # Copyright (c) 2012, the Dart project
authors.  Please see the AUTHORS file
On 2012/10/26 06:05:54, Mads Ager wrote:
> On 2012/10/26 04:48:17, Ivan Posva wrote:
> > This file should reside next to the actual sources in lib/collection. There
is
> > nothing runtime specific here.
> 
> I think it would be fine to move this to lib/collection. Most of this should
go
> away and be replaced by the one main library file which the VM should be able
to
> load. Any progress on issue 5099?

Done.

https://codereview.chromium.org/11274043/diff/24/runtime/lib/collection_sourc...
runtime/lib/collection_sources.gypi:1: # Copyright (c) 2012, the Dart project
authors.  Please see the AUTHORS file
On 2012/10/26 04:48:17, Ivan Posva wrote:
> This file should reside next to the actual sources in lib/collection. There is
> nothing runtime specific here.

Done.

https://codereview.chromium.org/11274043/diff/24/runtime/vm/bootstrap.cc
File runtime/vm/bootstrap.cc (right):

https://codereview.chromium.org/11274043/diff/24/runtime/vm/bootstrap.cc#newc...
runtime/vm/bootstrap.cc:45: RawScript* Bootstrap::LoadCollectionScript(bool
patch) {
On 2012/10/26 04:48:17, Ivan Posva wrote:
> I thought that the dart:collection library was not being patched, in which
case
> we do not need this extra support here.

I'm not sure if we are going to patch it yet. I'll make sure to clean this up if
we don't patch.

https://codereview.chromium.org/11274043/diff/24/tools/create_sdk.py
File tools/create_sdk.py (right):

https://codereview.chromium.org/11274043/diff/24/tools/create_sdk.py#newcode26
tools/create_sdk.py:26: # ......collections/
On 2012/10/26 04:51:23, dgrove wrote:
> DBC:collection

Ty, done.

[floitsch, 2012-10-26 12:31:28]

LGTM.

https://chromiumcodereview.appspot.com/11274043/diff/15001/lib/collection/map...
File lib/collection/maps.dart (right):

https://chromiumcodereview.appspot.com/11274043/diff/15001/lib/collection/map...
lib/collection/maps.dart:4: 
See if "part of" already works.

https://chromiumcodereview.appspot.com/11274043/diff/15001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/11274043/diff/15001/runtime/vm/object....
runtime/vm/object.cc:938: patch_script = Bootstrap::LoadMathScript(true);
Remove commented code.

[Anders Johnsen, 2012-11-12 12:07:11]

https://codereview.chromium.org/11274043/diff/15001/lib/collection/maps.dart
File lib/collection/maps.dart (right):

https://codereview.chromium.org/11274043/diff/15001/lib/collection/maps.dart#...
lib/collection/maps.dart:4: 
On 2012/10/26 12:31:28, floitsch wrote:
> See if "part of" already works.

No support in editor yet.

https://codereview.chromium.org/11274043/diff/15001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/11274043/diff/15001/runtime/vm/object.cc#newc...
runtime/vm/object.cc:938: patch_script = Bootstrap::LoadMathScript(true);
On 2012/10/26 12:31:28, floitsch wrote:
> Remove commented code.

Done.