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

GuiAction with a well-defined initial state #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

m4c0
Copy link

@m4c0 m4c0 commented May 24, 2024

I was watching your video and I noticed you want to avoid uninitialised state in GuiAction. You had to use MaybeUninit in Rust for Rust reasons, but that introduces an UB in your code.

I think the trick you wanted is an empty struct as the default enum state in GuiAction. It is as simple as struct {} none;, but I added as an explicit struct because it is easier to remember and explicitly informs your intent (it is also nicer if you use an automatic code formatting).

The empty struct should be fine in both sides because empty structs are actually modelled as a single byte in memory. It is an old C standard trick to make that memory byte's address as an identity for an empty struct.

Since it is on top of the enum, it will be the default state - therefore making any value in memory as a valid and well-defined union state.

Disclaimer: I don't know Zig or Rust but I did my best to follow the how the GuiAction is used in both sides.

Zig seems fine because it doesn't use the value of the union when the action is "none". So I left it as-is.

Rust might not need the MaybeUninit trick because the empty struct should make it a valid enum. So I think it is enough to initialise the tag in make_action.

Unfortunately, my Windows machine has VIM and Clang, but no Rust - so I could not test this change. But I checked if gui.h compiles via clang -x c. :)

I was watching [your video][Ref] and I noticed you want to avoid
uninitialised state in GuiAction. You had to use `MaybeUninit` in Rust
for Rust reasons, but that introduces an UB in your code.

[Ref]: https://www.youtube.com/watch?v=BlRD53b993Q

I think the trick you wanted is an empty struct as the default enum
state in GuiAction. It is as simple as `struct {} none;`, but I added as
an explicit struct because it is easier to remember and explicitly
informs your intent (it is also nicer if you use an automatic code
formatting).

The empty struct should be fine in both sides because empty structs are
actually modelled as a single byte in memory. It is an old C standard
trick to make that memory byte's address as an identity for an empty
struct.

Since it is on top of the enum, it will be the default state - therefore
making any value in memory as a valid and well-defined union state.

Disclaimer: I don't know Zig or Rust but I did my best to follow the
how the GuiAction is used in both sides.

Zig seems fine because it doesn't use the value of the union when the
action is "none". So I left it as-is.

Rust might not need the `MaybeUninit` trick because the empty struct
should make it a valid enum. So I think it is enough to initialise the
tag in `make_action`.

Unfortunately, my Windows machine has VIM and Clang, but no Rust - so I
could not test this change. But I checked if `gui.h` compiles via
`clang -x c`. :)
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.

1 participant