Attempt rudimentary repair issue 1110

[ckormanyos]

No description provided.

[ckormanyos]

Ok I put this on draft. There is a preliminary fix. I still need to put in explicit tests for the issue, as i seem to have forgotten these.

This first fix, if we can agree on it and its scope, could be about ready tomorrow.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.4%. Comparing base (7d44a84) to head (0cf81f3).
⚠️ Report is 29 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #1121     +/-   ##
=========================================
+ Coverage     99.4%   99.4%   +0.1%     
=========================================
  Files          246     247      +1     
  Lines        16808   16861     +53     
  Branches      1814    1814             
=========================================
+ Hits         16701   16754     +53     
  Misses         107     107             

Files with missing lines	Coverage Δ
include/boost/decimal/detail/cmath/cbrt.hpp	96.7% <100.0%> (ø)
include/boost/decimal/detail/cmath/log10.hpp	94.5% <100.0%> (ø)
include/boost/decimal/detail/cmath/pow.hpp	98.2% <100.0%> (ø)
include/boost/decimal/detail/cmath/sqrt.hpp	97.6% <100.0%> (ø)
...ude/boost/decimal/detail/remove_trailing_zeros.hpp	100.0% <100.0%> (ø)
test/github_issue_1110.cpp	100.0% <100.0%> (ø)

---

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 7d44a84...0cf81f3. Read the comment docs.

🚀 New features to boost your workflow:

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

[ckormanyos]

OK, this PR now handles small as well as large arguments, as mentioned in #1110.

I have marked this PR as ready for review. @mborland if you like, you can take a look. I'm happy with the patch as such, but somewhat concerned about the added time/complexity, even though I protected the run-time with several filtering-style if-queries.

We might be able to refine this patch down the road with improved internals from the library, but this patch seems to "do the trick".

Cc: @libbooze

[mborland]

Let me ask the original author of the remove_trailing_zeros algorithm if there's a better way to integrate the change. I'm sure we will get bit in other places (like charconv uses this for printing shortest mode)

[ckormanyos]

Let me ask the original author of the remove_trailing_zeros algorithm if there's a better way to integrate the change. I'm sure we will get bit in other places (like charconv uses this for printing shortest mode)

Yes Matt (@mborland) that is what I was thinking too. It is really just one single table entry needing special handling at 128-bits only. Maybe we could detect this case and treat it at its source.

I will just let this PR sit here until we reach a decision.

[mborland]

I think this cast is the real issue: https://github.com/cppalliance/decimal/blob/develop/include/boost/decimal/detail/cmath/sqrt.hpp#L64

The starting significand is: 1000000000000000000000000000000010
This is then trimmed to: 100000000000000000000000000000001

By casting to unsigned instead of just comparing against 1U the answer becomes true (1U == 1U) instead of false (100000000000000000000000000000001 == 1U)

[mborland]

See: #1130