Update to use witx tagged unions #165

pchickey · 2020-02-07T02:18:25Z

Implements changes proposed in WebAssembly/WASI#220.

Changes to generated code for union types

`$subscription`

Current Master

/**
 * The contents of a $subscription.
 */
typedef union __wasi_subscription_u_t {
    /**
     * When type is `eventtype::clock`:
     */
    __wasi_subscription_clock_t clock;

    /**
     * When type is `eventtype::fd_read` or `eventtype::fd_write`:
     */
    __wasi_subscription_fd_readwrite_t fd_readwrite;

} __wasi_subscription_u_t;

_Static_assert(sizeof(__wasi_subscription_u_t) == 32, "witx calculated size");
_Static_assert(_Alignof(__wasi_subscription_u_t) == 8, "witx calculated align");

/**
 * Subscription to an event.
 */
typedef struct __wasi_subscription_t {
    /**
     * User-provided value that is attached to the subscription in the
     * implementation and returned through `event::userdata`.
     */
    __wasi_userdata_t userdata;

    /**
     * The type of the event to which to subscribe.
     */
    __wasi_eventtype_t type;

    /**
     * The contents of the subscription.
     */
    __wasi_subscription_u_t u;

} __wasi_subscription_t;

_Static_assert(sizeof(__wasi_subscription_t) == 48, "witx calculated size");
_Static_assert(_Alignof(__wasi_subscription_t) == 8, "witx calculated align");
_Static_assert(offsetof(__wasi_subscription_t, userdata) == 0, "witx calculated offset");
_Static_assert(offsetof(__wasi_subscription_t, type) == 8, "witx calculated offset");
_Static_assert(offsetof(__wasi_subscription_t, u) == 16, "witx calculated offset");

Tagged Unions

/**
 * The contents of a $subscription.
 */
typedef union __wasi_subscription_u_u_t {
    __wasi_subscription_clock_t clock;
    __wasi_subscription_fd_readwrite_t fd_read;
    __wasi_subscription_fd_readwrite_t fd_write;
} __wasi_subscription_u_u_t;
typedef struct __wasi_subscription_u_t {
    __wasi_eventtype_t tag;
    __wasi_subscription_u_u_t u;
} __wasi_subscription_u_t;

_Static_assert(sizeof(__wasi_subscription_u_t) == 40, "witx calculated size");
_Static_assert(_Alignof(__wasi_subscription_u_t) == 8, "witx calculated align");
_Static_assert(offsetof(__wasi_subscription_u_t, u) == 8, "witx calculated union offset");
_Static_assert(sizeof(__wasi_subscription_u_u_t) == 32, "witx calculated union size");
_Static_assert(_Alignof(__wasi_subscription_u_u_t) == 8, "witx calculated union align");

/**
 * Subscription to an event.
 */
typedef struct __wasi_subscription_t {
    /**
     * User-provided value that is attached to the subscription in the
     * implementation and returned through `event::userdata`.
     */
    __wasi_userdata_t userdata;

    /**
     * The type of the event to which to subscribe, and its contents
     */
    __wasi_subscription_u_t u;

} __wasi_subscription_t;

_Static_assert(sizeof(__wasi_subscription_t) == 48, "witx calculated size");
_Static_assert(_Alignof(__wasi_subscription_t) == 8, "witx calculated align");
_Static_assert(offsetof(__wasi_subscription_t, userdata) == 0, "witx calculated offset");
_Static_assert(offsetof(__wasi_subscription_t, u) == 8, "witx calculated offset");

`$event`

Current Master

/**
 * The contents of an $event.
 */
typedef union __wasi_event_u_t {
    /**
     * When type is `eventtype::fd_read` or `eventtype::fd_write`:
     */
    __wasi_event_fd_readwrite_t fd_readwrite;

} __wasi_event_u_t;

_Static_assert(sizeof(__wasi_event_u_t) == 16, "witx calculated size");
_Static_assert(_Alignof(__wasi_event_u_t) == 8, "witx calculated align");

/**
 * An event that occurred.
 */
typedef struct __wasi_event_t {
    /**
     * User-provided value that got attached to `subscription::userdata`.
     */
    __wasi_userdata_t userdata;

    /**
     * If non-zero, an error that occurred while processing the subscription request.
     */
    __wasi_errno_t error;

    /**
     * The type of the event that occurred.
     */
    __wasi_eventtype_t type;

    /**
     * The contents of the event.
     */
    __wasi_event_u_t u;

} __wasi_event_t;

_Static_assert(sizeof(__wasi_event_t) == 32, "witx calculated size");
_Static_assert(_Alignof(__wasi_event_t) == 8, "witx calculated align");
_Static_assert(offsetof(__wasi_event_t, userdata) == 0, "witx calculated offset");
_Static_assert(offsetof(__wasi_event_t, error) == 8, "witx calculated offset");
_Static_assert(offsetof(__wasi_event_t, type) == 10, "witx calculated offset");
_Static_assert(offsetof(__wasi_event_t, u) == 16, "witx calculated offset");

Tagged Unions

/**
 * An event that occurred.
 */
typedef struct __wasi_event_t {
    /**
     * User-provided value that got attached to `subscription::userdata`.
     */
    __wasi_userdata_t userdata;

    /**
     * If non-zero, an error that occurred while processing the subscription request.
     */
    __wasi_errno_t error;

    /**
     * The type of event that occured
     */
    __wasi_eventtype_t type;

    /**
     * The contents of the event, if it is an `eventtype::fd_read` or
     * `eventtype::fd_write`. `eventtype::clock` events ignore this field.
     */
    __wasi_event_fd_readwrite_t fd_readwrite;

} __wasi_event_t;

_Static_assert(sizeof(__wasi_event_t) == 32, "witx calculated size");
_Static_assert(_Alignof(__wasi_event_t) == 8, "witx calculated align");
_Static_assert(offsetof(__wasi_event_t, userdata) == 0, "witx calculated offset");
_Static_assert(offsetof(__wasi_event_t, error) == 8, "witx calculated offset");
_Static_assert(offsetof(__wasi_event_t, type) == 10, "witx calculated offset");
_Static_assert(offsetof(__wasi_event_t, fd_readwrite) == 16, "witx calculated offset");

`$prestat`

Current Master

/**
 * The contents of an $prestat.
 */
typedef union __wasi_prestat_u_t {
    /**
     * When type is `preopentype::dir`:
     */
    __wasi_prestat_dir_t dir;

} __wasi_prestat_u_t;

_Static_assert(sizeof(__wasi_prestat_u_t) == 4, "witx calculated size");
_Static_assert(_Alignof(__wasi_prestat_u_t) == 4, "witx calculated align");

/**
 * Information about a pre-opened capability.
 */
typedef struct __wasi_prestat_t {
    /**
     * The type of the pre-opened capability.
     */
    __wasi_preopentype_t pr_type;

    /**
     * The contents of the information.
     */
    __wasi_prestat_u_t u;

} __wasi_prestat_t;

_Static_assert(sizeof(__wasi_prestat_t) == 8, "witx calculated size");
_Static_assert(_Alignof(__wasi_prestat_t) == 4, "witx calculated align");
_Static_assert(offsetof(__wasi_prestat_t, pr_type) == 0, "witx calculated offset");
_Static_assert(offsetof(__wasi_prestat_t, u) == 4, "witx calculated offset");

Tagged Unions

/**
 * Information about a pre-opened capability.
 */
typedef union __wasi_prestat_u_t {
    __wasi_prestat_dir_t dir;
} __wasi_prestat_u_t;
typedef struct __wasi_prestat_t {
    __wasi_preopentype_t tag;
    __wasi_prestat_u_t u;
} __wasi_prestat_t;

_Static_assert(sizeof(__wasi_prestat_t) == 8, "witx calculated size");
_Static_assert(_Alignof(__wasi_prestat_t) == 4, "witx calculated align");
_Static_assert(offsetof(__wasi_prestat_t, u) == 4, "witx calculated union offset");
_Static_assert(sizeof(__wasi_prestat_u_t) == 4, "witx calculated union size");
_Static_assert(_Alignof(__wasi_prestat_u_t) == 4, "witx calculated union align");

pchickey · 2020-02-14T01:24:03Z

I don't understand why the Azure windows build failed but the github windows build succeeded. Do we need to keep the Azure pipelines around anymore?

pchickey · 2020-02-18T20:40:40Z

Rebased on master, and submodule points at WASI master - ready to go @sunfishcode

sunfishcode · 2020-02-18T22:48:33Z

This PR contains a change to remove __wasilibc_unmodified_upstream markers. I discussed this with @pchickey offline, but I want to mention it here too. These markers were more useful when they were simpler and when we were first setting up wasi-libc and figuring out what parts of cloudlibc we needed and which we didn't. But as WASI has evolved the __wasilibc_unmodified_upstream markers have become increasingly complex and awkward to maintain, and we have a clearer picture of what wasi-libc's role is now, so it seems a good time to start dropping them.

The Azure Pipeline rules build wasi-libc, which the Github Action rules don't do yet. If we moved that over to Github Actions, then we could drop the Azure Pipelines.

@pchickey And just to confirm, this change doesn't change the ABI, right?

sbc100 · 2020-02-18T22:52:20Z

Are the markers still useful for the musl parts of the code (where we will most likely want to periodically update)?

pchickey · 2020-02-19T01:39:34Z

The markers may be useful in musl (libc-top-half) but as we've made more and more changes in the witx files and code generator, I agree with Dan that they're not useful in the bottom half anymore.

Correct, there is no ABI change, thats what you're supposed to be able to convince yourself from the changes summarized in the description.

pchickey · 2020-02-19T19:09:26Z

#168 merges this PR and #167, demonstrating that the windows build issue in azure here is something thats not the fault of this PR.

we decided to abandon the upstream code guarded by #ifdef __wasilibc_unmodified_upstream // non-anonymous unions because these changes are sprawling and those guards are of diminishing importance

fake required arguments are no good

pchickey · 2020-02-19T22:13:54Z

rebased after #167 merged to master

sunfishcode

I agree; let's keep the __wasilibc_unmodified_upstream markers in musl for now, but removing them in libc-bottom-half now makes sense.

pchickey force-pushed the pch/tagged_unions branch from a2228d4 to 8443958 Compare February 12, 2020 17:50

pchickey changed the title ~~Pch/tagged unions~~ Update to use witx tagged unions Feb 14, 2020

pchickey mentioned this pull request Feb 14, 2020

Change witx unions to be tagged WebAssembly/WASI#220

Merged

pchickey marked this pull request as ready for review February 14, 2020 01:21

pchickey requested a review from sunfishcode February 14, 2020 01:23

pchickey force-pushed the pch/tagged_unions branch from 5785f39 to f3c4f53 Compare February 18, 2020 20:37

cjihrig mentioned this pull request Feb 18, 2020

Changes to WASI spec in snapshot_preview2 nodejs/uvwasi#59

Closed

9 tasks

pchickey mentioned this pull request Feb 19, 2020

[TEST ONLY] Pch/tagged unions plus ghactions #168

Closed

pchickey added 10 commits February 19, 2020 14:13

wasi-headers: unions are tagged now

a15e09a

WASI: tagged unions PR wip

e66c5a4

wasi/api.h: regenerated with tagged unions

605ef25

cloudlibc & libpreopen: changes for tagged unions

4262176

we decided to abandon the upstream code guarded by #ifdef __wasilibc_unmodified_upstream // non-anonymous unions because these changes are sprawling and those guards are of diminishing importance

update to WASI where snashot event_u flattened to struct

3cfe4c8

libc: change to flattened event struct

b596978

minor change in union AST in wasi- tag is namedtype

f4a315a

main: get more useful help by requiring subcommand

2b7d17b

fake required arguments are no good

rebased wasi repo branch

a23d177

point at WASI with tagged unions merged into master

0f50362

pchickey force-pushed the pch/tagged_unions branch from f3c4f53 to 0f50362 Compare February 19, 2020 22:13

sunfishcode approved these changes Feb 24, 2020

View reviewed changes

pchickey merged commit e1149ab into master Feb 24, 2020

pchickey deleted the pch/tagged_unions branch February 24, 2020 19:36

pchickey mentioned this pull request Feb 17, 2021

Take another step towards interface types WebAssembly/WASI#395

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to use witx tagged unions #165

Update to use witx tagged unions #165

pchickey commented Feb 7, 2020 •

edited

Loading

pchickey commented Feb 14, 2020

pchickey commented Feb 18, 2020

sunfishcode commented Feb 18, 2020

sbc100 commented Feb 18, 2020

pchickey commented Feb 19, 2020

pchickey commented Feb 19, 2020

pchickey commented Feb 19, 2020

sunfishcode left a comment

Update to use witx tagged unions #165

Update to use witx tagged unions #165

Conversation

pchickey commented Feb 7, 2020 • edited Loading

Changes to generated code for union types

$subscription

Current Master

Tagged Unions

$event

Current Master

Tagged Unions

$prestat

Current Master

Tagged Unions

pchickey commented Feb 14, 2020

pchickey commented Feb 18, 2020

sunfishcode commented Feb 18, 2020

sbc100 commented Feb 18, 2020

pchickey commented Feb 19, 2020

pchickey commented Feb 19, 2020

pchickey commented Feb 19, 2020

sunfishcode left a comment

Choose a reason for hiding this comment

pchickey commented Feb 7, 2020 •

edited

Loading

`$subscription`

`$event`

`$prestat`