Change new List(n) to return fixed length list.

Change new List(n) to return fixed length list.
Deprecate List.fixedLength, add List.filled.

Make Iterable.toList and List.from take "growable" argument,
defaulting to false.

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

[floitsch, 2013-02-26 13:54:19]

LGTM.

https://codereview.chromium.org/12328104/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/cleanup/migration/Migrate_1M3_corelib_CleanUp.java
(right):

https://codereview.chromium.org/12328104/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/cleanup/migration/Migrate_1M3_corelib_CleanUp.java:339:
// new List(5)  --->  new List(5)
No.
This is a Cleanup. Leave the comment and coordinate with Konstantin.

https://codereview.chromium.org/12328104/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.tools.ui_test/src/com/google/dart/tools/ui/cleanup/MigrateCleanUpTest.java
(right):

https://codereview.chromium.org/12328104/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.tools.ui_test/src/com/google/dart/tools/ui/cleanup/MigrateCleanUpTest.java:1130:
"  new List(5);",
ditto.
Leave as is.

https://codereview.chromium.org/12328104/diff/1/pkg/analyzer-experimental/lib...
File pkg/analyzer-experimental/lib/src/generated/resolver.dart (left):

https://codereview.chromium.org/12328104/diff/1/pkg/analyzer-experimental/lib...
pkg/analyzer-experimental/lib/src/generated/resolver.dart:1: // This code was
auto-generated, is not intended to be edited, and is subject to
All these files in the "generated" directory are likely to be the result of
java2dart. You probably don't need/want to change these.

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/lib/src/bas...
File pkg/serialization/lib/src/basic_rule.dart (right):

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/lib/src/bas...
pkg/serialization/lib/src/basic_rule.dart:138: * The list need to be growable
for SimpleJson format to work.
If a list is returned, it is growable.

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/lib/src/bas...
pkg/serialization/lib/src/basic_rule.dart:141: useMaps ? new
_MapWrapper(fields.contents)
I would prefer an 'if', and the length-assignment in a separate statement.

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/lib/src/for...
File pkg/serialization/lib/src/format.dart (left):

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/lib/src/for...
pkg/serialization/lib/src/format.dart:260: result[2] = new List();
keep. It doesn't change anything.

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/test/serial...
File pkg/serialization/test/serialization_test.dart (right):

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/test/serial...
pkg/serialization/test/serialization_test.dart:475: print(eachFormat);
remove.

https://codereview.chromium.org/12328104/diff/1/runtime/lib/string_base.dart
File runtime/lib/string_base.dart (right):

https://codereview.chromium.org/12328104/diff/1/runtime/lib/string_base.dart#...
runtime/lib/string_base.dart:350: var stringList = new _ObjectArray(numValues);
new List(numValues) doesn't work?

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js/nodes.dart (right):

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:1084: var codes =
put on same line.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:653: new
List<jsAst.Parameter>(
probably fits on one line.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:657: new
List<jsAst.Expression>(
ditto.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/optimize.dart (right):

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/optimize.dart:350: new
Selector.callConstructor(const SourceString(""),
please test (manually) that this catches the right constructor.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/util/link_implementation.dart
(right):

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/util/link_implementation.dart:84:
List<T> toList({ bool growable: false }) {
=> new List<T>.from(this, growable: growable);

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

https://codereview.chromium.org/12328104/diff/1/sdk/lib/core/list.dart#newcode15
sdk/lib/core/list.dart:15: * * Growable list. Full implmentation of the
interface.
implementation

https://codereview.chromium.org/12328104/diff/1/sdk/lib/core/list.dart#newcode16
sdk/lib/core/list.dart:16: * * Unmodifiable list. A list where you can't change
length or values.
... list. Changing the length or values yields an exception. If an unmodifiable
list is backed by a mutable structure reading the values can yield different
results over time, though.

Example:

    var fixedLengthList = new List(5);
    fixedLengthList.length = 0;  // throws.
    fixedLengthList.add(499);  // throws
    var growableList = [1, 2];
    var unmodifiableList = growableList.reversed;
    unmodifiableList[0] = 5;  // throws.
    unmodifiableList.length = 0;  // throws.
    reversed[0];  // 2.
    growableList.add(499);
    reversed[0];  // 499.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/core/list.dart#newcode28
sdk/lib/core/list.dart:28: * Creates a fixed-sized list of the given [length]
where each entry is
fixed-length

https://codereview.chromium.org/12328104/diff/1/sdk/lib/web_sql/dart2js/web_s...
File sdk/lib/web_sql/dart2js/web_sql_dart2js.dart (right):

https://codereview.chromium.org/12328104/diff/1/sdk/lib/web_sql/dart2js/web_s...
sdk/lib/web_sql/dart2js/web_sql_dart2js.dart:269: new List<Map>.from(this,
growable: growable);
IterableMixinWorkaround.toListList(this, growable);

https://codereview.chromium.org/12328104/diff/1/sdk/lib/web_sql/dartium/web_s...
File sdk/lib/web_sql/dartium/web_sql_dartium.dart (right):

https://codereview.chromium.org/12328104/diff/1/sdk/lib/web_sql/dartium/web_s...
sdk/lib/web_sql/dartium/web_sql_dartium.dart:291: new List<Map>.from(this,
growable: growable);
IterableMixinWorkaround.toListList(this, growable);

https://codereview.chromium.org/12328104/diff/1/tests/corelib/list_insert_ran...
File tests/corelib/list_insert_range_test.dart (right):

https://codereview.chromium.org/12328104/diff/1/tests/corelib/list_insert_ran...
tests/corelib/list_insert_range_test.dart:26: list = [1, 1].toList(growable:
true); list.insertRange(1, 1, 2);
revert changes.

https://codereview.chromium.org/12328104/diff/1/tests/language/fannkuch_test....
File tests/language/fannkuch_test.dart (right):

https://codereview.chromium.org/12328104/diff/1/tests/language/fannkuch_test....
tests/language/fannkuch_test.dart:11: var p = new List()..length = n, q = new
List(n), s = new List(n);
p was a fixedLength list too.

https://codereview.chromium.org/12328104/diff/1/tools/dom/templates/html/impl...
File tools/dom/templates/html/impl/impl_Element.darttemplate (right):

https://codereview.chromium.org/12328104/diff/1/tools/dom/templates/html/impl...
tools/dom/templates/html/impl/impl_Element.darttemplate:20: ?
(<Element>[]..length = _childElements.length)
I would prefer an if with 2 lines for the length.

https://codereview.chromium.org/12328104/diff/1/tools/dom/templates/html/impl...
tools/dom/templates/html/impl/impl_Element.darttemplate:318: new
List<Element>.from(this, growable: growable);
IterableMixinWorkaround.toListList(this, growable).

https://codereview.chromium.org/12328104/diff/1/tools/dom/templates/html/impl...
File tools/dom/templates/html/impl/impl_Node.darttemplate (right):

https://codereview.chromium.org/12328104/diff/1/tools/dom/templates/html/impl...
tools/dom/templates/html/impl/impl_Node.darttemplate:161: new
List<Node>.from(this, growable: growable);
IterableMixinWorkaround.toListList(this, growable).

https://codereview.chromium.org/12328104/diff/1/tools/dom/templates/immutable...
File tools/dom/templates/immutable_list_mixin.darttemplate (right):

https://codereview.chromium.org/12328104/diff/1/tools/dom/templates/immutable...
tools/dom/templates/immutable_list_mixin.darttemplate:46: new
List<$E>.from(this, growable: growable);
IterableMixinWorkaround.

[Lasse Reichstein Nielsen, 2013-02-26 15:26:00]

Ready to commit. Will do first thing tomorrow (if you don't want to try it out
earlier).

https://codereview.chromium.org/12328104/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/cleanup/migration/Migrate_1M3_corelib_CleanUp.java
(right):

https://codereview.chromium.org/12328104/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.tools.ui/src/com/google/dart/tools/ui/internal/cleanup/migration/Migrate_1M3_corelib_CleanUp.java:339:
// new List(5)  --->  new List(5)
Reverted.

https://codereview.chromium.org/12328104/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.tools.ui_test/src/com/google/dart/tools/ui/cleanup/MigrateCleanUpTest.java
(right):

https://codereview.chromium.org/12328104/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.tools.ui_test/src/com/google/dart/tools/ui/cleanup/MigrateCleanUpTest.java:1130:
"  new List(5);",
On 2013/02/26 13:54:19, floitsch wrote:
> ditto.
> Leave as is.

Done.

https://codereview.chromium.org/12328104/diff/1/pkg/analyzer-experimental/lib...
File pkg/analyzer-experimental/lib/src/generated/resolver.dart (left):

https://codereview.chromium.org/12328104/diff/1/pkg/analyzer-experimental/lib...
pkg/analyzer-experimental/lib/src/generated/resolver.dart:1: // This code was
auto-generated, is not intended to be edited, and is subject to
WTF is generated code doing in the repository.
Since I don't know how to generate it, or who uses it, I'll keep the changes and
let whoever generates it next worry about the result.

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/lib/src/for...
File pkg/serialization/lib/src/format.dart (left):

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/lib/src/for...
pkg/serialization/lib/src/format.dart:260: result[2] = new List();
But it's also inconsistent with the declaration of flatData above. Consider it a
free cleanup in the passing.

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/test/serial...
File pkg/serialization/test/serialization_test.dart (right):

https://codereview.chromium.org/12328104/diff/1/pkg/serialization/test/serial...
pkg/serialization/test/serialization_test.dart:475: print(eachFormat);
On 2013/02/26 13:54:19, floitsch wrote:
> remove.

Done.

https://codereview.chromium.org/12328104/diff/1/runtime/lib/string_base.dart
File runtime/lib/string_base.dart (right):

https://codereview.chromium.org/12328104/diff/1/runtime/lib/string_base.dart#...
runtime/lib/string_base.dart:350: var stringList = new _ObjectArray(numValues);
It didn't at some point, probably before I changed the constructor-recognition
in the VM.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js/nodes.dart (right):

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:1084: var codes =
Done, and changed to be .filled.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:653: new
List<jsAst.Parameter>(
On 2013/02/26 13:54:19, floitsch wrote:
> probably fits on one line.

Done.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:657: new
List<jsAst.Expression>(
Doesn't fit.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/optimize.dart (right):

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/optimize.dart:350: new
Selector.callConstructor(const SourceString(""),
On 2013/02/26 13:54:19, floitsch wrote:
> please test (manually) that this catches the right constructor.

Done.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/util/link_implementation.dart
(right):

https://codereview.chromium.org/12328104/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/util/link_implementation.dart:84:
List<T> toList({ bool growable: false }) {
On 2013/02/26 13:54:19, floitsch wrote:
> => new List<T>.from(this, growable: growable);

Done.

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

https://codereview.chromium.org/12328104/diff/1/sdk/lib/core/list.dart#newcode15
sdk/lib/core/list.dart:15: * * Growable list. Full implmentation of the
interface.
On 2013/02/26 13:54:19, floitsch wrote:
> implementation

Done.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/core/list.dart#newcode16
sdk/lib/core/list.dart:16: * * Unmodifiable list. A list where you can't change
length or values.
Example is very long. And reversed doesn't return a List. I don't think we have
an example of a mutable-backed unmodifiable list in the standard library.

"yields an exception" is vague and slightly confusing. I prefer "It is an error
to".

Reworded and example added.

https://codereview.chromium.org/12328104/diff/1/sdk/lib/core/list.dart#newcode28
sdk/lib/core/list.dart:28: * Creates a fixed-sized list of the given [length]
where each entry is
On 2013/02/26 13:54:19, floitsch wrote:
> fixed-length

Done.

https://codereview.chromium.org/12328104/diff/1/tests/corelib/list_insert_ran...
File tests/corelib/list_insert_range_test.dart (right):

https://codereview.chromium.org/12328104/diff/1/tests/corelib/list_insert_ran...
tests/corelib/list_insert_range_test.dart:26: list = [1, 1].toList(growable:
true); list.insertRange(1, 1, 2);
On 2013/02/26 13:54:19, floitsch wrote:
> revert changes.

Done.

https://codereview.chromium.org/12328104/diff/1/tests/language/fannkuch_test....
File tests/language/fannkuch_test.dart (right):

https://codereview.chromium.org/12328104/diff/1/tests/language/fannkuch_test....
tests/language/fannkuch_test.dart:11: var p = new List()..length = n, q = new
List(n), s = new List(n);
On 2013/02/26 13:54:19, floitsch wrote:
> p was a fixedLength list too.

Done.

https://codereview.chromium.org/12328104/diff/1/tools/dom/templates/html/impl...
File tools/dom/templates/html/impl/impl_Element.darttemplate (right):

https://codereview.chromium.org/12328104/diff/1/tools/dom/templates/html/impl...
tools/dom/templates/html/impl/impl_Element.darttemplate:20: ?
(<Element>[]..length = _childElements.length)
On 2013/02/26 13:54:19, floitsch wrote:
> I would prefer an if with 2 lines for the length.

Done.

[Lasse Reichstein Nielsen, 2013-02-27 08:46:25]

Committed patchset #3 manually as r19112 (presubmit successful).

[ngeoffray, 2013-02-27 08:59:12]

https://codereview.chromium.org/12328104/diff/10002/sdk/lib/core/list.dart
File sdk/lib/core/list.dart (right):

https://codereview.chromium.org/12328104/diff/10002/sdk/lib/core/list.dart#ne...
sdk/lib/core/list.dart:42: * growable list if [length] is omitted.
Shouldn't you say is omitted or null?

[ngeoffray, 2013-02-27 09:01:30]

https://codereview.chromium.org/12328104/diff/10002/sdk/lib/html/dart2js/html...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/12328104/diff/10002/sdk/lib/html/dart2js/html...
sdk/lib/html/dart2js/html_dart2js.dart:7976: +      output = <Element>[];
output is final

[floitsch, 2013-02-27 10:04:58]

https://codereview.chromium.org/12328104/diff/10002/sdk/lib/core/list.dart
File sdk/lib/core/list.dart (right):

https://codereview.chromium.org/12328104/diff/10002/sdk/lib/core/list.dart#ne...
sdk/lib/core/list.dart:42: * growable list if [length] is omitted.
On 2013/02/27 08:59:12, ngeoffray wrote:
> Shouldn't you say is omitted or null?

No. null is an error.

[ngeoffray, 2013-02-27 12:02:32]

https://codereview.chromium.org/12328104/diff/10002/sdk/lib/core/list.dart
File sdk/lib/core/list.dart (right):

https://codereview.chromium.org/12328104/diff/10002/sdk/lib/core/list.dart#ne...
sdk/lib/core/list.dart:42: * growable list if [length] is omitted.
On 2013/02/27 10:04:58, floitsch wrote:
> On 2013/02/27 08:59:12, ngeoffray wrote:
> > Shouldn't you say is omitted or null?
> 
> No. null is an error.

Maybe add that too in a comment?

[Lasse Reichstein Nielsen, 2013-02-27 12:13:44]

Schneider, could you just check out the VM change?

[Florian Schneider, 2013-02-27 12:26:55]

VM changes LGTM.

https://codereview.chromium.org/12328104/diff/10002/runtime/vm/flow_graph_bui...
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/12328104/diff/10002/runtime/vm/flow_graph_bui...
runtime/vm/flow_graph_builder.cc:2008: if (node->arguments()->length() == 0) {
style suggestion:

return (node->arguments()->length() == 0)
   ? kGrowableObjectArrayCid
   : kArrayCid;

[Ivan Posva, 2013-02-27 18:14:35]

https://codereview.chromium.org/12328104/diff/10002/runtime/lib/array_patch.dart
File runtime/lib/array_patch.dart (right):

https://codereview.chromium.org/12328104/diff/10002/runtime/lib/array_patch.d...
runtime/lib/array_patch.dart:11: if ((length is! int) || (length < 0)) {
Why do we still do manual type checking here?

[floitsch, 2013-02-28 09:41:57]

https://codereview.chromium.org/12328104/diff/10002/runtime/lib/array_patch.dart
File runtime/lib/array_patch.dart (right):

https://codereview.chromium.org/12328104/diff/10002/runtime/lib/array_patch.d...
runtime/lib/array_patch.dart:11: if ((length is! int) || (length < 0)) {
On 2013/02/27 18:14:36, Ivan Posva wrote:
> Why do we still do manual type checking here?

1) because we didn't go through code to remove checks.
2) we do this check in dart2js, to avoid passing something bad to JS. This way
these two implementations are consistent.