update Map

update Map

R=sra@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/ef2f54c5d20c01fe29b2d7377ab2886a360e0c0a

[Harry Terkelsen, 2016-05-09 21:44:12]

het@google.com changed reviewers:
+ sra@chromium.org, vsm@google.com

[sra1, 2016-05-09 23:16:39]

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

[sra1, 2016-05-09 23:16:41]

Lets punt the ConstantMaps for now. Constants work differently in DDC so I'm not
sure we want to port them yet.

I have marked two examples [A] and [B] where we should be able to get rid of
checks by adding type parameters to the code.
Lets add the types and back-port to dart2js to keep the code the same.  The goal
of this 'game' is to have the same code shared by DDC and dart2js, remove the
DDC warnings and generated checks, all without changing the code generated by
`dart2js` AND `dart2js --checked`.

https://codereview.chromium.org/1963723003/diff/1/lib/runtime/dart_sdk.js
File lib/runtime/dart_sdk.js (right):

https://codereview.chromium.org/1963723003/diff/1/lib/runtime/dart_sdk.js#new...
lib/runtime/dart_sdk.js:10327: +          let cell =
dart.as(this[_getTableEntry](strings, key), _js_helper.LinkedHashMapCell);
[B] get rid of this dart.as(..., LinkedHashMapCell)

https://codereview.chromium.org/1963723003/diff/1/lib/runtime/dart_sdk.js#new...
lib/runtime/dart_sdk.js:10328: +          return dart.as(cell == null ? null :
cell.hashMapCellValue, V);
[A] get rid of this dart.as(..., V)

https://codereview.chromium.org/1963723003/diff/1/tool/input_sdk/patch/core_p...
File tool/input_sdk/patch/core_patch.dart (left):

https://codereview.chromium.org/1963723003/diff/1/tool/input_sdk/patch/core_p...
tool/input_sdk/patch/core_patch.dart:335: return new UnmodifiableMapView<K,
V>(new Map<K, V>.from(other));
I think it is OK to leave this as-is.

The various ConstantMaps are not necessarily a good fit for DDC - it still needs
work on what a const map literal really is.

https://codereview.chromium.org/1963723003/diff/1/tool/input_sdk/private/link...
File tool/input_sdk/private/linked_hash_map.dart (right):

https://codereview.chromium.org/1963723003/diff/1/tool/input_sdk/private/link...
tool/input_sdk/private/linked_hash_map.dart:10: // DDC-specific, never use ES6
maps
Why not always?
ES6 maps are used in many places in the runtime.

https://codereview.chromium.org/1963723003/diff/1/tool/input_sdk/private/link...
tool/input_sdk/private/linked_hash_map.dart:101: LinkedHashMapCell cell =
_getTableEntry(strings, key);
[B] since _getTableEntry returns dynamic, there is a cast to LinkedHashMapCell.

https://codereview.chromium.org/1963723003/diff/1/tool/input_sdk/private/link...
tool/input_sdk/private/linked_hash_map.dart:102: return (cell == null) ? null :
cell.hashMapCellValue;
[A] this is a cast from dynamic to V, which generates dart.as(..., V).

https://codereview.chromium.org/1963723003/diff/1/tool/input_sdk/private/link...
tool/input_sdk/private/linked_hash_map.dart:305: _getTableEntry(var table, var
key) {
[B] There is no return type, so it is dynamic.

https://codereview.chromium.org/1963723003/diff/1/tool/input_sdk/private/link...
tool/input_sdk/private/linked_hash_map.dart:365: class LinkedHashMapCell {
This class needs DDC comment-style type parameters.

https://codereview.chromium.org/1963723003/diff/1/tool/sdk_expected_errors.txt
File tool/sdk_expected_errors.txt (right):

https://codereview.chromium.org/1963723003/diff/1/tool/sdk_expected_errors.tx...
tool/sdk_expected_errors.txt:68: [warning] Unsound implicit cast from dynamic to
V (dart:_js_helper/linked_hash_map.dart, line 102, col 14)
[A] this warning points to a problem.

[Harry Terkelsen, 2016-05-10 00:59:27]

PTAL, got rid of ConstantMap and added type annotations to remove almost all
'as' checks

https://codereview.chromium.org/1963723003/diff/1/tool/input_sdk/patch/core_p...
File tool/input_sdk/patch/core_patch.dart (left):

https://codereview.chromium.org/1963723003/diff/1/tool/input_sdk/patch/core_p...
tool/input_sdk/patch/core_patch.dart:335: return new UnmodifiableMapView<K,
V>(new Map<K, V>.from(other));
On 2016/05/09 23:16:40, sra1 wrote:
> I think it is OK to leave this as-is.
> 
> The various ConstantMaps are not necessarily a good fit for DDC - it still
needs
> work on what a const map literal really is.

Done.

[Harry Terkelsen, 2016-05-10 16:31:09]

I had to change it back to JsLinkedHashMap instead of Es6LinkedHashMap because
the latter has a bug apparently.

[Harry Terkelsen, 2016-05-10 23:02:03]

ptal, put almost all type annotations in comments

[sra1, 2016-05-10 23:05:25]

lgtm

https://codereview.chromium.org/1963723003/diff/60001/tool/input_sdk/patch/co...
File tool/input_sdk/patch/core_patch.dart (right):

https://codereview.chromium.org/1963723003/diff/60001/tool/input_sdk/patch/co...
tool/input_sdk/patch/core_patch.dart:10: ConstantMap,
Drop this?

https://codereview.chromium.org/1963723003/diff/60001/tool/sdk_expected_error...
File tool/sdk_expected_errors.txt (right):

https://codereview.chromium.org/1963723003/diff/60001/tool/sdk_expected_error...
tool/sdk_expected_errors.txt:56: [warning] Unsound implicit cast from dynamic to
LinkedHashMapCell<K, V> (dart:_js_helper/linked_hash_map.dart, line 119, col 40)
I think all of these are from JS-calls, which is OK.
Please verify.

[Harry Terkelsen, 2016-05-10 23:19:22]

Description was changed from

==========
update Map
==========

to

==========
update Map

R=sra@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/ef2f54c5d20c01fe29b2d7377ab2...
==========

[Harry Terkelsen, 2016-05-10 23:19:24]

Committed patchset #6 (id:100001) manually as
ef2f54c5d20c01fe29b2d7377ab2886a360e0c0a (presubmit successful).

[Harry Terkelsen, 2016-05-10 23:24:49]

https://codereview.chromium.org/1963723003/diff/60001/tool/input_sdk/patch/co...
File tool/input_sdk/patch/core_patch.dart (right):

https://codereview.chromium.org/1963723003/diff/60001/tool/input_sdk/patch/co...
tool/input_sdk/patch/core_patch.dart:10: ConstantMap,
On 2016/05/10 23:05:24, sra1 wrote:
> Drop this?

Done.

https://codereview.chromium.org/1963723003/diff/60001/tool/sdk_expected_error...
File tool/sdk_expected_errors.txt (right):

https://codereview.chromium.org/1963723003/diff/60001/tool/sdk_expected_error...
tool/sdk_expected_errors.txt:56: [warning] Unsound implicit cast from dynamic to
LinkedHashMapCell<K, V> (dart:_js_helper/linked_hash_map.dart, line 119, col 40)
On 2016/05/10 23:05:25, sra1 wrote:
> I think all of these are from JS-calls, which is OK.
> Please verify.

Confirming that they are all from JS-calls