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.

Sign up for GitHub

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

Jump to bottom

set repr to transparent for various types #402

Merged
merged 2 commits into from Jan 25, 2023
Merged

set repr to transparent for various types #402

merged 2 commits into from Jan 25, 2023

Conversation

Freax13
Copy link
Contributor

@Freax13 Freax13 commented Jan 25, 2023

Now that the representation is fixed, these types can be embedded into other structures such as AMD's VMCB and VMSA or Intel's ATTRIBUTES (SGX).

@Freax13
set repr to transparent for various types
bf32052 
Now that the representation is fixed, these types can be embedded into
other structures such as AMD's `VMCB` and `VMSA` or Intel's
`ATTRIBUTES` (SGX).
phil-opp
phil-opp approved these changes Jan 25, 2023
View reviewed changes
Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Looks good!

@phil-opp
Copy link
Member

We should bump our bitflags dependency to v1.3 though. Earlier versions don't mention that repr(transparent) is valid.

@Freax13
bump bitflags dependency
94677aa 
This newer version has stronger guarantees about the type layout of the
generated types: It guarantees that types annotated with
`repr(transparent)` are newtypes around the base integer.
@Freax13
Copy link
Contributor Author

Freax13 commented Jan 25, 2023

We should bump our bitflags dependency to v1.3 though. Earlier versions don't mention that repr(transparent) is valid.

Good catch, thanks! Better be safe than sorry!

@Freax13 Freax13 requested a review from phil-opp January 25, 2023 12:28
@Freax13 Freax13 merged commit 642956d into rust-osdev:master Jan 25, 2023
@Freax13 Freax13 mentioned this pull request Sep 15, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phil-opp phil-opp phil-opp approved these changes

@josephlr josephlr Awaiting requested review from josephlr

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Freax13 @phil-opp