Re: Another Boost.Decimal Review

From: Matt Borland (matt_at_[hidden])
Date: 2025-01-23 15:53:42

> Thanks to Matt Borland and Chris Kormanyos for this good work. I vote to
> ACCEPT the library. I trust the authors to improve documentation, api, and
> performance as a matter of continued development. Let me say a few things
> about some discussions in the review threads up-front.
> 

> Standardization:
> 

> I'm an advocate for it and would be willing to help the authors with the
> process. Rationale: 1) this standardizes existing practice (aka IEEE 754),
> something like it is supported in many languages (specifically C23), and
> there's significant use cases for this in many domains even if it's
> slightly slower. 2) While a standalone library can go far, often a
> compiler/library can more easily optimize fundamental things like numeric
> types. 3) I spoke with Dietmar about the review and standardization -- he's
> still an advocate for having this capability, but has higher priorities at
> the moment.
> 

We would certainly appreciate your help with standardization. Since C23 added decimal types I don't know why C++ would not have them.

> N2849:
> 

> It's a decent design for 2009 -- less so in 2025. Specifically, w.r.t.
> conversion we should only perform automatic conversions from types (see
> below). The library already takes care of from_chars, format and some
> other more modern things N2849 wouldn't address.
> 

> C23 and _Decimal*
> 

> Should this library consider being a layer over C23 types where that's
> possible? GCC supports them at least in a limited fashion.
> 

> https://gcc.gnu.org/onlinedocs/gcc/Decimal-Float.html
> 

It would be easier to add a wrapper over those than the Intel lib because they already come with GCC.

> 

> > Does this library bring real benefit to C++ developers for real world
> 

> use-case?
> 

> > Do you have an application for this library?
> 

> 

> Yes and yes.
> 

> > Does the API match with current best practices?
> 

> 

> I have some quibbles here.
> 

> Construction:
> 

> template <typename T, typename T2>constexpr decimal32(T coeff, T2 exp,
> 

> bool sign = false) noexcept;
> 

> Is the sign flag helpful here? It seems simpler to just handle a negative
> coefficient -- like it already does. When I look at it there's already
> cognitive dissidence in my brain about if the sign flag overrides or
> inverts the coefficient sign. The N2849 api doesn't have this flag.
> 

I think a couple people have mentioned confusion over this so it's likely in everyone's best interest to remove.

> 

> Fast Types:
> 

> I read through the list conversation about the _fast types. My suggestion
> is to drop them. Simplifies everything for users and implementers. The
> movement in c++ over the last decade(s) has been to precisely say the size
> of the type to ensure correctness and not 'let the platform decide' (aka
> uint32_t, int64_t, etc). In my comment on documentation you'll notice the
> c++20 extended floating point types only provide fixed sizes. Also N2849
> and C23 don't support _fast types for decimal.
> 

> 

> Conversions:
> 

> I'd like to see the library pull back on convertibility.
> 

> long double ld{1.23456789234567};
> decimal32 d(ld); // what happens -- round, truncate, doesn't compile?
> 

> The best answer is doesn't compile. There's been significant work in the
> standard now to define floating point rank and limit conversions with the
> extended floating point types. Specifically:
> 

> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p1467r9.html#rank
> 

That's not a terrible idea. We made them explicit and added notes in the docs that it may not do what you expect it to do. It's probably best to just remove that particular foot gun.

> There's also other work on type traits and value preserving conversions in
> the SIMD capabilities. Also, std::chrono basically already does this -- any
> value destroying conversion has to be explicit. At a minimum the answer
> needs to be better documented - aka does what 754-2008 says?
> 

> For other examples of things I'd remove and make a free function cast/round
> -- aka explicit user decision:
> 

> class decimal64 {
> ...
> explicit constexpr operator std::int8_t() const noexcept;
> 

> This is highly likely to not be value preserving so shouldn't be possible
> without a decision on how to handle overflow.
> 

> class decimal64 {
> ...
> explicit constexpr operator decimal32() const noexcept;
> 

> Literals
> 

> constexpr auto operator "" _DF(const char* str) -> decimal32constexpr
> 

> auto operator "" _df(const char* str) -> decimal32
> 

> 

> Why not _d32, _d64, _d128? _df makes me think fast, not 32.
> 

The current ones match the TR and what GCC has been doing with the _DecimalXX types.  

> > Is the documentation helpful and clear?
> 

> 

> Not bad, but as usual improvements can be made. As a user I'm going to
> need to select one of these types -- I'd love to see a table like this one
> at cppreference for named floating point types:
> 

> Type name
> Defined in header
> <stdfloat> https://en.cppreference.com/w/cpp/header/stdfloat Literal
> 

> suffix Predefined macro C language type Type properties
> bits of storage bits of precision bits of exponent max exponent
> std::float16_t f16 or F16 STDCPP_FLOAT16_T _Float16 16 11 5 15
> std::float32_t f32 or F32 STDCPP_FLOAT32_T _Float32 32 24 8 127
> std::float64_t f64 or F64 STDCPP_FLOAT64_T _Float64 64 53 11 1023
> std::float128_t f128 or F128 STDCPP_FLOAT128_T _Float128 128 113 15 16383
> 

> std::bfloat16_t bf16 or BF16 STDCPP_BFLOAT16_T (N/A) 16 8 8 127
> 

> Not sure how it renders in email so source:
> https://en.cppreference.com/w/cpp/types/floating-point
> 

> Not looking for exact replication, but something like this.
> 

We will be switching a bunch of things to tables after Peter Turcan and other brought it up.

> In the using the library section I'd like to see some:
> - examples of using it it math
> - examples of the behavior of NAN/inf values
> - conversions from other types like float expanded
> 

On the math one there are a couple examples with math (including binding with Boost.Math). The others we can add.

> Where is the design decisions section? Some of the things I'm asking about
> should be covered in the docs.
> 

Bottom of the page. We can bring it up into a more prominent location.

> > Did you try to use it? What problems or surprises did you encounter?
> 

> 

> I didn't have time to play with it unfortunately -- godbolt would be
> helpful.
> 

> That's all for now.
> 

> Jeff
> 

Thank you for your review.

Matt