Enhance type classification and simplify numeric conversion functions#16
Enhance type classification and simplify numeric conversion functions#16FrozenLemonTee wants to merge 27 commits intomainfrom
Conversation
…g_operand concept
…move unused error handling functions
…ric casting integration
…bility in casting
…ssment and type traits
…ng and casting safety
… numeric conversion
There was a problem hiding this comment.
Pull request overview
This PR introduces a new conversion submodule to centralize numeric/underlying casting helpers, expands type classification concepts (e.g., *_like), and updates exports/imports so downstream code can consume the new conversion APIs.
Changes:
- Add
mcpplibs.primitives.conversion(traits + underlying casting utilities) and export it from the umbrellamcpplibs.primitivesmodule. - Refine type concepts by adding operand/“*_like” concepts to better accept primitives and/or underlying types.
- Replace legacy numeric casting helpers with the new conversion utilities and add new unit tests for conversions.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/basic/test_underlying.cpp | Removes legacy alias test; adds coverage for new meta::*_like concepts. |
| tests/basic/test_conversion.cpp | Adds tests for numeric_risk and cast variants (checked/saturating/truncating/exact) including underlying bridging. |
| src/underlying/traits.cppm | Adds underlying_operand concept (cvref-stripped underlying_type). |
| src/primitives.cppm | Exports the new mcpplibs.primitives.conversion module. |
| src/primitive/traits.cppm | Updates default policies resolution and adds meta::*_like concepts; removes legacy alias exports. |
| src/primitive/impl.cppm | Switches underlying bridging to use conversion::saturating_cast. |
| src/policy/impl.cppm | Removes legacy numeric narrowing/casting helpers; keeps to_error_payload in policy::details. |
| src/operations/operators.cppm | Switches checked overflow detection and assignment casting to conversion APIs; adds mapping from conversion risks to policy error kinds. |
| src/conversion/underlying.cppm | Implements numeric risk detection and cast operations for reps and underlying types. |
| src/conversion/traits.cppm | Defines conversion risk kinds and cast_result alias. |
| src/conversion/conversion.cppm | Aggregates and exports conversion submodules. |
| .gitignore | Adds /.vscode ignore entry. |
| template <typename DestRep, typename SrcRep> | ||
| concept builtin_numeric_pair = | ||
| numeric_underlying_type<DestRep> && numeric_underlying_type<SrcRep>; | ||
|
|
||
| template <integer_underlying_type DestRep, integer_underlying_type SrcRep> |
There was a problem hiding this comment.
builtin_numeric_pair / numeric_risk are constrained with numeric_underlying_type / integer_underlying_type, which includes custom underlying types. The implementation then relies on std::is_signed_v, std::intmax_t/std::uintmax_t, and static_cast from value, which will not compile (or will behave incorrectly) for custom underlying types. Consider restricting these helpers to built-in numeric types (e.g., std_integer/std_floating from underlying.traits, or std::integral/std::floating_point) and/or providing an overload that first bridges custom underlying types through underlying::traits<T>::to_rep() before evaluating risk.
| auto const assigned_common = out->load(); | ||
| if constexpr (std::same_as<lhs_value_policy, policy::value::checked> && | ||
| std::integral<lhs_rep>) { | ||
| if (auto const kind = | ||
| policy::details::narrow_numeric_error<lhs_rep>(assigned_common); | ||
| conversion::numeric_risk<lhs_rep>( | ||
| assigned_common); |
There was a problem hiding this comment.
assigned_common is the value of the negotiated common_rep underlying type, but conversion::numeric_risk<lhs_rep>(assigned_common) assumes the source operand behaves like a built-in numeric (casts to intmax/uintmax, etc.). If common_rep is a custom underlying type (possible via common_rep_traits), this call can become ill-formed. Consider running the risk check on the common rep's rep_type (via underlying::traits<common_rep>::to_rep(assigned_common)) and/or guarding the check with constraints (e.g., only when the source is std_numeric).
| template <typename T> | ||
| concept primitive_like = | ||
| primitive_type<T> || underlying_type<std::remove_cvref_t<T>>; | ||
|
|
||
| // Backward-compatible aliases for existing downstream users. | ||
| export namespace mcpplibs::primitives::traits { | ||
| using policy_category [[deprecated]] = meta::policy_category; | ||
| template <typename T> |
There was a problem hiding this comment.
This module no longer exports the previously provided (deprecated) mcpplibs::primitives::traits compatibility aliases. That is an API-breaking change for downstream users still on the legacy namespace; consider restoring the deprecated forwards to mcpplibs::primitives::meta (or explicitly bumping/documenting the breaking change).
PR changes: