Skip to content

uint256: Modernize code to use `noexcept` and `constexpr` where possible

Summary

This is an optional refactoring/code quality MR. This MR modernizes the code behind the core base_blob type used to make the uint160, uint256, and derived types to use constexpr and noexcept where possible. This improves the compiler's ability to optimize and do static analysis. The reason we are doing this now is because C++17 made more things constexpr-able, so we should leverage that.

Summary of changes:

  • Use constexpr / noexcept for various methods of base_blob
  • Changed SetNull to just assign *this to a default constructed value. Our default c'tor is constexpr and in most cases that produces only 4 machine instructions without a loop to zero it out (assigns 4 zero'd qwords to the array). I checked on godbolt.org and the code generated here with -O2 is very compact. The old way of using memset would generate equally compact code, but it is less idiomatic and modern to rely on C memset, and it made the function not constexpr.
  • Changed IsNull loop around a bit and made it constexpr & noexcept.
  • Added some static checks in uint256.cpp using the above constexpr functions.
  • Ensured that WIDTH is always sufficiently large but no larger by adding a static_assert.
  • Simplified the code for Compare and also made it constexpr.
  • Made various other methods constexpr and/or noexcept, including dependant functions in strencodings.h which were made noexcept where possible and even constexpr where possible. The more constexpr we have, the more the compiler can analyze and optimize, leading to more efficient code.
  • Made the SetHex function noexcept and also made it not clear the contents before executing -- instead now it clears whatever bytes it didn't write to and that are left over (if any) at the end. Most of the time in our app the unconditional clear was superfluous since we are parsing 32-bytes of hex -- so doing it unconditionally at the beginning was just a redundant memory write.
  • Updated the dependant types CScriptID, BlockHash, and CKeyID to be constexpr/noexcept where applicable. Again, this helps with static analysis and code generation.
  • Updated the uint256_tests.cpp tests to test more corner cases methods and subtleties than were being tested before (this was to test all the code I touched, basically).

No semantic of behavioral changes are introduced by this MR. All externally observable behavior is identical. This is a pure code quality/refactor change.

Test Plan

  • review
  • Do the following with both -DCMAKE_BUILD_TYPE=Release and -DCMAKE_BUILD_TYPE-Debug:
    • ninja all bench_bitcoin check-all to build everything and test
  • Extra credit: Build with the autoconf based build system and ensure everything is kosher.
Edited by Calin Culianu

Merge request reports