Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(155)

Issue 70703002: Add constructors for JSArray (Closed)

Created:
7 years, 1 month ago by sra1
Modified:
7 years, 1 month ago
CC:
reviews_dartlang.org, karlklose
Visibility:
Public.

Description

This change makes it easier to put type parameters on JavaScript Arrays. There is one special factory constructor for putting the type on an existing Array, and some higher level factory constructors that implement the common List subtypes. The effect of this is to move the type parameter assignment from every "new List" site to a handful of site within JSArray. This shrinks some programs by half a percent, e,g, 3k of swarm. JSArray.toList() and JSArray.sublist() now return JSArrays with the expected type parameters. R=karlklose@google.com, ngeoffray@google.com Committed: https://code.google.com/p/dart/source/detail?r=30374

Patch Set 1 : #

Total comments: 2

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+107 lines, -53 lines) Patch
M sdk/lib/_internal/compiler/implementation/js_backend/backend.dart View 1 2 chunks +2 lines, -0 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/ssa/builder.dart View 1 5 chunks +4 lines, -4 lines 0 comments Download
M sdk/lib/_internal/lib/core_patch.dart View 2 chunks +3 lines, -16 lines 0 comments Download
M sdk/lib/_internal/lib/js_array.dart View 4 chunks +73 lines, -8 lines 0 comments Download
M sdk/lib/_internal/lib/js_names.dart View 1 2 2 chunks +2 lines, -3 lines 0 comments Download
M sdk/lib/_internal/lib/js_primitives.dart View 2 chunks +0 lines, -17 lines 0 comments Download
M tests/compiler/dart2js/mirrors_used_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/compiler/dart2js/mock_compiler.dart View 1 chunk +3 lines, -1 line 0 comments Download
M tests/compiler/dart2js/resolver_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/compiler/dart2js/value_range_test.dart View 1 chunk +2 lines, -0 lines 0 comments Download
M tests/corelib/iterable_to_list_test.dart View 4 chunks +12 lines, -0 lines 0 comments Download
M tests/utils/dummy_compiler_test.dart View 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
sra1
7 years, 1 month ago (2013-11-13 02:56:42 UTC) #1
sra1
Hi Karl, Kasper is busy with the conference, can you take a look? Thanks!
7 years, 1 month ago (2013-11-14 18:53:41 UTC) #2
sra1
ping
7 years, 1 month ago (2013-11-15 23:27:34 UTC) #3
ngeoffray
LGTM! https://codereview.chromium.org/70703002/diff/110013/sdk/lib/_internal/lib/js_array.dart File sdk/lib/_internal/lib/js_array.dart (right): https://codereview.chromium.org/70703002/diff/110013/sdk/lib/_internal/lib/js_array.dart#newcode41 sdk/lib/_internal/lib/js_array.dart:41: factory JSArray.growable(int length) { Is anyone using this ...
7 years, 1 month ago (2013-11-18 07:08:18 UTC) #4
karlklose
LGTM.
7 years, 1 month ago (2013-11-18 07:31:19 UTC) #5
sra1
7 years, 1 month ago (2013-11-18 21:06:59 UTC) #6
Message was sent while issue was closed.
Committed patchset #3 manually as r30374 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698