Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: minor
---

### Added
- `WrappingArithmetic` composite trait bundling `WrappingAdd`, `WrappingSub`, `WrappingMul`, `WrappingNeg`, `WrappingShl`, `WrappingShr` from `num-traits`
- `WrappingArithmetic` as a supertrait of `LinkReference`, enabling downstream crates to use wrapping arithmetic with `T: LinkReference` without additional `+ WrappingAdd` bounds
107 changes: 107 additions & 0 deletions docs/case-studies/issue-146/analysis.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# Case Study: Issue #146 — Add WrappingAdd to LinkReference Trait Bounds

## Timeline / Sequence of Events

| Date | Event | Repository |
|------|-------|-----------|
| 2026-04-13 | doublets-rs#47 opened — quality audit requiring latest dependencies and trait unification | linksplatform/doublets-rs |
| 2026-04-14 | data-rs#16 opened — Replace funty dependency with platform-num (`LinkReference`) | linksplatform/data-rs |
| 2026-04-14 | data-rs#17 PR opened — Implements the replacement, discovers `WrappingAdd` gap | linksplatform/data-rs |
| 2026-04-14 | Numbers#146 opened — Request to add `WrappingAdd` to `LinkReference` trait bounds | linksplatform/Numbers |
| 2026-04-14 | Numbers#147 PR opened — Implementation of `WrappingArithmetic` composite trait | linksplatform/Numbers |

## Requirements from the Issue

### R1: Add wrapping arithmetic to `LinkReference` supertraits (Original request)
The original issue requested adding `WrappingAdd` from `num-traits` to `LinkReference` so downstream crates don't need explicit `+ WrappingAdd` bounds.

### R2: Better naming than `WrappingAdd` (From comment by @konard)
The maintainer disliked the name `WrappingAdd` and suggested something like `AdditionCapable`. They asked to check if similar traits already exist in `num-traits`.

### R3: Support similar operations for other arithmetic ops (From comment by @konard)
Not just `WrappingAdd` — all wrapping arithmetic operations normally supported by numeric types should be included.

### R4: Compile data and perform deep case study analysis (From comment by @konard)
Download all logs and data related to the issue, compile to `./docs/case-studies/issue-146` folder, reconstruct timeline, find root causes, propose solutions.

## Root Cause Analysis

### Root Cause 1: Trait ecosystem fragmentation
The Rust ecosystem has two competing unsigned integer trait crates:
- `funty` (provides `funty::Unsigned`)
- `num-traits` (provides `num_traits::Unsigned`)

`platform-data` used `funty::Unsigned` while `platform-num` (Numbers) used `num_traits::Unsigned`. These are **different traits from different crates**, even though they represent the same concept. This made it impossible to write unified `T: data::LinkType + trees::LinkType` bounds.

### Root Cause 2: Missing wrapping arithmetic in `PrimInt`
The `PrimInt` trait from `num-traits` (which is a supertrait of `Number`) provides **non-wrapping** arithmetic. The wrapping variants (`WrappingAdd`, `WrappingSub`, `WrappingMul`, `WrappingNeg`, `WrappingShl`, `WrappingShr`) are separate traits in `num_traits::ops::wrapping`. This means code that needs wrapping semantics (like `Hybrid` in `platform-data`) must add extra trait bounds.

### Root Cause 3: No composite wrapping trait in `num-traits`
Unlike `PrimInt` which bundles many non-wrapping operations, `num-traits` does **not** provide a composite trait that bundles all wrapping operations. Each wrapping operation is a separate trait, leading to verbose bounds like `T: WrappingAdd + WrappingSub + WrappingMul + WrappingNeg`.

## Solution Implemented

### Approach: Composite `WrappingArithmetic` trait

Created a new `WrappingArithmetic` composite trait in `platform-num` that bundles all six wrapping arithmetic traits from `num-traits`:

```rust
pub trait WrappingArithmetic:
WrappingAdd + WrappingSub + WrappingMul + WrappingNeg + WrappingShl + WrappingShr
{}
```

With a blanket implementation for any type that implements all six traits.

Then added `WrappingArithmetic` as a supertrait of `LinkReference`.

### Why this approach

1. **Addresses naming concern**: `WrappingArithmetic` is descriptive and covers all operations, not just addition
2. **Broader than requested**: Includes all 6 wrapping traits, not just `WrappingAdd`
3. **Compatible addition**: All unsigned primitive integer types already implement all 6 wrapping traits
4. **Follows existing patterns**: Same composite trait pattern as `Number` (which bundles `PrimInt + Default + Debug + AsPrimitive<usize> + ToPrimitive`)
5. **Preserves ecosystem naming**: Individual traits retain their `num-traits` names; the composite provides a convenient bundle

### Alternatives Considered

| Alternative | Pros | Cons |
|------------|------|------|
| Add only `WrappingAdd` | Minimal change | Doesn't address other wrapping ops; name issue remains |
| Add individual traits directly to `LinkReference` | No new trait needed | Longer trait definition; no reusable composite |
| Create `AdditionCapable` trait | Addresses naming | Too narrow; name implies only addition |
| Re-export `WrappingAdd` as a different name | Simple | Confusing; diverges from ecosystem naming |

## Impact on Downstream Crates

### `platform-data` (data-rs#17)
Currently the PR uses `T: LinkReference + WrappingAdd` everywhere. After this change is released as `platform-num` 0.8.0, `data-rs` can:
1. Remove the explicit `num-traits` dependency
2. Replace `T: LinkReference + WrappingAdd` with just `T: LinkReference`
3. All wrapping operations will be available through the `LinkReference` bound

### `doublets-rs`
Can use a single `T: LinkReference` bound for all numeric operations including wrapping arithmetic.

## Verification

- All 112 unit tests pass (96 existing + 16 new wrapping arithmetic tests)
- All 7 doc tests pass (including new `WrappingArithmetic` doctest)
- `cargo clippy --all-targets` reports zero warnings
- Verified that all 6 wrapping traits are implemented for `u8`, `u16`, `u32`, `u64`, `u128`, `usize`

## Related Issues and PRs

| Reference | Repository | Description |
|-----------|-----------|-------------|
| [#146](https://github.com/linksplatform/Numbers/issues/146) | Numbers | This issue — Add WrappingAdd to LinkReference |
| [#147](https://github.com/linksplatform/Numbers/pull/147) | Numbers | This PR — Implementation |
| [#16](https://github.com/linksplatform/data-rs/issues/16) | data-rs | Replace funty with platform-num |
| [#17](https://github.com/linksplatform/data-rs/pull/17) | data-rs | PR implementing funty replacement |
| [#47](https://github.com/linksplatform/doublets-rs/issues/47) | doublets-rs | Quality audit driving the unification |

## References

- [num-traits wrapping module documentation](https://docs.rs/num-traits/latest/num_traits/ops/wrapping/)
- [PrimInt trait documentation](https://docs.rs/num-traits/latest/num_traits/int/trait.PrimInt.html) — provides non-wrapping arithmetic
- [Rust RFC #1530](https://github.com/rust-lang/rfcs/issues/1530) — Discussion on WrappingAdd, WrappingSub, WrappingMul, WrappingDiv traits
1 change: 1 addition & 0 deletions docs/case-studies/issue-146/data-rs-issue-16-raw.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"body":"## Summary\n\nReplace `funty::Unsigned` and `FuntyPart` in the `LinkType` trait definition with `LinkReference` from `platform-num` (Numbers), eliminating the `funty` dependency entirely.\n\n## Current state\n\n`platform-data` 1.0.0 defines `LinkType` as:\n\n```rust\npub trait LinkType:\n Unsigned // from funty 2.0.0\n + FuntyPart // local trait wrapping TryFrom<u8>\n + TryFrom<i8, Error: Debug>\n + TryFrom<u8, Error: Debug>\n // ... all integer TryFrom/TryInto bounds\n{}\n```\n\nThis depends on `funty` 2.0.0 for the `Unsigned` trait. Meanwhile, `platform-trees` uses `num_traits::Unsigned` (via `platform-num::Number`), which is a **different trait** from a different crate. This creates incompatibility at the generic level — you can't write `T: data::LinkType + trees::LinkType` without dual bounds because neither implies the other.\n\n## Proposed change\n\nOnce linksplatform/Numbers#144 is implemented (adding `TryFrom`/`TryInto` bounds and `funty()` method to `LinkReference`), refactor `LinkType` to:\n\n```rust\npub trait LinkType: LinkReference {}\n```\n\nOr simply replace `LinkType` with `LinkReference` throughout the crate.\n\nThis would:\n1. **Remove `funty` from dependencies** — `Unsigned` from `num_traits` (included in `LinkReference` via `Number`) replaces `funty::Unsigned`\n2. **Remove `FuntyPart` trait** — the `funty()` method will be on `LinkReference` directly\n3. **Remove all explicit `TryFrom`/`TryInto` bounds** — they'll be part of `LinkReference`\n4. **Unify with `platform-trees`** — both crates will use the same base trait\n\n## Impact on downstream crates\n\n- `doublets-rs` can use a single `T: LinkReference` bound instead of `T: data::LinkType + trees::LinkType`\n- Any crate depending on `platform-data` for `LinkType` would get the same concrete type support\n\n## Related\n\n- linksplatform/Numbers#144 — Add required bounds to `LinkReference`\n- linksplatform/trees-rs issue — Corresponding change for trees\n- linksplatform/doublets-rs#47 — Original issue driving this unification\n- linksplatform/doublets-rs#48 — PR with full analysis","createdAt":"2026-04-14T10:12:34Z","state":"OPEN","title":"Replace funty dependency with platform-num (Numbers) for LinkType trait"}
1 change: 1 addition & 0 deletions docs/case-studies/issue-146/data-rs-pr-17-raw.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"body":"## Summary\n\n- Replaced `funty` dependency with `platform-num` (`LinkReference` trait) and `num-traits` (`WrappingAdd`)\n- **Removed `LinkType` trait entirely** — all bounds now use `LinkReference` directly\n- Replaced all `T::funty(n)` calls with `T::from_byte(n)` across the codebase\n- Re-exported `LinkReference` from crate root for downstream convenience\n\n## Motivation\n\n`platform-data` used `funty::Unsigned` while `platform-trees` used `num_traits::Unsigned` (via `platform-num::Number`). These are **different traits from different crates**, making it impossible to write `T: data::LinkType + trees::LinkType` without dual bounds. With `LinkReference` from `platform-num` 0.7.0, both crates now share the same base trait, enabling downstream crates like `doublets-rs` to use a single `T: LinkReference` bound.\n\nPer [review feedback](https://github.com/linksplatform/data-rs/pull/17#issuecomment-4243668806), `LinkType` has been removed entirely rather than kept as a thin wrapper.\n\n## Changes\n\n| File | Change |\n|---|---|\n| `Cargo.toml` | `funty` → `platform-num` + `num-traits` |\n| `src/link_type.rs` | **Deleted** — `LinkType` trait removed entirely |\n| `src/lib.rs` | Removed `mod link_type` and `pub use LinkType`; kept `pub use LinkReference` |\n| `src/hybrid.rs` | Uses `LinkReference + WrappingAdd` directly |\n| `src/constants.rs` | Uses `LinkReference + WrappingAdd` directly |\n| `src/converters.rs` | Uses `LinkReference + WrappingAdd` directly |\n| `src/links.rs` | Uses `LinkReference + WrappingAdd` directly |\n| `README.md` | Updated docs to reference `LinkReference` instead of `LinkType` |\n| `changelog.d/` | Updated breaking change fragment |\n\n## Breaking changes\n\n- `LinkType` trait **removed** (replaced by `LinkReference` from `platform-num`)\n- `FuntyPart` trait removed (replaced by `LinkReference::from_byte`)\n- `T::funty(n)` → `T::from_byte(n)` migration required for downstream users\n- All generic bounds changed from `T: LinkType` to `T: LinkReference + WrappingAdd`\n\n## Follow-up\n\n- linksplatform/Numbers#146 — Filed issue to add `WrappingAdd` to `LinkReference` trait bounds in `platform-num`, which would allow removing the explicit `+ WrappingAdd` bounds and the `num-traits` dependency\n\n## Test plan\n\n- [x] `cargo build` compiles cleanly\n- [x] `cargo clippy --all-targets` passes with zero warnings\n- [x] `cargo test --all-features` passes (all 94 tests + 1 doctest)\n- [ ] CI pipeline passes on GitHub Actions\n\nCloses #16\n\n🤖 Generated with [Claude Code](https://claude.com/claude-code)","createdAt":"2026-04-14T11:37:40Z","files":[{"path":"Cargo.lock","additions":27,"deletions":8,"changeType":"MODIFIED"},{"path":"Cargo.toml","additions":2,"deletions":1,"changeType":"MODIFIED"},{"path":"README.md","additions":5,"deletions":4,"changeType":"MODIFIED"},{"path":"changelog.d/20260414_replace_funty_with_platform_num.md","additions":14,"deletions":0,"changeType":"ADDED"},{"path":"src/constants.rs","additions":18,"deletions":16,"changeType":"MODIFIED"},{"path":"src/converters.rs","additions":5,"deletions":3,"changeType":"MODIFIED"},{"path":"src/hybrid.rs","additions":10,"deletions":7,"changeType":"MODIFIED"},{"path":"src/lib.rs","additions":1,"deletions":2,"changeType":"MODIFIED"},{"path":"src/link_type.rs","additions":0,"deletions":87,"changeType":"DELETED"},{"path":"src/links.rs","additions":5,"deletions":3,"changeType":"MODIFIED"}],"state":"OPEN","title":"feat!: replace funty with platform-num, remove LinkType"}
1 change: 1 addition & 0 deletions docs/case-studies/issue-146/doublets-rs-issue-47-raw.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"body":"Double check that our code is highest possible quality implementation for the same API it supports now. We should use all the latest versions of dependencies, and latest stable version of rust.\n\nMake sure docs are in sync with code, and fully describe all the features we have in the code.\n\nDouble check if it possible to increase tests coverage and docs coverage. And also if it possible to make automated documentation generation as we do in https://github.com/linksplatform/Numbers and https://github.com/linksplatform/trees-rs, and https://github.com/linksplatform/mem-rs, so the final website published in GitHub Pages contains generated documentation for all supported languages. Also code should contain all comments necessary to generate high quality documentation.\n\nWe should depend on real cargo/crates packages, not source code, so all submodules with dependencies source code can be removed.\n\nAlso no tests should be in src folder, use tests folder for tests.\n\nPlease fully support best practices of CI/CD from:\n- https://github.com/linksplatform/mem-rs\n- https://github.com/linksplatform/trees-rs\n- https://github.com/linksplatform/Numbers\n- https://github.com/link-foundation/rust-ai-driven-development-pipeline-template\n\nIf something is missing at https://github.com/link-foundation/rust-ai-driven-development-pipeline-template (make sure to compare full file tree for CI/CD) report the issue there.\n\nSo https://github.com/link-foundation/rust-ai-driven-development-pipeline-template will accumulate all the best practices of CI/CD for Rust from all our projects, so it will have highest possible quality and we will need less iterations to make our CI/CD work in all projects.\n\nWe need to collect data related about the issue to this repository, make sure we compile that data to `./docs/case-studies/issue-{id}` folder, and use it to do deep case study analysis (also make sure to search online for additional facts and data), list of each and all requirements from the issue, and propose possible solutions and solution plans for each requirement (we should also check known existing components/libraries, that solve similar problem or can help in solutions).","createdAt":"2026-04-13T08:55:31Z","state":"OPEN","title":"Double check we don't depend on any non stable features of rust and use all the latest best practices and versions of rust in the code"}
1 change: 1 addition & 0 deletions docs/case-studies/issue-146/issue-146-raw.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"author":{"id":"MDQ6VXNlcjE0MzE5MDQ=","is_bot":false,"login":"konard","name":"Konstantin Diachenko"},"body":"## Summary\n\n`LinkReference` should include `num_traits::WrappingAdd` (and potentially other wrapping arithmetic traits) in its supertraits.\n\n## Motivation\n\nAfter removing the `LinkType` trait from `platform-data` (linksplatform/data-rs#16), all bounds that were on `LinkType` need to come from `LinkReference` directly. The `WrappingAdd` trait is needed for wrapping unsigned arithmetic in `Hybrid` (used for internal/external link reference encoding).\n\nCurrently, `platform-data` must add `num-traits` as an explicit dependency and add `+ WrappingAdd` bounds alongside every `LinkReference` bound. If `LinkReference` included `WrappingAdd`, downstream crates could use `T: LinkReference` without additional trait bounds.\n\n## Proposed change\n\nAdd `WrappingAdd` (from `num-traits`) to the `LinkReference` trait definition:\n\n```rust\npub trait LinkReference:\n // ... existing bounds ...\n + WrappingAdd\n{}\n```\n\nSince `WrappingAdd` is implemented for all unsigned primitive integer types (the same types `LinkReference` is implemented for), this is a compatible addition.\n\n## Context\n\n- linksplatform/data-rs#16 — Replace funty with platform-num\n- linksplatform/data-rs#17 — PR implementing the change","comments":[{"id":"IC_kwDOC8wDk8788pyG","author":{"login":"konard"},"authorAssociation":"MEMBER","body":"I don't like the name of `WrappingAdd`, instead we should use something like `AdditionCapable`, check something similar already exists in num traites and we really need, and check what the best practice naming for that.\n\nWe also support similar for other operations that are usually supported by numeric types.\n\nWe need to download all logs and data related about the issue to this repository, make sure we compile that data to `./docs/case-studies/issue-{id}` folder, and use it to do deep case study analysis (also make sure to search online for additional facts and data), in which we will reconstruct timeline/sequence of events, list of each and all requirements from the issue, find root causes of the each problem, and propose possible solutions and solution plans for each requirement (we should also check known existing components/libraries, that solve similar problem or can help in solutions).\n\nIf there is not enough data to find actual root cause, add debug output and verbose mode if not present, that will allow us to find root cause on next iteration.\n\nIf issue related to any other repository/project, where we can report issues on GitHub, please do so. Each issue must contain reproducible examples, workarounds and suggestions for fix the issue in code.","createdAt":"2026-04-14T12:11:05Z","includesCreatedEdit":true,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/linksplatform/Numbers/issues/146#issuecomment-4243758214","viewerDidAuthor":true}],"createdAt":"2026-04-14T12:08:46Z","title":"Add WrappingAdd to LinkReference trait bounds"}
13 changes: 13 additions & 0 deletions experiments/check_wrapping.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use num_traits::ops::wrapping::*;

fn check<T: WrappingAdd + WrappingSub + WrappingMul + WrappingNeg + WrappingShl + WrappingShr>() {}

fn main() {
check::<u8>();
check::<u16>();
check::<u32>();
check::<u64>();
check::<u128>();
check::<usize>();
println!("All unsigned types implement all wrapping traits!");
}
16 changes: 16 additions & 0 deletions experiments/check_wrapping_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#[cfg(test)]
mod tests {
use num_traits::ops::wrapping::*;

fn check_all_wrapping<T: WrappingAdd + WrappingSub + WrappingMul + WrappingNeg + WrappingShl + WrappingShr>() {}

#[test]
fn test_all_unsigned_types_implement_wrapping() {
check_all_wrapping::<u8>();
check_all_wrapping::<u16>();
check_all_wrapping::<u32>();
check_all_wrapping::<u64>();
check_all_wrapping::<u128>();
check_all_wrapping::<usize>();
}
}
2 changes: 1 addition & 1 deletion rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading