Decimal Library re-review begins today
The second review of the proposed Decimal Number library by Matt Borland and Chris Kormanyos begins today and runs until 15th Oct. You will find documentation here: https://develop.decimal.cpp.al/decimal/overview.html And the code repository is here: https://github.com/cppalliance/decimal/ Boost.Decimal is an implementation of IEEE 754 <https://standards.ieee.org/ieee/754/6210/> <https://standards.ieee.org/ieee/754/6210/%3E> and ISO/IEC DTR 24733 <https://www.open-std.org/JTC1/SC22/WG21/docs/papers/2009/n2849.pdf> <https://www.open-std.org/JTC1/SC22/WG21/docs/papers/2009/n2849.pdf%3E> Decimal Floating Point numbers. The library is header-only, has no dependencies, and requires C++14. This re-review is happening because the original review had an indeterminate result (see my summary at https://lists.boost.org/archives/list/boost@lists.boost.org/message/2Q7UBOTE...) and resulted in my filing a number of issues against the library based on the reviewers comments. Now that Matt and Chris have been busy fixing these and addressing the reviewers concerns, the library is now back for a second look, see https://github.com/cppalliance/decimal/issues?q=is%3Aissue%20state%3Aclosed%... for a complete list of issues addressed from the last review. I hope both the original reviewers, and new ones will come back for a second look. Please feel free to amend your original review, or if you're starting from scratch then please provide feedback on the following general topics: - What is your evaluation of the design? - What is your evaluation of the implementation? - What is your evaluation of the documentation? - What is your evaluation of the potential usefulness of the library? - Do you already use it in industry? - Did you try to use the library? With which compiler(s)? Did you have any problems? - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? - Are you knowledgeable about the problem domain? Ensure to explicitly include with your review: ACCEPT, REJECT, or CONDITIONAL ACCEPT (with acceptance conditions). Best, John Maddock (review manager).
This re-review is happening because the original review had an indeterminate result (see my summary at https://lists.boost.org/archives/list/boost@lists.boost.org/message/2Q7UBOTE...) and resulted in my filing a number of issues against the library based on the reviewers comments. Now that Matt and Chris have been busy fixing these and addressing the reviewers concerns, the library is now back for a second look, see https://github.com/cppalliance/decimal/issues?q=is%3Aissue state%3Aclosed label%3A"Boost Review" for a complete list of issues addressed from the last review.
In addition to the issues list Boost Review here's a summary of the activities from the last 9 months: Breaking Changes: - Based on bitwise comparisons with other similar libraries and database software, we have changed the internal encoding of our IEEE 754-compliant types - We spent about 3 months optimizing just back end integer types that are now used throughout the library, and as the internals of decimal128_t - We have changed the type names to better match conventions: - `decimalXX` is now `decimalXX_t` - `decimalXX_fast` is now `decimal_fastXX_t` - The headers have been similarly renamed (e.g. decimal32.hpp -> decimal32_t.hpp), and can now be used independently instead of requiring the monolith based on feedback in Review - Constructors have been simplified to reduce confusion (no more double negative logic) - The default rounding mode has changed to align with IEEE 754, with rounding bugs being squashed across the modes as well Other Changes: - The documentation content has been overhauled thanks to feedback from Peter Turcan and others during the first review - The docs are no longer a single long page of Asciidoc; we have moved to Antora. Thanks to Joaquín and Christian for making it trivial to copy from Unordered to make that happen. - https://develop.decimal.cpp.al/ - We now support formatting with {fmt} - Benchmarks have been expanded to include GCC `_DecimalXX` types, and Intel's libbid. I think people should be pleased with the results now, since that was a huge point of contention at the end of the review - We have added support for CMake pkg config for ease of use Matt
pon., 6 paź 2025 o 17:47 John Maddock via Boost <boost@lists.boost.org> napisał(a):
The second review of the proposed Decimal Number library by Matt Borland and Chris Kormanyos begins today and runs until 15th Oct.
You will find documentation here: https://develop.decimal.cpp.al/decimal/overview.html
And the code repository is here: https://github.com/cppalliance/decimal/
Boost.Decimal is an implementation of IEEE 754 <https://standards.ieee.org/ieee/754/6210/> <https://standards.ieee.org/ieee/754/6210/%3E> and ISO/IEC DTR 24733 <https://www.open-std.org/JTC1/SC22/WG21/docs/papers/2009/n2849.pdf> <https://www.open-std.org/JTC1/SC22/WG21/docs/papers/2009/n2849.pdf%3E> Decimal Floating Point numbers.
Thank you for the update, and thank you for the author's efforts to deliver the second incarnation of the library. A couple of quick questions. Regarding the ISO/IEC DTR 24733, shall we treat Boost.Decimal as a literal implementation of that TR up to the last detail, or rather that it wanted to be as close as reasonable but not closer. For instance, N2849 has a different initialization interface for the decimal types. From the documentation on rounding modes ( https://develop.decimal.cpp.al/decimal/examples.html#examples_rounding_mode), I cannot figure out how I actually get to control it for my operations. Do I just create the object and as long as it is in scope all my operations have the altered mode? Or do I need to pass it to operations? If the former, does it have all the problems that globals have? For instance, the interactions with coroutines where the same lexical scope can start on one thread and finish on another? Anyway, the documentation on the rounding mode is missing. The link points to the branch `develop` on GitHub. Is it the branch `develop` that is the subject of the review? Regards, &rzej;
The library is header-only, has no dependencies, and requires C++14.
This re-review is happening because the original review had an indeterminate result (see my summary at
https://lists.boost.org/archives/list/boost@lists.boost.org/message/2Q7UBOTE...)
and resulted in my filing a number of issues against the library based on the reviewers comments. Now that Matt and Chris have been busy fixing these and addressing the reviewers concerns, the library is now back for a second look, see
https://github.com/cppalliance/decimal/issues?q=is%3Aissue%20state%3Aclosed%... for a complete list of issues addressed from the last review.
I hope both the original reviewers, and new ones will come back for a second look.
Please feel free to amend your original review, or if you're starting from scratch then please provide feedback on the following general topics:
- What is your evaluation of the design?
- What is your evaluation of the implementation?
- What is your evaluation of the documentation?
- What is your evaluation of the potential usefulness of the library?
- Do you already use it in industry?
- Did you try to use the library? With which compiler(s)? Did you have any problems?
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
- Are you knowledgeable about the problem domain?
Ensure to explicitly include with your review: ACCEPT, REJECT, or CONDITIONAL ACCEPT (with acceptance conditions).
Best, John Maddock (review manager).
_______________________________________________ Boost mailing list -- boost@lists.boost.org To unsubscribe send an email to boost-leave@lists.boost.org https://lists.boost.org/mailman3/lists/boost.lists.boost.org/ Archived at: https://lists.boost.org/archives/list/boost@lists.boost.org/message/2GQFSND3...
Thank you for the update, and thank you for the author's efforts to deliver the second incarnation of the library.
Thank you for taking another look. Your comments and issues the first time around were helpful.
A couple of quick questions. Regarding the ISO/IEC DTR 24733, shall we treat Boost.Decimal as a literal implementation of that TR up to the last detail, or rather that it wanted to be as close as reasonable but not closer. For instance, N2849 has a different initialization interface for the decimal types.
Consider ISO/IEC DTR 24733 to be a starting point. It was written in 2009, so much has progressed since then. Example is I don't see the need to offer make_decimalXX or decimalXX_to_ functions since classes have the facilities for those. The discussion on required functions, rounding modes, etc. is derived from IEEE 754 so that is absolutely valid (and more readable than the IEEE version).
From the documentation on rounding modes ( https://develop.decimal.cpp.al/decimal/examples.html#examples_rounding_mode), I cannot figure out how I actually get to control it for my operations. Do I just create the object and as long as it is in scope all my operations have the altered mode? Or do I need to pass it to operations? If the former, does it have all the problems that globals have? For instance, the interactions with coroutines where the same lexical scope can start on one thread and finish on another? Anyway, the documentation on the rounding mode is missing.
There's a global rounding mode flag[1] that is set at compile time that you can query with fegetround() and set with fesetround(rounding_mode)[2][3]. These are globals and will have the same problems you described in theory. In practice binary floating point will also have the same issues because you are reading/writing to an FPU flag. Unlike binary floating point we do let you set the initial rounding mode via macro. This was one of the concerns in the constexpr <cmath> paper where you could have different results because of divergent rounding modes between compile time and run time [4].
The link points to the branch `develop` on GitHub. Is it the branch `develop` that is the subject of the review?
Yes, the docs match develop, and the website is automatically updated on every documentation change to committed to develop. Matt [1] https://github.com/cppalliance/decimal/blob/develop/include/boost/decimal/cf... [2] https://en.cppreference.com/w/cpp/numeric/fenv/feround.html [3] https://develop.decimal.cpp.al/decimal/cfenv.html [4] https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p0533r9.pdf
wt., 7 paź 2025 o 07:50 Matt Borland <matt@mattborland.com> napisał(a):
The link points to the branch `develop` on GitHub. Is it the branch `develop` that is the subject of the review?
Yes, the docs match develop, and the website is automatically updated on every documentation change to committed to develop.
I can see changes being committed to branch `develop`. I do not think this is desirable. I think the shape of the library (source code, docs) should remain frozen for the time of the review. All the reviewers need to be able to look at and talk about the same thing. Either provide a branch dedicated for the review, or refrain from doing changes to `develop` during the review period. Regards, &rzej;
I can see changes being committed to branch `develop`. I do not think this is desirable. I think the shape of the library (source code, docs) should remain frozen for the time of the review. All the reviewers need to be able to look at and talk about the same thing. Either provide a branch dedicated for the review, or refrain from doing changes to `develop` during the review period.
The only changes have been for clarity: you asked for an improved error message, and someone on Reddit asked for clarification on underflow/overflow of constructors to be added to the Docs. I'm fine with freezing the branch, but I thought it would make sense to address clarity issues along the way so they don't confuse potentially multiple reviewers when it could be dealt with on the spot. Matt
participants (3)
-
Andrzej Krzemienski -
John Maddock -
Matt Borland