Pat Hickey (Feb 06 2020 at 20:47): 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
Dan Gohman (Feb 06 2020 at 20:48):ok
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
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?
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
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
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
Pat Hickey (Feb 06 2020 at 20:50):ah, ok.
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.
Dan Gohman (Feb 06 2020 at 20:52):In general, try to leave the unmodified upstream version unmodified, including the original field names.
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.
Dan Gohman (Feb 06 2020 at 20:55):s/changes/code blocks/
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
Pat Hickey (Feb 06 2020 at 20:59):poll.diff heres one example diff (just the first one ive done)
Dan Gohman (Feb 06 2020 at 21:02):That looks good to me, though what's going on in the last part?
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)
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
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)
Pat Hickey (Feb 06 2020 at 21:07):so we dispatch on the tag, then use the variant accordingly
Pat Hickey (Feb 06 2020 at 21:07):and i unfolded the other ternary into that same if/else
Dan Gohman (Feb 06 2020 at 21:08):Ah, ok.
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
Dan Gohman (Feb 06 2020 at 21:12):I think we can go ahead and do that.
Dan Gohman (Feb 06 2020 at 21:17):As WASI has been evolving, maintaining these extra code paths has gotten increasingly awkward.
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: Oct 23 2024 at 20:03 UTC