Skip to content

Comments

Define error handling of MLNamedArrayBufferViews transfer algorithm#723

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

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

Conversation

@inexorabletash
Copy link
Contributor

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

  • Throw if the passed buffer source is detached.
  • Explicitly rethrow if transfering throws.

The callers already handle failure.

Fixes #351

Preview | Diff

@inexorabletash
Copy link
Contributor Author

inexorabletash commented Jul 11, 2024

@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
@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
Copy link
Contributor Author

inexorabletash commented Jul 12, 2024

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
Copy link
Contributor Author

inexorabletash commented Jul 12, 2024

Thanks @reillyeon !

@huningxin & @fdwr - can you please review?

huningxin
huningxin approved these changes Jul 13, 2024
View reviewed changes
Copy link
Contributor

@huningxin huningxin left a comment

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
Copy link
Collaborator

@fdwr fdwr left a comment

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