util: Add SaturatingAdd helper #24224

1. 
   MarcoFalke commented at 12:49 pm on February 1, 2022: member

   Seems good to have this in the repo, as it might be needed to write cleaner code. For example:

   • #24090 (comment)
   • #23418 (review)
   • …
2. DrahtBot added the label Utils/log/libs on Feb 1, 2022
3. 
   laanwj commented at 11:08 am on February 3, 2022: member

   Concept ACK. This seems useful, hopefully C++ will have built-ins for explicitly saturating/wrapping/checked arithmetic like rust some day. But until then…
4. 
   MarcoFalke commented at 12:35 pm on February 3, 2022: member

   saturating/wrapping/checked arithmetic like rust

   I wasn’t aware that rust has those built-in. Mind sharing a link to the docs?

5. 
   MarcoFalke commented at 12:41 pm on February 3, 2022: member

   Oh, I see. By default rust will panic, but there are https://doc.rust-lang.org/std/num/index.html:

   • https://doc.rust-lang.org/std/num/struct.Saturating.html
   • https://doc.rust-lang.org/std/num/struct.Wrapping.html
6. in src/util/overflow.h:33 in fa90189cbf outdated

   27@@ -28,4 +28,22 @@ template <class T>
   28     return i + j;
   29 }
   30 
   31+template <class T>
   32+[[nodiscard]] T SaturatingAdd(const T i, const T j) noexcept
   

   ---

   ---

   

   klementtan commented at 4:52 am on February 12, 2022:

   Out of curiosity, any reason you didn’t constexpr this function?

   ---

   

   MarcoFalke commented at 7:09 am on February 12, 2022:

   Outside of unit tests I don’t think this will ever be called in a context that would be constexpr.
7. klementtan approved
8. 
   klementtan commented at 5:14 am on February 12, 2022: contributor

   Code Review ACK fa90189cbfe10552f94fdaa8607cdd5a10341ced
9. in src/test/util_tests.cpp:1478 in fa90189cbf outdated

   1472@@ -1473,9 +1473,15 @@ static void TestAddMatrixOverflow()
   1473     constexpr T MAXI{std::numeric_limits<T>::max()};
   1474     BOOST_CHECK(!CheckedAdd(T{1}, MAXI));
   1475     BOOST_CHECK(!CheckedAdd(MAXI, MAXI));
   1476+    BOOST_CHECK_EQUAL(MAXI, SaturatingAdd(T{1}, MAXI));
   1477+    BOOST_CHECK_EQUAL(MAXI, SaturatingAdd(MAXI, MAXI));
   

   ---

   ---

   

   vasild commented at 12:52 pm on February 21, 2022:

   Maybe add a test like:

   0BOOST_CHECK_EQUAL(MAXI - 1, SaturatingAdd(T{1}, MAXI - 2));
   

   to ensure that SaturatingAdd() does not always, or too often, return MAXI. Or, keeps being accurate close to the limit.

   (same for MINI + 1 tests below)

   ---

   

   MarcoFalke commented at 1:33 pm on February 21, 2022:

   thx, done
10. vasild approved
11. 
    vasild commented at 12:54 pm on February 21, 2022: member

    ACK fa90189cbfe10552f94fdaa8607cdd5a10341ced
12. util: Add SaturatingAdd helper faa7d8a3f7
13. MarcoFalke force-pushed on Feb 21, 2022
14. 
    MarcoFalke commented at 1:34 pm on February 21, 2022: member

    Added a test. Should be trivial to re-ACK with git range-diff bitcoin-core/master fa90189cbf faa7d8a3f7
15. vasild approved
16. 
    vasild commented at 2:21 pm on February 21, 2022: member

    ACK faa7d8a3f7cba02eca7e247108a6b98ea9daf373
17. 
    klementtan commented at 2:34 pm on February 21, 2022: contributor

    reACK faa7d8a3f7
18. MarcoFalke merged this on Feb 21, 2022
19. MarcoFalke closed this on Feb 21, 2022

    ---

20. MarcoFalke deleted the branch on Feb 21, 2022
21. sidhujag referenced this in commit deb823f097 on Feb 22, 2022
22. DrahtBot locked this on Feb 21, 2023

Contributors
MarcoFalke laanwj klementtan vasild

Labels
Utils/log/libs