Skip to content

Add a hidden trait to discourage manual impls of BitFlags#261

Merged
KodrAus merged 7 commits intobitflags:mainfrom
KodrAus:feat/bitflags-macro
Oct 26, 2021
Merged

Add a hidden trait to discourage manual impls of BitFlags#261
KodrAus merged 7 commits intobitflags:mainfrom
KodrAus:feat/bitflags-macro

Conversation

@KodrAus
Copy link
Copy Markdown
Member

@KodrAus KodrAus commented Oct 23, 2021

Followup to #220

cc @konsumlamm @arturoc

This adds a hidden supertrait to our new BitFlags trait that suggests you shouldn't implement it manually. Since we only support BitFlags through the bitflags! macro it means we could safely add new methods and supertraits (like Add and friends) to BitFlags without breaking consumers.

I've also just been adding to our test suite wherever I can too so there are some other new tests added there too.

@arturoc
Copy link
Copy Markdown

arturoc commented Oct 23, 2021

That makes a lot of sense yeah.

KodrAus and others added 4 commits October 25, 2021 09:04
Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
@KodrAus
Copy link
Copy Markdown
Member Author

KodrAus commented Oct 25, 2021

Thanks @konsumlamm and @arturoc! This should be ready for another look now.

@konsumlamm
Copy link
Copy Markdown
Contributor

I expected this to be an error after #261 (comment), but it still compiles (because of the extern crate core as _core in lib.rs). Nevertheless, this should just return Option<Self>:

https://github.com/KodrAus/bitflags/blob/b924d4c6487041ae491b860b6b8e21739e9602c5/src/bitflags_trait.rs#L17

@KodrAus
Copy link
Copy Markdown
Member Author

KodrAus commented Oct 26, 2021

Alrighty, I’ll merge this one in and we can follow up with some more bits and pieces. Thanks!

@KodrAus KodrAus merged commit 1aa25e1 into bitflags:main Oct 26, 2021
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