Add constexpr support for dynamic allocated cpp_int

[marcoffee]

Added constexpr support for dynamic allocated cpp_ints as long as compiler implements features __cpp_constexpr_dynamic_alloc and __cpp_lib_constexpr_dynamic_alloc from P0784R7.

If those features are disabled, it behaves as the current implementation. Note that those features require C++20, so they will only work when passing -std=c++2a (or beyond) flag during compilation through a supported compiler (tested here on g++-11+ and clang++-16+ using libstdc++ with g++-10+ toolset on ubuntu noble).

It might work on g++-10, but for some reason the b2 check for if constexpr keep returning "no" for it.

Also fixed constexpr construction from strings and added some tests for construction/operations on dynamic allocated cpp_ints.

Had to make small changes to existing code to tackle some warnings given when compiled without the feature's support (such as uninitialized variables not being supported in constexpr methods before C++20).

[codecov]

Codecov Report

❌ Patch coverage is 96.74797% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.2%. Comparing base (965194a) to head (f1aeeb3).

Files with missing lines	Patch %	Lines
include/boost/multiprecision/cpp_int.hpp	96.2%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop    #654     +/-   ##
=========================================
+ Coverage     96.2%   96.2%   +0.1%     
=========================================
  Files          301     304      +3     
  Lines        29462   29492     +30     
=========================================
+ Hits         28328   28365     +37     
+ Misses        1134    1127      -7     

Files with missing lines	Coverage Δ
...de/boost/multiprecision/cpp_int/cpp_int_config.hpp	100.0% <ø> (ø)
include/boost/multiprecision/cpp_int/limits.hpp	100.0% <100.0%> (ø)
...nclude/boost/multiprecision/detail/empty_value.hpp	100.0% <100.0%> (ø)
...nclude/boost/multiprecision/detail/number_base.hpp	98.0% <ø> (ø)
test/constexpr_test_dynamic_cpp_int.hpp	100.0% <100.0%> (ø)
test/constexpr_test_dynamic_cpp_int_1.cpp	100.0% <100.0%> (ø)
test/constexpr_test_dynamic_cpp_int_2.cpp	100.0% <100.0%> (ø)
include/boost/multiprecision/cpp_int.hpp	96.4% <96.2%> (+0.9%)	⬆️

... and 6 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 965194a...f1aeeb3. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ckormanyos]

Hi @marcoffee I'm sorry this took so long. Your PR was waiting in the queue for a while. I've just approved the CI/CD run on this PR. In Multiprecision, first-time contributors need approval to even run CI/CD, and this can be a bit of a point where we lose some time. Sorry for that.

There is renewed interest in constexpr-ification of cpp_int with allocator storage.

Of course, @jzmaddock will need to look at this, and I will also. But let's see how CI/CD runs with the full battery of compilers running it.

Cc: @jzmaddock and @mborland and @eisenwave

[ckormanyos]

Hi @marcoffee and @jzmaddock CI has run green. One thing, however, that I did not test is a simple code verifying the constexpre-ness of an allocator-based cpp_int.

@marcoffee could you work up an example (post-C++20) that exercises this? Then we can put the test into CI/CD.

Cc: @mborland

[ckormanyos]

The added missing lines in code coverage seem to be lines that aare never expected to run, using BOOST_MP_IS_CONST_EVALUATED. I am not too (perhaps naively) concerned about these?

[ckormanyos]

I can't merge this thing until we get a consensus from @jzmaddock and @mborland

[mborland]

The added missing lines in code coverage seem to be lines that aare never expected to run, using BOOST_MP_IS_CONST_EVALUATED. I am not too (perhaps naively) concerned about these?

It's not that they are never used, it's that they never used at run-time so codecov can't pick them up. I normally just LCOV_EXCL_LINE everything inside these kinds of if consteval blocks.

[marcoffee]

@ckormanyos thanks for your reply!

@marcoffee could you work up an example (post-C++20) that exercises this? Then we can put the test into CI/CD.

Please correct me if I'm wrong, but some of the tests (e.g., here) can only run during constexpr evaluation since they are called in a static_assert. Since we use an infinite cpp_int type, they are also dynamically allocated.

If there is need for more tests I can add them. Also, if there is need for adding some LCOV_EXCL_LINE or LCOV_EXCL_START/LCOV_EXCL_STOP blocks I could look into adding them.

[mborland]

@ckormanyos thanks for your reply!

@marcoffee could you work up an example (post-C++20) that exercises this? Then we can put the test into CI/CD.

Please correct me if I'm wrong, but some of the tests (e.g., here) can only run during constexpr evaluation since they are called in a static_assert. Since we use an infinite cpp_int type, they are also dynamically allocated.

I looked through the tests this morning and yes, it looks like you're already exercising these dynamic allocation paths.

If there is need for more tests I can add them. Also, if there is need for adding some LCOV_EXCL_LINE or LCOV_EXCL_START/LCOV_EXCL_STOP blocks I could look into adding them.

If you would add LCOV_EXCL_LINE or LCOV_EXCL_START/LCOV_EXCL_STOP to blocks that look like this:

if (BOOST_MP_IS_CONST_EVALUATED(n))
{
    // LCOV_EXCL_START

   ...

    // LCOV_EXCL_STOP
}

Or LCOV_EXCL_LINE if it's a one liner.

Codecov will erroneously mark these paths as never taken, but it doesn't support coverage of constant evaluated paths.

[jzmaddock]

BOOST_MP_CXX20_DYNAMIC_ALLOC_CONSTEXPR_IF_DETECTION is a long name ;)

I assume it evaluates to just constexpr? If so we can probably just call it BOOST_MP_CXX20_CONSTEXPR and be consistent with the other constexpr macros?

[marcoffee]

I agree. This name is a mix between the BOOST_MP_HAS_CONSTEXPR_DYNAMIC_ALLOC and BOOST_CXX14_CONSTEXPR_IF_DETECTION (used on the other overloads). I added it in case some compiler just implements the dynamic allocation without the constexpr if detection, but I think it might not happen. In this case, is it ok if I just remove this macro and replace it with BOOST_MP_HAS_CONSTEXPR_DYNAMIC_ALLOC?

[mborland]

Does Boost.Config have BOOST_CXX20_CONSTEXPR? I guess I would associate that with __cpp_constexpr > 201907L since there was a huge expansion of constexpr in 20. I would go with BOOST_MP_CXX20_ALLOC_CONSTEXPR. A macro with _HAS_ in the name looks odd in the function signature, usually those are associated with some in body configuration or configuration of other macros.

[marcoffee]

I will use the BOOST_MP_CXX20_DYNAMIC_ALLOC_CONSTEXPR then. I prefer using DYNAMIC_ALLOC instead of just ALLOC because it closer to the standard feature tests __cpp_constexpr_dynamic_alloc and __cpp_lib_constexpr_dynamic_alloc, but I can change it to BOOST_MP_CXX20_ALLOC_CONSTEXPR if required.

[marcoffee]

If you would add LCOV_EXCL_LINE or LCOV_EXCL_START/LCOV_EXCL_STOP to blocks that look like this:

if (BOOST_MP_IS_CONST_EVALUATED(n))
{
    // LCOV_EXCL_START

   ...

    // LCOV_EXCL_STOP
}

Or LCOV_EXCL_LINE if it's a one liner.

There is just the case for this check that may run at consteval time or not depending on the input types. In this case, should I:

1. leave it without the LCOV_EXCL_*
2. add the LCOV_EXCL_*
3. split the check into two checks and add the LCOV_EXCL_* to the one with BOOST_MP_IS_CONST_EVALUATED?

[mborland]

There is just the case for this check that may run at consteval time or not depending on the input types. In this case, should I:

1. leave it without the LCOV_EXCL_*
2. add the LCOV_EXCL_*
3. split the check into two checks and add the LCOV_EXCL_* to the one with BOOST_MP_IS_CONST_EVALUATED?

In that case it looks like you could write tests that would cover both paths at runtime. One with same DestT and SrcT and one where they are different.

[marcoffee]

In that case it looks like you could write tests that would cover both paths at runtime. One with same DestT and SrcT and one where they are different.

I found out that this method is only called for copying limbs, so the types should always be the same. I just removed the possibility of passing two different types and ended up with just the BOOST_MP_IS_CONST_EVALUATED, where I added the LCOV_EXCL_* comments.

[marcoffee]

Just resolved the conflicts locally and started the tests. As soon as it is finished here I will push a new version with the changes.

[marcoffee]

It seems like the Install Packages step is failing due to network errors. I'm trying to force push to the branch to trigger rerun.