Move Decimal to shared by pentp · Pull Request #18948 · dotnet/coreclr (original) (raw)

This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation10 Commits3 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This is the minimum set of changes required to move decimal to a fully managed implementation shared with CoreRT. Fixes #18249.
Managed decimal performance work over the last year has made it at least as fast as the native implementation, often much faster.

pentp mentioned this pull request

Jul 16, 2018

How good is the code coverage of the managed implementation with our Corefx test bed?

I've been adding new tests also while doing the perf work so the coverage should be pretty good.

throw new SerializationException(SR.Overflow_Decimal, e);
}
if (!IsValid(flags))
throw new SerializationException(SR.Overflow_Decimal);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will throw exception without the inner exception now. Does it matter?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From an API compatibility perspective it doesn't matter.
Human readability of the exception is somewhat misleading either way (the inner exception was "Decimal byte array constructor requires an array of length four containing valid decimal bytes.").