Stream: wasi

Topic: wasi-libc / gen-headers changes


view this post on Zulip Pat Hickey (Feb 06 2020 at 20:47):

mostly for @Dan Gohman - in wasi-libc i need to adjust various C files in mild ways for the changes in the C headers generated to implement the tagged unions

view this post on Zulip Dan Gohman (Feb 06 2020 at 20:48):

ok

view this post on Zulip Pat Hickey (Feb 06 2020 at 20:48):

https://github.com/CraneStation/wasi-libc/blob/master/libc-bottom-half/cloudlibc/src/libc/poll/poll.c#L25
in this one, theres some //non-anonymous union changes marked off with the __wasilibc_unmodified_upstream macros

WASI libc implementation for WebAssembly. Contribute to CraneStation/wasi-libc development by creating an account on GitHub.

view this post on Zulip Pat Hickey (Feb 06 2020 at 20:48):

my impression was that upstream was cloudlibc, which wouldnt know about any of these wasi macros/structs, right?

view this post on Zulip Pat Hickey (Feb 06 2020 at 20:49):

my question is basically, do i keep making changes in the else case of these macros and move other fields that need changes into the if case and else case

view this post on Zulip Dan Gohman (Feb 06 2020 at 20:50):

cloudabi is renamed to wasi unconditionally everywhere, and then we mark further changes with #ifdef __wasilibc_unmodified_upstream

view this post on Zulip Pat Hickey (Feb 06 2020 at 20:50):

or do we just erase the macro leaving only the else case, because we've drifted sufficiently far from cloudlibc that tracking the changes in macros isnt needed

view this post on Zulip Pat Hickey (Feb 06 2020 at 20:50):

ah, ok.

view this post on Zulip Pat Hickey (Feb 06 2020 at 20:51):

so then i should name the fields named ...->type into the if case and the part where theyre renamed as ...->u.tag in the else case.

view this post on Zulip Dan Gohman (Feb 06 2020 at 20:52):

In general, try to leave the unmodified upstream version unmodified, including the original field names.

view this post on Zulip Dan Gohman (Feb 06 2020 at 20:55):

Although, I'd be open to just removing all the wasilibc-unmodified_upstream changes entirely. They're increasingly turning out to be more of a hassle than they're worth.

view this post on Zulip Dan Gohman (Feb 06 2020 at 20:55):

s/changes/code blocks/

view this post on Zulip Pat Hickey (Feb 06 2020 at 20:56):

ok. i think they can be maintained without a ton of trouble for this transformation, but im also not against just getting rid of them

view this post on Zulip Pat Hickey (Feb 06 2020 at 20:59):

poll.diff heres one example diff (just the first one ive done)

view this post on Zulip Dan Gohman (Feb 06 2020 at 21:02):

That looks good to me, though what's going on in the last part?

view this post on Zulip Pat Hickey (Feb 06 2020 at 21:06):

(btw i made a minor mistake in the witx and im making changes to far more stuff outside of those ifdefs than before, so im leaning towards deleting at least the // non-anonymous union ones)

view this post on Zulip Pat Hickey (Feb 06 2020 at 21:06):

in that last part, theres a precondition that the tag is either fd_read or fd_write, so the existing code uses the same union variant for each of those

view this post on Zulip Pat Hickey (Feb 06 2020 at 21:07):

the new code needs to use a separate union variant whether the tag is read or write (not strictly necessary right now, since the two variants are equal, but if we added a field to one and not the other, it would be incorrect and no way to catch it)

view this post on Zulip Pat Hickey (Feb 06 2020 at 21:07):

so we dispatch on the tag, then use the variant accordingly

view this post on Zulip Pat Hickey (Feb 06 2020 at 21:07):

and i unfolded the other ternary into that same if/else

view this post on Zulip Dan Gohman (Feb 06 2020 at 21:08):

Ah, ok.

view this post on Zulip Pat Hickey (Feb 06 2020 at 21:10):

anyhow im now of the opinion that the code will be a lot cleaner (easier to read and maintain) if we delete the unmodified case (and dont continue maintaining the unmodified case) for these unions

view this post on Zulip Dan Gohman (Feb 06 2020 at 21:12):

I think we can go ahead and do that.

view this post on Zulip Dan Gohman (Feb 06 2020 at 21:17):

As WASI has been evolving, maintaining these extra code paths has gotten increasingly awkward.

view this post on Zulip Pat Hickey (Feb 06 2020 at 21:18):

yep. and since theres no CI that checks that the other side of the #ifdef even works, we're relying on discipline to keep everything guarded properly


Last updated: Dec 23 2024 at 12:05 UTC