Skip to content

Define error handling of MLNamedArrayBufferViews transfer algorithm - #723

Merged
fdwr merged 2 commits into
webmachinelearning:mainfrom
inexorabletash:transfer-errors
Jul 16, 2024
Merged

Define error handling of MLNamedArrayBufferViews transfer algorithm#723
fdwr merged 2 commits into
webmachinelearning:mainfrom
inexorabletash:transfer-errors

Conversation

@inexorabletash

@inexorabletash inexorabletash commented Jul 11, 2024 •
edited by pr-preview Bot
Loading

Copy link
Copy Markdown
Contributor
  • Throw if the passed buffer source is detached.
  • Explicitly rethrow if transfering throws.

The callers already handle failure.

Fixes #351

Preview | Diff

@inexorabletash

inexorabletash commented Jul 11, 2024

Copy link
Copy Markdown
Contributor Author

@reillyeon can you take an initial look at this? It seems like WebIDL changes have made this trivial, but maybe I'm missing something.

@inexorabletash inexorabletash mentioned this pull request Jul 11, 2024
Closed
reillyeon
reillyeon suggested changes Jul 12, 2024
View reviewed changes
Comment thread index.bs
@inexorabletash
inexorabletash marked this pull request as draft July 12, 2024 16:24
@inexorabletash
Define error handling of MLNamedArrayBufferViews transfer algorithm
73e9d64 
* Add a "detachable" algorithm; should move to [WEBIDL] eventually.
* Verify that all views are detachable before detaching anything.

The callers already handle failure.

Fixes #351
@inexorabletash
inexorabletash marked this pull request as ready for review July 12, 2024 17:02
@inexorabletash

inexorabletash commented Jul 12, 2024

Copy link
Copy Markdown
Contributor Author

Okay @reillyeon - now that I've really read the issue, this should match what was proposed there.

My plan is:

  • Land this with a local copy of the BufferSource/detachable algorithm.
  • File a PR against WebIDL referencing this spec (i.e. indicating it's useful)
  • Remove the local copy if/when that merges.

@inexorabletash
inexorabletash requested review from fdwr and huningxin July 12, 2024 18:38
@inexorabletash

inexorabletash commented Jul 12, 2024

Copy link
Copy Markdown
Contributor Author

Thanks @reillyeon !

@huningxin & @fdwr - can you please review?

huningxin
huningxin approved these changes Jul 13, 2024
View reviewed changes

@huningxin huningxin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

fdwr
fdwr approved these changes Jul 16, 2024
View reviewed changes

@fdwr fdwr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fdwr
fdwr merged commit f5fb055 into webmachinelearning:main Jul 16, 2024
github-actions Bot added a commit that referenced this pull request Jul 16, 2024
@inexorabletash @github-actions
Define error handling of MLNamedArrayBufferViews transfer algorithm (#…
d6dd1ec 
…723)

SHA: f5fb055
Reason: push, by fdwr

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@inexorabletash
inexorabletash deleted the transfer-errors branch July 17, 2024 16:37
@dontcallmedom dontcallmedom mentioned this pull request Jan 15, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@huningxin huningxin huningxin approved these changes
@fdwr fdwr fdwr approved these changes
+1 more reviewer
@reillyeon reillyeon reillyeon approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Need to define error handling of MLNamedArrayBufferViews transfer algorithm

4 participants

@inexorabletash @reillyeon @huningxin @fdwr