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

Fix up incorrect sub assign behavior and other cleanups #366

Merged
merged 15 commits into from Jun 27, 2023

Conversation

KodrAus
Copy link
Member

@KodrAus KodrAus commented Jun 26, 2023

Closes #365
Closes #364

This PR reworks our test suite to more rigorously test each method on flags types and fixes up the issue of - and -= having different results with unknown bits.

This PR makes the following behavioral changes and clarifications:

  1. -= now behaves the same as - and difference: It respects set bits that don't correspond to known flags.
  2. Zero-valued flags are never printed in formatting output.
  3. Composite flags like const ABC = A | B | C won't be printed in formatting output if A, B, and C have already been printed. Formatting just prints some set of flags that will produce an equal bitpattern when parsed. It doesn't necessarily print every flag that intersects the value.
  4. a.difference(b) is not the same as a & !b when there are set bits that don't correspond to known flags. a.difference will respect them, but ! will unset them.
  5. ! will use !self.bits() & Self::all().bits() instead of Self::from_bits_truncate(!self.bits()). This is a very subtle distinction. Self::from_bits_truncate would remove 0b0000_0001 if the only valid flag was 0b1111_1111, but & Self::all().bits() will retain 0b0000_0001. It retains more bits when there are bits that don't correspond to a known flag, and there is a multi-bit flag defined that does cover them. This is unlikely to have any practical implication until we recommend the flag const ALL = !0.

src/iter.rs Outdated Show resolved Hide resolved
src/parser.rs Outdated Show resolved Hide resolved
@KodrAus KodrAus marked this pull request as ready for review June 26, 2023 12:14
@KodrAus
Copy link
Member Author

KodrAus commented Jun 26, 2023

cc @sunfishcode

This cleans up the issue around -=, some formatting, and starts to document what operators and methods are bitwise. I'd like to spend some more time on docs to clarify the difference between a flag and a bit. It seems like we're working on a bit of a shaky conceptual foundation and it would be good to tighten it all up.

I haven't changed the behavior of ! to respect bits that don't correspond to known flags, because it seems like an easy way to accidentally pollute your flags with bits you'd never mean to set.

@KodrAus
Copy link
Member Author

KodrAus commented Jun 27, 2023

In the interests of getting a fix out for the current regression around -=, I'll go ahead and merge this and release as 2.3.3. I'll keep #363 open to talk about unifying operators with unknown bits.

@KodrAus KodrAus merged commit 3ac85e2 into bitflags:main Jun 27, 2023
8 checks passed
@KodrAus KodrAus deleted the feat/test-cleanups branch June 27, 2023 04:12
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.

SWC - Update to 2.3.x causes incorrect output Inconsistent debug output for flag with no bits
1 participant