Skip to content

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

Merged
KodrAus merged 2 commits intobitflags:mainfrom
GnomedDev:packed-internal
Dec 30, 2023
Merged

Add support for impl mode structs to be repr(packed)#388
KodrAus merged 2 commits intobitflags:mainfrom
GnomedDev:packed-internal

Conversation

@GnomedDev
Copy link
Contributor

@GnomedDev GnomedDev commented Nov 29, 2023

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

GnomedDev commented Nov 29, 2023

Seems like CI failing is unrelated?

@Noratrieb
Copy link

Noratrieb commented Nov 29, 2023

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

@GnomedDev
Copy link
Contributor Author

GnomedDev commented Nov 29, 2023

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

@GnomedDev GnomedDev mentioned this pull request Nov 29, 2023
Closed
@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

GnomedDev commented Dec 18, 2023

Hey @KodrAus, just done that!

KodrAus
KodrAus approved these changes Dec 30, 2023
View reviewed changes
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
@GnomedDev GnomedDev deleted the packed-internal branch December 30, 2023 13:39
@GnomedDev
Copy link
Contributor Author

GnomedDev commented Dec 30, 2023

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

@GnomedDev GnomedDev mentioned this pull request Jan 2, 2024
Merged
@KodrAus KodrAus mentioned this pull request Jan 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KodrAus KodrAus KodrAus approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@GnomedDev @Noratrieb @KodrAus