-
Notifications
You must be signed in to change notification settings - Fork 5k
Cleanup BitMapper sizeof type #86881
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
Conversation
The newly introduced internal BitMapper in DiagnosticsHelper uses an internal `Span<ulong>` however a couple of `sizeof()` used `long`. While these types are the same size, this is a minor correctness/cleanliness change with no actual code delta. Also added a comment to document the right-shift by 6 bits.
Wondering if we care about |
Tagging subscribers to this area: @tommcdon Issue DetailsIn fix for #86740, the newly introduced internal BitMapper in DiagnosticsHelper.cs uses an internal
|
Additionally, wondering if there is value in making this a |
Performance differences are possible, but functionally it shouldn't matter.
Given that we already have BitArray and BitVector32, I'm skeptical there are many .NET users who would benefit from another. I don't rule it out, but also no plan to proactively pursue it.
Assuming that you used 'public class' to mean any public type in .NET then sure, BitVector32 is an example that is struct and there are many public structs in .NET. If you meant 'class' as in the C# keyword, then no, struct and class are mutually exclusive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me, but @tarekgh owns the code so his call if he wants the change.
In fix for #86740, the newly introduced internal BitMapper in DiagnosticsHelper.cs uses an internal
Span<ulong>
however a couple ofsizeof()
usedlong
. While these types are the same size, this is a minor correctness/cleanliness change with no actual code delta. Also added a comment to document the right-shift by 6 bits.