Add Map.unmodifiable constructor.

Add Map.unmodifiable constructor.

R=sgjesse@google.com, sra@google.com

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

[Lasse Reichstein Nielsen, 2015-05-04 07:18:07]

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

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

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart
File sdk/lib/core/map.dart (right):

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart#newco...
sdk/lib/core/map.dart:66: external factory Map.unmodifiable(Map other);
Is UnmodifiableMapView used that much that it warrants this instead of just
writing

var someMap = ...;
new UnmodifiableMapView<K, V>(new Map.from(someMap))

Another option could be a .from constructor on UnmodifiableMapView.

[Lasse Reichstein Nielsen, 2015-05-05 05:09:40]

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart
File sdk/lib/core/map.dart (right):

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart#newco...
sdk/lib/core/map.dart:66: external factory Map.unmodifiable(Map other);
This constructor, like List.unmodifable, is intended to support Fletch's deeply
immutable data structures.
It needs a way to create immutable maps as well.

[Lasse Reichstein Nielsen, 2015-05-07 10:06:48]

lrn@google.com changed reviewers:
+ iposva@google.com, sra@google.com
- iposva@chromium.org

[Lasse Reichstein Nielsen, 2015-05-07 10:06:48]

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

[sra1, 2015-05-07 19:24:42]

js_lib changes lgtm

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/_internal/compiler/...
File sdk/lib/_internal/compiler/js_lib/constant_map.dart (right):

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/_internal/compiler/...
sdk/lib/_internal/compiler/js_lib/constant_map.dart:9: ConstantMapView(Map base)
: super(base);
indentation

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/_internal/compiler/...
sdk/lib/_internal/compiler/js_lib/constant_map.dart:14: factory
ConstantMap.from(Map other) {
Under what conditions can you return 'other'?
Or create a new instance to capture K,V but otherwise share structure?

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/_internal/compiler/...
sdk/lib/_internal/compiler/js_lib/constant_map.dart:30: if (k != "__proto__") {
Use single quotes for consistency with rest of file,

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/_internal/compiler/...
sdk/lib/_internal/compiler/js_lib/constant_map.dart:32: JS("void", "#[#] = #",
object, k, v);
ditto

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart
File sdk/lib/core/map.dart (right):

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart#newco...
sdk/lib/core/map.dart:64: * except that the map returned by this constructor
diallows modification.
disallows
or
does not allow
or
is not modifiable.

[Ivan Posva, 2015-05-11 18:16:54]

-Ivan

https://codereview.chromium.org/1127533002/diff/1/runtime/lib/core_patch.dart
File runtime/lib/core_patch.dart (left):

https://codereview.chromium.org/1127533002/diff/1/runtime/lib/core_patch.dart...
runtime/lib/core_patch.dart:11: 
?

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart
File sdk/lib/core/map.dart (right):

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart#newco...
sdk/lib/core/map.dart:61: * The created map iterates keys in the same order as
[other].
This is a strong statement to make. If other iterates in random order every time
we iterate, then it will be hard to preserve this behaviour. I strongly suggest
you just drop this line/requirement.

[Anders Johnsen, 2015-05-11 18:51:07]

ajohnsen@google.com changed reviewers:
+ ajohnsen@google.com

[Anders Johnsen, 2015-05-11 18:51:08]

DBC

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart
File sdk/lib/core/map.dart (right):

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart#newco...
sdk/lib/core/map.dart:61: * The created map iterates keys in the same order as
[other].
On 2015/05/11 18:16:54, Ivan Posva wrote:
> This is a strong statement to make. If other iterates in random order every
time
> we iterate, then it will be hard to preserve this behaviour. I strongly
suggest
> you just drop this line/requirement.

From
https://api.dartlang.org/apidocs/channels/stable/dartdoc-viewer/dart:core.Ite...

"The iterators of the same iterable should provide the same values in the same
order (unless the underlying collection is modified between the iterations,
which some collections allow)."

[Søren Gjesse, 2015-05-12 11:07:57]

lgtm

We already have the List.unmodifiable constructor.

[Lasse Reichstein Nielsen, 2015-05-12 11:09:46]

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

[Lasse Reichstein Nielsen, 2015-05-13 11:18:59]

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart
File sdk/lib/core/map.dart (right):

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart#newco...
sdk/lib/core/map.dart:61: * The created map iterates keys in the same order as
[other].
This can probably still be written a little better, just to be on the safe side.

The intend is that:
- the created map has a fixed iteration order.
- that iteration order matches an iteration order of the input map.
We only iterate the input map once, so if it changes iteration order, that's not
being tracked. It shouldn't change, but then, a lot of things shouldn't happen.

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart#newco...
sdk/lib/core/map.dart:64: * except that the map returned by this constructor
diallows modification.
On 2015/05/07 19:24:42, sra1 wrote:
> disallows
> or
> does not allow
> or
> is not modifiable.

Done.

[Lasse Reichstein Nielsen, 2015-05-13 11:18:59]

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart
File sdk/lib/core/map.dart (right):

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart#newco...
sdk/lib/core/map.dart:61: * The created map iterates keys in the same order as
[other].
This can probably still be written a little better, just to be on the safe side.

The intend is that:
- the created map has a fixed iteration order.
- that iteration order matches an iteration order of the input map.
We only iterate the input map once, so if it changes iteration order, that's not
being tracked. It shouldn't change, but then, a lot of things shouldn't happen.

https://codereview.chromium.org/1127533002/diff/1/sdk/lib/core/map.dart#newco...
sdk/lib/core/map.dart:64: * except that the map returned by this constructor
diallows modification.
On 2015/05/07 19:24:42, sra1 wrote:
> disallows
> or
> does not allow
> or
> is not modifiable.

Done.