Skip to content

dec: use decimal representation when converting to Decimal #86

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.

Sign up for GitHub

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

Merged
petrosagg merged 1 commit into from
May 7, 2025
Merged

dec: use decimal representation when converting to Decimal #86

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions dec/src/decimal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use std::cmp::Ordering;
use std::convert::{TryFrom, TryInto};
use std::ffi::{CStr, CString};
use std::fmt::{self, LowerExp};
use std::fmt::{self, Display, LowerExp};
use std::io::Write;
use std::iter::{Product, Sum};
use std::marker::PhantomData;
Expand Down Expand Up @@ -1775,23 +1775,16 @@ impl<const N: usize> Context<Decimal<N>> {
/// constraints.
// NOTE: The code is generic over any T: LowerExp but passing something like f128 wouldn't
// always work since there are f128 values that don't fit in 24 bytes.
pub fn from_float<T: LowerExp>(&mut self, n: T) -> Decimal<N> {
pub fn from_float<T: LowerExp + Display>(&mut self, n: T) -> Decimal<N> {
Copy link

@ParkMyCar ParkMyCar May 7, 2025

Choose a reason for hiding this comment

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

We should probably mark this #[inline(never)] like std does, wdyt?

Copy link
Contributor Author

@petrosagg petrosagg May 7, 2025

Choose a reason for hiding this comment

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

std marks it as such because the entry point of the user facing interface (i.e the Display impl on f64) goes through this function that only conditionally calls float_to_decimal_common_exact. So #[inline(never)] serves as a means to not use the large stack space if fmt.options.precision is None.

In our case we call from_float uncoditionally so the stackspace will be used no matter what, which makes inlining not problematic.

Copy link

@ParkMyCar ParkMyCar May 7, 2025

Choose a reason for hiding this comment

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

Makes sense! Thanks for double checking!

// The maximum bytes needed to store the decimal representation of a f64.
// This is because you have at most:
// * 1 byte for a possible leading negative sign
// * 17 bytes of significant digits
// See: https://stdrs.dev/nightly/x86_64-pc-windows-gnu/core/num/flt2dec/constant.MAX_SIG_DIGITS.html
// * 1 byte for a decimal point
// * 2 bytes for a negative exponent (e-)
// * 3 bytes for the largest possible exponent (308)
// An example of such maximal float value is f64::from_bits(0x8008000000000000) whose
// decimal representation is '-1.1125369292536007e-308'
const MAX_LEN: usize = 24;
// We follow stdlib's buffer size of 1024 bytes.
// See: https://github.com/rust-lang/rust/blob/1.86.0/library/core/src/fmt/float.rs#L37
const MAX_LEN: usize = 1024;

// Create a buffer that can hold the longest float plus a nul character for the C string
let mut buf = [0u8; MAX_LEN + 1];
let mut unwritten = &mut buf[..MAX_LEN];
write!(unwritten, "{:e}", n).unwrap();
write!(unwritten, "{}", n).unwrap();
let unwritten_len = unwritten.len();
// SAFETY:
// * buf was zero-initialized
Expand Down