Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create witx-type-representation.md #318

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

monoclex
Copy link

@monoclex monoclex commented Sep 6, 2020

This document summarizes the types that can be found in the phaser snapshots. The goal of this is that it should be a one-stop shop for people looking to implement ABI compatibility with certain types. This PR was created to resolve one of the issues brought up with #316

Points of improvement before merging:

  • More condensed representation of whole bytes? subscription_u would be hard to read on mobile
  • Pointer needs more elaboration
  • Are the images exactly accurate to the positions of bytes?
  • Will it answer all ABI protocol related questions? Are there odd edge cases?

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

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

Cool! This looks like a great start documenting a part of witx which is currently undocumented!

Could you add a link to this from here? https://github.com/WebAssembly/WASI/blob/master/docs/witx.md

One other thing which may be useful to mention in the intro; the representations of these types are exposed when used with linear memory, however witx is also meant to be used in the future in non-linear-memory contexts as well, in which case the representation is opaque.

design/witx-type-representation.md Outdated Show resolved Hide resolved
design/witx-type-representation.md Outdated Show resolved Hide resolved
design/witx-type-representation.md Outdated Show resolved Hide resolved
design/witx-type-representation.md Outdated Show resolved Hide resolved
design/witx-type-representation.md Outdated Show resolved Hide resolved
design/witx-type-representation.md Show resolved Hide resolved
design/witx-type-representation.md Outdated Show resolved Hide resolved
Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Nice!

I'm sure folks will find this useful.

I wonder if it makes sense to integrate this into the existing docs somewhere? Or at least be linked to from witx-generated docs? @pchickey

design/witx-type-representation.md Outdated Show resolved Hide resolved
design/witx-type-representation.md Show resolved Hide resolved
design/witx-type-representation.md Outdated Show resolved Hide resolved
design/witx-type-representation.md Outdated Show resolved Hide resolved
design/witx-type-representation.md Outdated Show resolved Hide resolved
design/witx-type-representation.md Outdated Show resolved Hide resolved
@monoclex
Copy link
Author

my bad on taking forever lol

Base automatically changed from master to main January 19, 2021 23:08
iovec

buf: Pointer<u8> @ offset 0
buf_len: size @ offset 4
Copy link
Contributor

Choose a reason for hiding this comment

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

The offset may be 4 or 8 depends on sizeof pointer

Copy link
Author

@monoclex monoclex May 11, 2021

Choose a reason for hiding this comment

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

It seems that the docs don't mention this distinction - I'm unsure if it's within scope of this PR to mention it if the docs don't.

@monoclex monoclex marked this pull request as draft May 11, 2021 05:37
@monoclex monoclex force-pushed the SirJosh3917-witx-type-representation branch from ae8b5ac to 0c4b9ec Compare May 11, 2021 05:42
@monoclex monoclex force-pushed the SirJosh3917-witx-type-representation branch from 0c4b9ec to ca38d96 Compare May 11, 2021 05:46
@monoclex
Copy link
Author

Apologies for it having been a while since I've worked on this. I tried to update the terminology to be more in-line with what's currently in use, but I might've glossed something over.

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.

4 participants