Skip to content

In the Canonical ABI, disallow empty types. #218

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

Merged
merged 9 commits into from
Jul 24, 2023

Conversation

sunfishcode
Copy link
Member

In C++, all types must have a non-zero size, so empty structs have size 1. In C, structs with no members are non-standard, though widely supported, with size 0, making them ABI-incompatible with C++. In Go, the spec says "Two distinct zero-size variables may have the same address in memory", and as a user, my understanding is the fact that it says "may" means one isn't guaranteed it's always one way or the other.

To avoid these complexities, modify the Canonical ABI to require that all types have a size of at least one.

@alexcrichton
Copy link
Collaborator

I do sympathize with the complexity here but I feel at least that this isn't something that we can escape from. An alternative to saying everything has a minimum size of 1 is to make the C/C++ bindings generator fancier. This "fancier" is likely a nontrivial chunk of work, however, which I'm presuming is the main motivation for proposing this change (correct me if I'm wrong though).

Going through with this change though languages like Rust will want the opposite, to represent zero-size types as actually zero-size rather than with 1 byte. So in some sense this'll shift the work from a C/C++ generator to a Rust generator.

A possible entirely different alternative to dealing with empty types, however, is to perhaps forbid them. The component model could make empty records, flags, and tuples errors along the same lines of empty variants, unions, and enums are already errors. I'm not sure if there's a concrete use case for empty things other than "it's a nice orthogonal thing to have" and it could offer the ability to defer this complexity of bindings generation to later perhaps.

In C++, all types must have a non-zero size, so empty structs have
size 1. In C, structs with no members are non-standard, though widely
supported, with size 0, making them ABI-incompatible with C++. In Go,
the spec says "Two distinct zero-size variables may have the same
address in memory", and as a user, my understanding is the fact that
it says "may" means one isn't guaranteed it's always one way or the
other.

To avoid these complexities, for now, disallow empty record and flags
types.
@sunfishcode sunfishcode force-pushed the sunfishcode/empty-type-size branch from dfe58f3 to 7054027 Compare July 19, 2023 15:03
@sunfishcode sunfishcode changed the title In the Canonical ABI, give empty types a size of one. In the Canonical ABI, disallow empty types. Jul 19, 2023
@sunfishcode
Copy link
Member Author

That sounds good to me. I've now changed this PR to disallow empty types. If we do this, I'll follow up by submitting PRs to the WASI proposals that currently have empty types.

@alexcrichton
Copy link
Collaborator

Could this also update Binary.md with the extra validation required for these types?

Copy link
Collaborator

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Oh I should also say I'm ok with this at-large too

@sunfishcode
Copy link
Member Author

Ok, I've now change the *s to +s in the AST and binary format.

Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

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

Makes sense, since this is a conservative choice that we can always relax in the future.

sunfishcode and others added 6 commits July 24, 2023 11:25
Co-authored-by: Luke Wagner <mail@lukewagner.name>
Co-authored-by: Luke Wagner <mail@lukewagner.name>
Co-authored-by: Luke Wagner <mail@lukewagner.name>
Co-authored-by: Luke Wagner <mail@lukewagner.name>
Co-authored-by: Luke Wagner <mail@lukewagner.name>
Co-authored-by: Luke Wagner <mail@lukewagner.name>
@lukewagner lukewagner merged commit e6d50af into WebAssembly:main Jul 24, 2023
alexcrichton added a commit to alexcrichton/wasm-tools that referenced this pull request Jul 25, 2023
This updates the component validation phase to account for
WebAssembly/component-model#218
alexcrichton added a commit to bytecodealliance/wasm-tools that referenced this pull request Jul 25, 2023
This updates the component validation phase to account for
WebAssembly/component-model#218
@sunfishcode sunfishcode deleted the sunfishcode/empty-type-size branch August 7, 2023 18:16
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.

3 participants