Stream: git-wasmtime

Topic: wasmtime / PR #11467 Refactor exception objects to share ...

Wasmtime GitHub notifications bot (Aug 19 2025 at 22:27):

cfallin opened PR #11467 from cfallin:wasm-exception-object-refactor to bytecodealliance:main:

This PR updates the implementation of Wasm exception objects to share more layout-computation code with structs, except for a single fixed tag slot. The former is simply a nice refactor, and the latter is in preparation for full exception support, which will require accessing an exception's tag without knowing its layout dynamically at runtime.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Aug 19 2025 at 22:27):

cfallin requested fitzgen for a review on PR #11467.

Wasmtime GitHub notifications bot (Aug 19 2025 at 22:27):

cfallin requested wasmtime-core-reviewers for a review on PR #11467.

Wasmtime GitHub notifications bot (Aug 19 2025 at 22:28):

cfallin commented on PR #11467:

This has been pulled out of #11326 for separate landing.

Wasmtime GitHub notifications bot (Aug 19 2025 at 22:30):

cfallin updated PR #11467.

Wasmtime GitHub notifications bot (Aug 19 2025 at 23:45):

github-actions[bot] commented on PR #11467:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:ref-types"

Thus the following users have been cc'd because of the following labels:

• fitzgen: wasmtime:ref-types

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Aug 20 2025 at 17:11):

fitzgen submitted PR review:

LGTM, thanks for splitting things up and making that PR a little smaller!

Wasmtime GitHub notifications bot (Aug 20 2025 at 17:11):

fitzgen created PR review comment:

Does it make sense to still have GcExceptionLayout but have it just be a newtype wrapper over GcStructLayout? That way we can still use helpers that want to take a GcStructLayout argument but we also have some tidiness with a dedicated type and can remove the is_exception: bool field from GcStructLayout.

Not sure this is actually a useful suggestion yet, going to look at a little bit more of the PR...

Wasmtime GitHub notifications bot (Aug 20 2025 at 17:11):

fitzgen created PR review comment:

This is not doing any hash consing because tags are nominal, correct? Can we add that note in a comment somewhere?

And also something about how this is only for our implementation-specific ModuleInternedTypeIndex and does not affect the the Wasm-level types index space. That is, these aren't types at the wasm-level, but we treat them as types for convenience in our implementation and this is in service of that.

Wasmtime GitHub notifications bot (Aug 20 2025 at 17:11):

fitzgen created PR review comment:

FWIW, if you have appetite for a little clean up, I think these methods and the existing ones above could use the unwrap_* methods on WasmCompositeType to dedupe a little bit of code and then become something like

        let comp_ty = &self.types[interned_ty].composite_type;
        if comp_ty.shared {
            return Err(wasm_unsupported!(
                "shared functions are not yet implemented"
            ));
        }
        Ok(comp_ty.unwrap_func())

Although I must also say (and maybe this is my fault) I am also confused by the combination of error-returning sometimes and panicking other times. Seems like we could move everything to one or the other, depending on usage.

Feel free to deal with both or one or none of these things in this PR, as you see fit.

Wasmtime GitHub notifications bot (Aug 20 2025 at 17:11):

fitzgen created PR review comment:

Okay I guess it doesn't really make sense to remove the is_exception: bool field and do that newtype thing. Or else we would have to thread an is_exception: bool argument through here which seems like 6 of one and half a dozen of another. Nevermind!

Wasmtime GitHub notifications bot (Aug 20 2025 at 18:16):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 20 2025 at 18:16):

cfallin created PR review comment:

Yep, this seemed like the most reasonable way to do it to me...

Wasmtime GitHub notifications bot (Aug 20 2025 at 18:16):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 20 2025 at 18:16):

cfallin created PR review comment:

(resolving as per below)

Wasmtime GitHub notifications bot (Aug 20 2025 at 18:23):

cfallin updated PR #11467.

Wasmtime GitHub notifications bot (Aug 20 2025 at 18:23):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 20 2025 at 18:23):

cfallin created PR review comment:

Did the simple cleanup as suggested at least -- good call.

Wasmtime GitHub notifications bot (Aug 20 2025 at 18:24):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 20 2025 at 18:24):

cfallin created PR review comment:

Ah, great point, actually we can hash-cons -- tags are nominal, but that is covered by the dynamic tag-instance reference in an exception object; the Wasmtime exception-object type describes only the layout, which can be structural / shared across all exceptions with the same layout. I've added hash-consing here, as well as comments about this (and about the Wasmtime-specific nature of the exception types).

Wasmtime GitHub notifications bot (Aug 20 2025 at 18:24):

cfallin has enabled auto merge for PR #11467.

Wasmtime GitHub notifications bot (Aug 20 2025 at 19:00):

cfallin merged PR #11467.

Last updated: Dec 06 2025 at 06:05 UTC