Skip to content

Improve holey array handling and remove Array.create #855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5865,7 +5865,7 @@ export class Compiler extends DiagnosticEmitter {
}

// apply concrete types to the generic function signature
let resolvedTypeArguments = Array.create<Type>(numTypeParameters);
let resolvedTypeArguments = new Array<Type>(numTypeParameters);
for (let i = 0; i < numTypeParameters; ++i) {
let name = typeParameterNodes[i].name.text;
if (contextualTypeArguments.has(name)) {
Expand Down
61 changes: 24 additions & 37 deletions std/assembly/array.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ export class Array<T> extends ArrayBufferView {
// to work with typed and normal arrays interchangeably. Technically, normal arrays do not need
// `dataStart` (equals `data`) and `dataLength` (equals computed `data.byteLength`).

// Also note that Array<T> with non-nullable T must guard against implicit null values whenever
// length is modified in a way that a null value would exist. Otherwise, the compiler wouldn't be
// able to guarantee type-safety anymore. For lack of a better word, such an array is "holey".
// Also note that Array<T> with non-nullable T must guard against uninitialized null values
// whenever an element is accessed. Otherwise, the compiler wouldn't be able to guarantee
// type-safety anymore. For lack of a better word, such an array is "holey".

private length_: i32;

Expand All @@ -42,20 +42,14 @@ export class Array<T> extends ArrayBufferView {
}

static create<T>(capacity: i32 = 0): Array<T> {
if (<u32>capacity > <u32>BLOCK_MAXSIZE >>> alignof<T>()) throw new RangeError(E_INVALIDLENGTH);
var array = changetype<Array<T>>(__allocArray(capacity, alignof<T>(), idof<T[]>())); // retains
changetype<Array<T>>(array).length_ = 0; // safe even if T is a non-nullable reference
memory.fill(array.dataStart, 0, <usize>array.dataLength);
WARNING("'Array.create' is deprecated. Use 'new Array' instead, making sure initial elements are initialized.");
var array = new Array<T>(capacity);
array.length = 0;
return array;
}

constructor(length: i32 = 0) {
super(length, alignof<T>());
if (isReference<T>()) {
if (!isNullable<T>()) {
if (length) throw new Error(E_HOLEYARRAY);
}
}
this.length_ = length;
}

Expand All @@ -69,19 +63,17 @@ export class Array<T> extends ArrayBufferView {

set length(newLength: i32) {
var oldLength = this.length_;
if (isReference<T>()) {
if (!isNullable<T>()) {
if (<u32>newLength > <u32>oldLength) throw new Error(E_HOLEYARRAY);
}
}
ensureSize(changetype<usize>(this), newLength, alignof<T>());
if (isManaged<T>()) { // release no longer used refs
if (oldLength > newLength) {
let dataStart = this.dataStart;
do __release(load<usize>(dataStart + (<usize>--oldLength << alignof<T>())));
while (oldLength > newLength);
// no need to zero memory on shrink -> is zeroed on grow
if (isManaged<T>()) {
if (oldLength > newLength) { // release no longer used refs
let cur = (<usize>newLength << alignof<T>());
let end = (<usize>oldLength << alignof<T>());
do __release(load<usize>(cur));
while ((cur += sizeof<T>()) < end);
} else {
ensureSize(changetype<usize>(this), newLength, alignof<T>());
}
} else {
ensureSize(changetype<usize>(this), newLength, alignof<T>());
}
this.length_ = newLength;
}
Expand All @@ -101,35 +93,30 @@ export class Array<T> extends ArrayBufferView {
}

@operator("[]") private __get(index: i32): T {
if (<u32>index >= <u32>this.length_) throw new RangeError(E_INDEXOUTOFRANGE);
var value = this.__unchecked_get(index);
if (isReference<T>()) {
if (!isNullable<T>()) {
if (<u32>index >= <u32>this.length_) throw new Error(E_HOLEYARRAY);
if (!changetype<usize>(value)) throw new Error(E_HOLEYARRAY);
}
}
if (<u32>index >= <u32>this.dataLength >>> alignof<T>()) throw new RangeError(E_INDEXOUTOFRANGE);
return this.__unchecked_get(index);
return value;
}

@operator("{}") private __unchecked_get(index: i32): T {
@unsafe @operator("{}") private __unchecked_get(index: i32): T {
return load<T>(this.dataStart + (<usize>index << alignof<T>()));
}

@operator("[]=") private __set(index: i32, value: T): void {
var length = this.length_;
if (isReference<T>()) {
if (!isNullable<T>()) {
if (<u32>index > <u32>length) throw new Error(E_HOLEYARRAY);
}
}
ensureSize(changetype<usize>(this), index + 1, alignof<T>());
this.__unchecked_set(index, value);
if (index >= length) this.length_ = index + 1;
if (index >= this.length_) this.length_ = index + 1;
}

@operator("{}=") private __unchecked_set(index: i32, value: T): void {
@unsafe @operator("{}=") private __unchecked_set(index: i32, value: T): void {
if (isManaged<T>()) {
let offset = this.dataStart + (<usize>index << alignof<T>());
let oldRef: usize = load<usize>(offset);
let oldRef = load<usize>(offset);
if (changetype<usize>(value) != oldRef) {
store<usize>(offset, __retain(changetype<usize>(value)));
__release(oldRef);
Expand Down
29 changes: 15 additions & 14 deletions std/assembly/fixedarray.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ export class FixedArray<T> {

constructor(length: i32) {
if (<u32>length > <u32>BLOCK_MAXSIZE >>> alignof<T>()) throw new RangeError(E_INVALIDLENGTH);
if (isReference<T>()) {
if (!isNullable<T>()) {
if (length) throw new Error(E_HOLEYARRAY);
}
}
var outSize = <usize>length << alignof<T>();
var out = __alloc(outSize, idof<FixedArray<T>>());
memory.fill(out, 0, outSize);
Expand All @@ -31,25 +26,31 @@ export class FixedArray<T> {

@operator("[]") private __get(index: i32): T {
if (<u32>index >= <u32>this.length) throw new RangeError(E_INDEXOUTOFRANGE);
return this.__unchecked_get(index);
var value = this.__unchecked_get(index);
if (isReference<T>()) {
if (!isNullable<T>()) {
if (!changetype<usize>(value)) throw new Error(E_HOLEYARRAY);
}
}
return value;
}

@unsafe @operator("{}") private __unchecked_get(index: i32): T {
return load<T>(changetype<usize>(this) + (<usize>index << alignof<T>()));
}

@operator("[]=") private __set(index: i32, value: T): void {
if (<u32>index >= <u32>this.length) throw new RangeError(E_INDEXOUTOFRANGE);
this.__unchecked_set(index, value);
}

@operator("{}") private __unchecked_get(index: i32): T {
return load<T>(changetype<usize>(this) + (<usize>index << alignof<T>()));
}

@operator("{}=") private __unchecked_set(index: i32, value: T): void {
@unsafe @operator("{}=") private __unchecked_set(index: i32, value: T): void {
if (isManaged<T>()) {
let offset = changetype<usize>(this) + (<usize>index << alignof<T>());
let oldValue = load<usize>(offset);
if (changetype<usize>(value) != oldValue) {
let oldRef = load<usize>(offset);
if (changetype<usize>(value) != oldRef) {
store<usize>(offset, __retain(changetype<usize>(value)));
__release(changetype<usize>(oldValue));
__release(changetype<usize>(oldRef));
}
} else {
store<T>(changetype<usize>(this) + (<usize>index << alignof<T>()), value);
Expand Down
4 changes: 1 addition & 3 deletions std/assembly/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1249,13 +1249,11 @@ declare class Array<T> {

/** Tests if a value is an array. */
static isArray<U>(value: any): value is Array<any>;
/** Creates a new array with at least the specified capacity and length zero. */
static create<T>(capacity?: i32): Array<T>;

[key: number]: T;
/** Current length of the array. */
length: i32;
/** Constructs a new array. If length is greater than zero and T is a non-nullable reference, use `Array.create` instead.*/
/** Constructs a new array. */
constructor(capacity?: i32);

fill(value: T, start?: i32, end?: i32): this;
Expand Down
10 changes: 8 additions & 2 deletions std/assembly/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,27 +197,33 @@ export class Map<K,V> {
// FIXME: this is preliminary, needs iterators/closures
var start = changetype<usize>(this.entries);
var size = this.entriesOffset;
var keys = Array.create<K>(size);
var keys = new Array<K>(size);
var length = 0;
for (let i = 0; i < size; ++i) {
let entry = changetype<MapEntry<K,V>>(start + <usize>i * ENTRY_SIZE<K,V>());
if (!(entry.taggedNext & EMPTY)) {
keys.push(entry.key);
++length;
}
}
keys.length = length;
return keys;
}

values(): V[] {
// FIXME: this is preliminary, needs iterators/closures
var start = changetype<usize>(this.entries);
var size = this.entriesOffset;
var values = Array.create<V>(size);
var values = new Array<V>(size);
var length = 0;
for (let i = 0; i < size; ++i) {
let entry = changetype<MapEntry<K,V>>(start + <usize>i * ENTRY_SIZE<K,V>());
if (!(entry.taggedNext & EMPTY)) {
values.push(entry.value);
++length;
}
}
values.length = length;
return values;
}

Expand Down
5 changes: 4 additions & 1 deletion std/assembly/set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,16 @@ export class Set<T> {
// FIXME: this is preliminary, needs iterators/closures
var start = changetype<usize>(this.entries);
var size = this.entriesOffset;
var values = Array.create<T>(size);
var values = new Array<T>(size);
var length = 0;
for (let i = 0; i < size; ++i) {
let entry = changetype<SetEntry<T>>(start + <usize>i * ENTRY_SIZE<T>());
if (!(entry.taggedNext & EMPTY)) {
values.push(entry.key);
++length;
}
}
values.length = length;
return values;
}

Expand Down
1 change: 0 additions & 1 deletion std/portable/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,6 @@ declare class DataView {
declare class Array<T> {

static isArray<U>(value: any): value is Array<any>;
static create<T>(capacity?: i32): Array<T>;

[key: number]: T;
length: i32;
Expand Down
6 changes: 0 additions & 6 deletions std/portable/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,6 @@ globalScope["isArrayLike"] = function isArrayLike(expr) {
&& Math.trunc(expr.length) === expr.length;
};

Array.create = function(capacity) {
var arr = new Array(capacity);
arr.length = 0;
return arr;
};

globalScope["isDefined"] = function isDefined(expr) {
return typeof expr !== "undefined";
}
Expand Down
41 changes: 18 additions & 23 deletions tests/compiler/assert-nonnull.optimized.wat
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
(type $FUNCSIG$v (func))
(import "env" "abort" (func $~lib/builtins/abort (param i32 i32 i32 i32)))
(memory $0 1)
(data (i32.const 8) "^\00\00\00\01\00\00\00\01\00\00\00^\00\00\00E\00l\00e\00m\00e\00n\00t\00 \00t\00y\00p\00e\00 \00m\00u\00s\00t\00 \00b\00e\00 \00n\00u\00l\00l\00a\00b\00l\00e\00 \00i\00f\00 \00a\00r\00r\00a\00y\00 \00i\00s\00 \00h\00o\00l\00e\00y")
(data (i32.const 120) "\1a\00\00\00\01\00\00\00\01\00\00\00\1a\00\00\00~\00l\00i\00b\00/\00a\00r\00r\00a\00y\00.\00t\00s")
(data (i32.const 168) "$\00\00\00\01\00\00\00\01\00\00\00$\00\00\00I\00n\00d\00e\00x\00 \00o\00u\00t\00 \00o\00f\00 \00r\00a\00n\00g\00e")
(data (i32.const 8) "$\00\00\00\01\00\00\00\01\00\00\00$\00\00\00I\00n\00d\00e\00x\00 \00o\00u\00t\00 \00o\00f\00 \00r\00a\00n\00g\00e")
(data (i32.const 64) "\1a\00\00\00\01\00\00\00\01\00\00\00\1a\00\00\00~\00l\00i\00b\00/\00a\00r\00r\00a\00y\00.\00t\00s")
(data (i32.const 112) "^\00\00\00\01\00\00\00\01\00\00\00^\00\00\00E\00l\00e\00m\00e\00n\00t\00 \00t\00y\00p\00e\00 \00m\00u\00s\00t\00 \00b\00e\00 \00n\00u\00l\00l\00a\00b\00l\00e\00 \00i\00f\00 \00a\00r\00r\00a\00y\00 \00i\00s\00 \00h\00o\00l\00e\00y")
(table $0 1 funcref)
(elem (i32.const 0) $null)
(global $~lib/argc (mut i32) (i32.const 0))
Expand Down Expand Up @@ -63,28 +63,25 @@
i32.ge_u
if
i32.const 24
i32.const 136
i32.const 106
i32.const 45
i32.const 80
i32.const 96
i32.const 41
call $~lib/builtins/abort
unreachable
end
i32.const 0
local.get $0
i32.load offset=8
i32.const 2
i32.shr_u
i32.ge_u
call $~lib/array/Array<assert-nonnull/Foo>#__unchecked_get
local.tee $0
i32.eqz
if
i32.const 184
i32.const 136
i32.const 109
i32.const 61
i32.const 128
i32.const 80
i32.const 100
i32.const 39
call $~lib/builtins/abort
unreachable
end
local.get $0
call $~lib/array/Array<assert-nonnull/Foo>#__unchecked_get
)
(func $assert-nonnull/testArr (; 6 ;) (type $FUNCSIG$ii) (param $0 i32) (result i32)
local.get $0
Expand All @@ -98,15 +95,13 @@
(func $~lib/array/Array<assert-nonnull/Foo | null>#__get (; 7 ;) (type $FUNCSIG$ii) (param $0 i32) (result i32)
i32.const 0
local.get $0
i32.load offset=8
i32.const 2
i32.shr_u
i32.load offset=12
i32.ge_u
if
i32.const 184
i32.const 136
i32.const 109
i32.const 61
i32.const 24
i32.const 80
i32.const 96
i32.const 41
call $~lib/builtins/abort
unreachable
end
Expand Down
Loading