Stream: git-wasmtime

Topic: wasmtime / PR #2203 Clean up `clif-util`: use anyhow and ...

Wasmtime GitHub notifications bot (Sep 15 2020 at 01:41):

fitzgen opened PR #2203 from clean-up-clif-util to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Sep 15 2020 at 01:41):

fitzgen requested cfallin for a review on PR #2203.

Wasmtime GitHub notifications bot (Sep 15 2020 at 04:52):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Sep 15 2020 at 04:52):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Sep 15 2020 at 04:52):

cfallin created PR Review Comment:

At a slight risk of overengineering, I wonder if we could do something a bit nicer here:

Could we, for example, return a CodegenErrorWithContext that wraps the error with the necessary context to interpret it (func and isa) and return that from the codegen entry points? That would feel a bit better than holding a bare CodegenError and recombining it with its context here.

(On second thought, this would force us to plumb lifetime params through in order to carry the func/isa borrows -- maybe too verbose. Maybe there's something else in that spirit though. Needs more thought?)

Wasmtime GitHub notifications bot (Sep 15 2020 at 16:34):

abrown submitted PR Review.

Wasmtime GitHub notifications bot (Sep 15 2020 at 16:34):

abrown created PR Review Comment:

    /// Just decode Wasm into Cranelift IR, don't compile it to native code

Wasmtime GitHub notifications bot (Sep 15 2020 at 16:42):

fitzgen updated PR #2203 from clean-up-clif-util to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Sep 15 2020 at 16:44):

fitzgen updated PR #2203 from clean-up-clif-util to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Sep 15 2020 at 16:47):

fitzgen created PR Review Comment:

I think ideally we would make CodegenError something that can have context applied to it, the same way that the wast crate's error type can have source text and filename context applied to it. This would mutate the error, switching it to the string-y form, without holding borrows to the context itself.

https://docs.rs/wast/24.0.0/wast/struct.Error.html#method.set_path
https://docs.rs/wast/24.0.0/wast/struct.Error.html#method.set_text

This is a bit more than I want to tackle right now, so I'll file a follow up issue instead.

Wasmtime GitHub notifications bot (Sep 15 2020 at 16:47):

fitzgen submitted PR Review.

Wasmtime GitHub notifications bot (Sep 15 2020 at 16:51):

fitzgen submitted PR Review.

Wasmtime GitHub notifications bot (Sep 15 2020 at 16:51):

fitzgen created PR Review Comment:

Filed https://github.com/bytecodealliance/wasmtime/issues/2204

Wasmtime GitHub notifications bot (Sep 15 2020 at 17:22):

fitzgen merged PR #2203.

Last updated: Nov 22 2024 at 16:03 UTC