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
Bitfield cleanup #204
Bitfield cleanup #204
Conversation
|
Fixed Windows build + spacing. |
|
I can't find any glaring issues and it looks like a nice addition. LGTM |
| // copyright notice, this list of conditions and the following disclaimer | ||
| // in the documentation and/or other materials provided with the | ||
| // distribution. | ||
| // * Neither the name of Google Inc. nor the names of its |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Fixed license header copy-paste derp. |
|
|
||
| BitField& operator = (T val) | ||
| { | ||
| storage = (storage & ~GetMask()) | ((val<<position) & GetMask()); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
…ianness-independent way. The underlying storage type of a bitfield can be any intrinsic integer type, but also any enumeration. Custom storage types are supported if the following things are defined on the storage type: - casting 0 to the storage type - bit shift operators (in both directions) - bitwise & operator - bitwise ~ operator - std::make_unsigned specialization
|
Fixed spacing and improved documentation a tiny bit (it said "u32" for the storage before, now it says T in general). |
| // Now make sure the value is indeed correct | ||
| EXPECT_EQ(val, object.full_u64); | ||
| EXPECT_EQ(*(s64*)&val, object.full_s64); | ||
| EXPECT_EQ((val>>9) & 0x7, object.regular_field_unsigned); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Other than these few style comments, LGTM. Please migrate the rest of Dolphin to this new class too :p Would be cool if we could start forbidding new bitfields. |
This branch adds my BitField class (BSD-licensed) to the Dolphin code base and converts a small number of structures to it. In the long term we should target a full conversion of the source code, but at this stage I don't think the advantage justified the effort, especially since the monotonous task might be prone to stupid errors.
Cf. PR #157 for details.