Skip to content

Add accessor support - #1726

Draft
bvisness wants to merge 5 commits into
bytecodealliance:mainfrom
bvisness:getters-setters
Draft

bvisness wants to merge 5 commits into
bytecodealliance:mainfrom
bvisness:getters-setters

Conversation

@bvisness

Copy link
Copy Markdown

Currently implemented for C, C++, and Rust. Other languages have todo!() in several switch statements. This is probably not even actually sufficient on its own but tests are being a real pain locally so I want to start running some things here.

We might need to propagate updates into wasm-component-ld and friends for all of them to run...

@bvisness

Copy link
Copy Markdown
Author

Yeah, there's quite a lot of this:

        Error: failed to parse WebAssembly module
        
        Caused by:
            export name `[method][get]blob.position` is not a valid extern name
        `[get]blob` is not in kebab case (at offset 0x6e4)

I guess this means more wasm-tools version bumps are in order.

(fwiw, I determined that the case I was worried about on Zulip was not, in fact, a bug, so as of now I am not in fact aware of any bugs of mine in wasm-tools.)

@alexcrichton alexcrichton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Most of the errors to me look like it's wasmtime not supporting getters/setters yet. I think that'd be fixable by updating Wasmtime to plumb the feature through to the CLI and then using the dev release here to run tests (and the test will probably need some sort of configuration to pass the right -W flag or something like that)

For building the theory for most targets is to emit a core wasm module and then use the tooling via dependencies to do what wasm-component-ld-would-have-done, precisely to handle this issue where new features are in development. C/Rust for example should both pass --skip-wit-component to the linker. It looks like C++ here might not be doing that, but that should in theory be a pretty small change to crates/test/src/cpp.rs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

generated code?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants