-
-
Notifications
You must be signed in to change notification settings - Fork 677
Add TypedArray#fill #274
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
Add TypedArray#fill #274
Conversation
std/assembly/internal/typedarray.ts
Outdated
var buffer = this.buffer; | ||
var byteOffset = this.byteOffset; | ||
var len = this.length; | ||
start = start < 0 ? max(len + start, byteOffset >> alignof<T>()) : min(start, len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about lower and upper limits. Need more tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be possible to ignore byteOffset up to the point where the memory.fill
operation is used, like with a normal array. That's just the offset into the backing buffer when accessing it internally.
std/assembly/internal/typedarray.ts
Outdated
memory.fill( | ||
changetype<usize>(buffer) + start + byteOffset + AB_HEADER_SIZE, | ||
<u8>value, | ||
<usize>(end - start) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's necessary to make sure that end >= start
, otherwise this might overflow and copy like all the memory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g. fill(0xff, 1, 0)
on an Uint8Array
: start = 1, end = 0, <usize>(0 - 1) = 0xffffffff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah makes sense
Looking good with the conflicts resolved |
Thank you! |
No description provided.