Skip to content
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

[browser] Fixed the 3rd argument for TypedArray.fill() #82480

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

shin1m
Copy link
Contributor

@shin1m shin1m commented Feb 22, 2023

According to
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/fill
, TypedArray.fill() expects end as the 3rd argument.

`TypedArray.fill()` expects `end` as the 3rd argument.
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Feb 22, 2023
@pavelsavara pavelsavara added the arch-wasm WebAssembly architecture label Feb 22, 2023
@ghost
Copy link

ghost commented Feb 22, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

According to
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/fill
, TypedArray.fill() expects end as the 3rd argument.

Author: shin1m
Assignees: -
Labels:

arch-wasm, community-contribution

Milestone: -

@pavelsavara pavelsavara added this to the 8.0.0 milestone Feb 22, 2023
@pavelsavara
Copy link
Member

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara changed the title Fixed the 3rd argument for TypedArray.fill(). [browser] Fixed the 3rd argument for TypedArray.fill() Feb 22, 2023
@kg
Copy link
Contributor

kg commented Feb 23, 2023

Hm, you're right about the mdn docs, but the change caused a ton of test failures. Maybe it's some sort of infrastructure problem. I'll re-run the tests tomorrow.

@pavelsavara
Copy link
Member

Strange, this is on chrome

fail: ReferenceError: SharedArrayBuffer is not defined
                     at http://127.0.0.1:33047/dotnet.js:12:14138
                     at Object.create (http://127.0.0.1:33047/dotnet.js:3:215325)
                     at run (http://127.0.0.1:33047/test-main.js:293:36)

@pavelsavara pavelsavara removed their assignment Feb 23, 2023
@kg kg merged commit 5ba867f into dotnet:main Feb 24, 2023
@lewing
Copy link
Member

lewing commented Feb 24, 2023

/backport to release/7.0

@github-actions
Copy link
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/4258087063

@lewing
Copy link
Member

lewing commented Feb 24, 2023

@shin1m thank you so much for finding this and contributing a fix.

@github-actions
Copy link
Contributor

@lewing backporting to release/7.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Fixed the 3rd argument for TypedArray.fill().
Using index info to reconstruct a base tree...
M	src/mono/wasm/runtime/memory.ts
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/runtime/memory.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/memory.ts
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fixed the 3rd argument for TypedArray.fill().
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions
Copy link
Contributor

@lewing an error occurred while backporting to release/7.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

kg added a commit to kg/runtime that referenced this pull request Feb 24, 2023
@shin1m
Copy link
Contributor Author

shin1m commented Feb 24, 2023

Thank you for reviewing.

@shin1m shin1m deleted the fix-_zero_region branch February 24, 2023 12:47
carlossanlop pushed a commit that referenced this pull request Mar 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants