Adopt prior-art implementation patterns; fix fp-store overflow terminate#16
Merged
Conversation
Comparison pass over the resources.md libraries (bounded::integer, CNL, Boost.SafeNumerics, fpm, SafeInt, Intel safe-arithmetic, type_safe, PSsst, google/integers), adopting the patterns bound was missing: - tests/fail/: compile-fail diagnostics suite (bounded::integer/CNL) — each TU names its expected static_assert message in an EXPECT line; a control TU proves the harness builds valid code - tests/accuracy.cpp + docs/accuracy.md: fpm-style per-function accuracy tables for all three math engines against a long-double reference, regenerated by the accuracy_report target - detail/rep.hpp: fp representation propagation deduped out of addition/multiplication/division into one fp_rep trait (CNL single-concern style); codegen-neutral - grid hull + std::common_type + mixed-grid min/max (bounded::integer): hull(grid, grid) = interval hull + notch gcd, common_bound_t, and a std::common_type specialisation so mixed-grid bounds interoperate - docs: resources.md "implementation patterns compared" section (adopted/rejected with reasons), roadmap notes for type-level interval unions and the 128-bit rounded store The accuracy harness immediately found a real bug: storing a full-mantissa double-derived rational onto a snap grid with large |Lower| (e.g. dbl::tan's auto output grid) overflowed the exact 64-bit (rhs - Lower)/Notch rational and terminated through the noexcept engine as bad_optional_access. Fixed: - rational::add_impl rescues mixed-sign cross-product overflow in 128-bit when the difference fits umax - store_checked reports errc::overflow instead of dereferencing nullopt - round_quotient is now total: overflow-safe q/r offset rule replaces the historical formulas and serves as fallback when the signed value-index rebuild cannot fit 64 bits Same-sign exact offsets needing >64 bits still report errc::overflow; the full fix (compute the rounded index in 128-bit like to_fixed) is on the roadmap. Regression tests in test_rational.cpp and test_storage_bugs.cpp. Verified: 428/428 ctest (incl. compile-fail, amalgamate, codegen guard), 418/418 under BND_MATH_FIXED, fuzz seed 42 x 20k iters clean, clang-tidy clean under the CI header filter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Comparison pass over the resources.md libraries, adopting the implementation patterns
boundwas missing, plus a crash fix the new accuracy harness surfaced on its first run.Adopted patterns
tests/fail/, from bounded::integer/CNL): each deliberately ill-formed TU names its expectedstatic_assertmessage in a// EXPECT:line; CMake passes the test only when the build fails with that message. A control TU proves the harness builds valid code.tests/accuracy.cpp+ committed docs/accuracy.md, fpm-style): all 16 one-arg transcendentals × 3 engines swept against a long-double reference;accuracy_reporttarget regenerates the doc. dbl is correctly rounded everywhere (max 0.5 notch); flt degrades to ~3 notches on sinh/cosh/exp; cordic sits at 0.5–0.9.detail/rep.hpp: fp representation propagation deduped out of addition/multiplication/division into onefp_reptrait (codegen-neutral, gates pass).std::common_type+ mixed-gridmin/max(bounded::integer):hull(grid, grid)= interval hull + notch gcd (valid by construction via the lattice anchor),common_bound_t,std::common_typespecialisation innumeric_limits.hpp.Bug fix
Storing a full-mantissa double-derived rational onto a snap grid with large
|Lower|(e.g.math::dbl::tan's auto output grid) overflowed the exact 64-bit(rhs − Lower)/Notchrational and terminated through thenoexceptengine asbad_optional_access. Now:rational::add_implrescues mixed-sign cross-product overflow in 128-bit when the difference fitsumaxstore_checkedreportserrc::overflowinstead of dereferencing nulloptround_quotientis total: overflow-safe q/r offset rule, used as fallback when the signed value-index rebuild cannot fit 64 bitsSame-sign exact offsets needing >64 bits still report
errc::overflow; the full fix (compute the rounded index in 128-bit liketo_fixed) is on the roadmap. Regression tests intest_rational.cpp/test_storage_bugs.cpp.Verification
428/428 ctest (unit, examples, compile-fail, amalgamate-up-to-date, codegen guard) · 418/418 under
BND_MATH_FIXED· fuzz seed 42 × 20k iters clean (110M property checks) · clang-tidy clean under the CI header filter · amalgamation regenerated.🤖 Generated with Claude Code