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

Add support for impl mode structs to be repr(packed) #388

Merged
merged 2 commits into from Dec 30, 2023

Conversation

GnomedDev
Copy link
Contributor

This allows the following code to compile, with the only downside being a possible copy of the internal value for non-packed types, although I'm somewhat certain that would be optimised out.

#[repr(packed)]
struct Example(u64);

bitflags::bitflags! {
    impl Example: u64 {
        ...
    }
}

@GnomedDev
Copy link
Contributor Author

Seems like CI failing is unrelated?

@Nilstrieb
Copy link

This should probably have a test to ensure it continues to work.

@GnomedDev
Copy link
Contributor Author

Seems like there aren't any tests for impl other than the examples, should I add a file for specifically this in src/tests?

@KodrAus
Copy link
Member

KodrAus commented Dec 11, 2023

Hi @GnomedDev 👋

Thanks for the PR. If you add a test to tests/compile-pass called something like bitflags_impl_repr_packed with your example that should make sure we don't regress anything. The current UI failure is unrelated to your changes. If you rebase on main they should start working again.

@GnomedDev
Copy link
Contributor Author

Hey @KodrAus, just done that!

Copy link
Member

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

Thanks @GnomedDev! This looks good to me.

@KodrAus KodrAus merged commit 586d819 into bitflags:main Dec 30, 2023
9 checks passed
@GnomedDev GnomedDev deleted the packed-internal branch December 30, 2023 13:39
@GnomedDev
Copy link
Contributor Author

Thanks for merging, any chance for a release now with this in? I'm stoked to use it!

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.

None yet

3 participants