Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Conversation

@ghost
Copy link

@ghost ghost commented Jun 8, 2017

Change Decimal's fields to ints.

https://devdiv.visualstudio.com/DevDiv/_workitems?id=447645

The code was ported from DWORD-based C++ and expects to
see unsigned values. Serialization expects to see signed fields.

Fixing this in three steps:

Iteration #1: Use VS autorename to rename the "hi/med/lo/flags" to
"uhi/umed/ulo/uflags"

Iteration #2: Manually revert the field rename (but not the references
to them) and change their types to signed int. Add back the
"u" versions as unsigned ints overlaying the signed versions
and invisible to serialization.

Iteration #3: Opportunistically change a few "u" references
back to the original mostly to get rid of obviously unnecessary casts
to uint.. Doing this very conservatively - only
where it's dead obvious without knowing all the intricacies of
uint vs. int math and and where it wouldn't create local mismatches
of "u" vs non-"u" references. Ugly as this looks, I don't think
converting the entire codebase to signed is in the cards.

Other approaches rejected:

  • Implement a "ulo" property wrapping the "lo" field. Nope -
    some code passes refs to these fields.

  • Implement a "private ref uint ulo => ref Unsafe.As<>..." property

    No-can-do - C# notices that the field belongs to a struct
    and won't let you pass around a ref to that so freely.

Atsushi Kanamori added 3 commits June 8, 2017 09:49
@ghost ghost requested a review from morganbr June 8, 2017 16:54
@morganbr
Copy link
Contributor

morganbr commented Jun 8, 2017

Really nice approach on this, @atsushikan . Thanks for the fix!

@ghost ghost merged commit 069cf16 into dotnet:nmirror Jun 8, 2017
@ghost ghost deleted the decimal branch June 8, 2017 19:46
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants