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).
⚠️ Report is 8 commits behind head on develop.

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.

[mborland]

What prevents these from being static in the constexpr case?

[marcoffee]

Trying to remember here, but I think it was me following the standard from other cases (e.g., here) or it was causing a compile time failure. I will try to compile without those checks and push if it works.

[mborland]

Prior to C++23 constexpr local variables could not also be static which is what those macro guards are for. In this case it looks like you are just using a const instead of a constexpr so it should work even with your C++20 requirement.

[jzmaddock]

As noted in my comments above, we should find a better solution here, otherwise these variables get recreated/recalculated every time the function is run. In other words making things compile for the constexpr case is dis-optimising the more common runtime case.

[marcoffee]

I tried re-enabling those static modifiers and got the following error under ubuntu 24.04 running GCC 13:

| gcc.compile.c++ ../../../bin.v2/libs/multiprecision/test/test_numeric_limits_cpp_bin_float.test/gcc-13/debug/x86_64/threading-multi/visibility-hidden/test_numeric_limits.o
| In file included from ../../../boost/multiprecision/cpp_int.hpp:2442,
|                  from ../../../boost/multiprecision/cpp_bin_float.hpp:15,
|                  from test_numeric_limits.cpp:69:
| ../../../boost/multiprecision/cpp_int/limits.hpp: In function ‘constexpr boost::multiprecision::number<boost::multiprecision::backends::cpp_int_backend<MinBits, MaxBits, SignType, Checked, Allocator>, ExpressionTemplates> std::detail::get_min(const std::integral_constant<bool, true>&, const std::integral_constant<bool, true>&, const std::integral_constant<bool, false>&)’:
| ../../../boost/multiprecision/cpp_int/limits.hpp:45:29: error: ‘val’ defined ‘static’ in ‘constexpr’ function only available with ‘-std=c++2b’ or ‘-std=gnu++2b’
|    45 |    static const result_type val = -result_type(~ui_type(0));
|       |                             ^~~

I could try only removing those static on consteval contexts (which would ensure the build will only happen during constexpr evaluation or runtime startup), but it might either require if consteval (from C++23) or BOOST_MP_IS_CONST_EVALUATED (with some trick to make the compiler think the variable we pass is not constant evaluated) and some code repetition/macros to initialize val in both contexts. I could also try to move those variables to the outer scope and just return them, which might work. I will try the last option and report it here.

[marcoffee]

Unfortunately the last option did not work, as I cannot return a non-constexpr static variable (they were marked static const) from a constexpr evaluated function. Adding constexpr to the variable would also not work because heap allocations must stay in constexpr evaluation, i.e., they cannot "leak" to runtime.

During the tests I think I found another solution. Since types with an allocator can be initialized from types without one, I can just call the non-allocator method and just construct the resulting type. I will try it and report the results as before.

[marcoffee]

During the tests I think I found another solution. Since types with an allocator can be initialized from types without one, I can just call the non-allocator method and just construct the resulting type. I will try it and report the results as before.

This solution worked. Regarding @jzmaddock comments about performance, I just found out that on return 0 cases the compilers (g++-13 and clang++-19) return a much more efficient assembly for non-static cases (compiler explorer comparison: non-static vs static). This is not the case for more complex returns, such as minimum of signed integers (compiler explorer comparison: non-static vs static).

[jzmaddock]

This is something we should be careful of: in the C++20 case this code could be evaluated at either compile or run time. In the latter case we recreate the local variable val each time it's run. And given that val is a multiprecision type that's not actually a completely trivial thing. So perhaps we could make val either static const or else constexpr depending on BOOST_MP_NO_CXX20_DYNAMIC_ALLOC_CONSTEXPR_DETECTION ?

[marcoffee]

Unfortunately we will not be able to make them constexpr because no allocated heap storage can propagate from constexpr contexts to runtime. About making them static, I'm working on testing them

[mborland]

@marcoffee were you able to make these work as static const or something similar so we don't have to recreate the variable each time?

[marcoffee]

@mborland for the cases returning 0 I found out that simply building the value every time generates a better assembly than using static (comment with compiler explorer proof here, code here). For the other cases where static const would not work due to constexpr evaluation limitations I'm using the static const/static constexpr values from the overload that accepts static and copying to the result, which would be the case of static const because since we are not returning a reference, the value would still be copied at every invocation (comment here, code here).

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

[mborland]

I just took a look through this again and it all looks good to me. One thing I think would be worth doing is running some of our benchmarks for cpp_int such as in performance/test_performance.cpp at develop and at the head of your PR to make sure that changes required for constexpr support didn't cause any runtime regressions such as in a few places I think John pointed out.