Skip to content

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

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

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

Conversation

@KodrAus

@KodrAus KodrAus commented Oct 23, 2021

Copy link
Copy Markdown
Member

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

arturoc commented Oct 23, 2021

Copy link
Copy Markdown

That makes a lot of sense yeah.

Comment thread src/bitflags_trait.rs Outdated
Comment thread src/bitflags_trait.rs Outdated
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

KodrAus commented Oct 25, 2021

Copy link
Copy Markdown
Member Author

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

KodrAus commented Oct 26, 2021

Copy link
Copy Markdown
Member Author

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