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.
-->
fitzgen requested cfallin for a review on PR #2203.
cfallin submitted PR Review.
cfallin submitted PR Review.
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
andisa
) and return that from the codegen entry points? That would feel a bit better than holding a bareCodegenError
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?)
abrown submitted PR Review.
abrown created PR Review Comment:
/// Just decode Wasm into Cranelift IR, don't compile it to native code
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.
-->
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.
-->
fitzgen created PR Review Comment:
I think ideally we would make
CodegenError
something that can have context applied to it, the same way that thewast
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_textThis is a bit more than I want to tackle right now, so I'll file a follow up issue instead.
fitzgen submitted PR Review.
fitzgen submitted PR Review.
fitzgen created PR Review Comment:
Filed https://github.com/bytecodealliance/wasmtime/issues/2204
fitzgen merged PR #2203.
Last updated: Dec 23 2024 at 12:05 UTC