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 99.20000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.3%. Comparing base (965194a) to head (d0b80d2).

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

Additional details and impacted files

@@            Coverage Diff            @@
##           develop    #654     +/-   ##
=========================================
+ Coverage     96.2%   96.3%   +0.1%     
=========================================
  Files          301     304      +3     
  Lines        29462   29484     +22     
=========================================
+ Hits         28328   28366     +38     
+ Misses        1134    1118     -16     

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	97.2% <99.1%> (+1.7%)	⬆️

... 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...d0b80d2. 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.

[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.

[marcoffee]

1 Missing ⚠️

I could not cover this line. Since the previous and next line are covered, I'm assuming the compiler is optimizing out this assignment because it can prove that val is always reassigned after. I cannot remove the = 0 (probably the reason codecov is assuming it is a valid line, different from the previous coverage), otherwise it will not compile for standards less than C++20 due to uninitialized variables (even if that variable is always initialized later, which is the case here).

I can add LCOV_EXCL_LINE, but this line is valid for runtime contexts when the compiler is unable to optimize it out (e.g., similar case).

[ckormanyos]

I could not cover

Line 1908 unconditionally follows line 1907 (where 1907 is covered).

This is, in my opinion, a clear false-negative. You can LCOV_EXCL_LINE on 1908.

Coverage is challenging with optimized C++. Sometimes it's just not right in LCOV. In some other projects, we specifically reduce the optimization level on coverage runs. This is also not perfect. In the Multiprecision case, we collect coverage from relatively aggressively optimized runs. And it takes a keen eye to find false negatives.

Good catch @marcoffee

[ckormanyos]

I don't think you need to mandate 100% coverage on the patch. Other opinions will vary.

But the entire project is around 96% right now. So we are looking for , maybe, close to 100% patch coverage. The file you mention goes on to have entirely uncovered blocks. These might be more interesting. Or they simply might be out of the scope of your changes.

[marcoffee]

Good catch @marcoffee

@ckormanyos Thanks!

Or they simply might be out of the scope of your changes.

Those other blocks were already uncovered before the PR. I can try to cover them now, but it will add more information to review on the PR and I agree that it might be out of scope. Another option is opening a new PR after this one is merged to try to cover those lines, but the new one would focus entirely on tests (and some small refactoring).

Curiously enough this very similar line is marked as covered.

[ckormanyos]

Marco (@marcoffee) I would recommend that you do NOT try to cover previously uncovered blocks within the scope of your PR.

This is something we will work on in our team as we evolve.

You concentrate on your scope of changes.