Skip to content

chore(edition): port to Rust 2024 + declare MSRV 1.85 (#401) - #406

Merged
Mec-iS merged 4 commits into
developmentfrom
edition-2024
Aug 9, 2026
Merged

chore(edition): port to Rust 2024 + declare MSRV 1.85 (#401)#406
Mec-iS merged 4 commits into
developmentfrom
edition-2024

Conversation

@Mec-iS

@Mec-iS Mec-iS commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Complete fix for #401 — the edition-2024 porting epic. Closes #402 (A), #403 (B), #404 (C). D (#368, unsafesplit_at_mut) is deliberately deferred to its own PR: the bespoke raw-pointer iteration logic in matrix.rs::iterator_mut must be preserved per the "keep numerical-system logic and performance" constraint, and a safe rewrite deserves a dedicated review.

Changes

A — Edition flip (#402)

  • edition = "2021""2024" in Cargo.toml.
  • cargo fix --edition made zero auto-edits. The only behavioral-adjacent change:
    • linalg/basic/arrays.rs::approximate_eq rewritten to the 2024-safe tail-expr drop-order form — bind the owned intermediate (self.sub(other)) before the borrowing iterator, instead of chaining (self.sub(other)).iterator(0).all(...) in tail position (2024 reorders temporaries → the borrow would dangle). Numerical logic unchanged, only structural form. Annotated #[expect(clippy::let_and_return)] because this binding is load-bearing, not redundant.
  • AGENTS.md updated: edition line + a new "Edition 2024 invariants" section.

B — #[allow]#[expect] (#403)

  • Migrated #[allow(...)]#[expect(...)] at every site where the lint still fires under --all-features (verifies the suppression is honest).
  • Retained #[allow] at ~19 sites where the lint does not fire (e.g. clippy::ptr_arg, upper_case_acronyms, several dead_code) — converting those to #[expect] would trip unfulfilled_lint_expectations. Choice follows the TDD-skill guidance: #[expect] only where the lint actually fires.
  • lib.rs block split: 4 lints kept as #![expect], 2 demoted to #![allow].

C — MSRV declaration + CI verification (#404)

  • rust-version = "1.85" in Cargo.toml (edition 2024 stabilized in 1.85).
  • New msrv job in ci.yml: builds with dtolnay/rust-toolchain@1.85.0 (both --all-features and no-features) — this actually verifies the MSRV claim, not just declares it.
  • [lints.rust] unexpected_cfgs check-cfg table added for cfg(coverage, coverage_nightly) and cfg(tarpaulin) (edition-2024 unexpected_cfgs lint).
  • AGENTS.md commands section updated to add cargo +1.85.0 build --all-features and the -Drust-2024-compatibility clippy group.

Other

  • svm/svc.rs: removed two redundant let svc = ...; svc tail expressions surfaced by the 2024 clippy::let_and_return lint.
  • preprocessing/categorical.rs: kept the nested-if form (annotated #[allow(clippy::collapsible_if)]). Collapsing to a let-chain requires let-chains, which stabilized only in Rust 1.88 — incompatible with the declared MSRV 1.85. This was discovered by running cargo +1.85.0 build (not by guessing).
  • cargo fmt normalization across the tree (CI enforces --check).
  • AGENTS.md adds the "preserve bespoke numerical-system logic and performance" constraint for non-behavioral refactors.

Pre-migration audit (verified, read-only)

Verification (run, not guessed)

Gate Result
cargo fmt --all -- --check exit 0
cargo clippy --all-features -- -Drust-2018-idioms -Drust-2024-compatibility -Dwarnings exit 0, no warnings
cargo test --all-features exit 0 — 474 unit, 68+3 doctests
cargo +1.85.0 build --all-features (MSRV) exit 0
cargo +1.85.0 build (no features, MSRV) exit 0

Version bumped 0.6.10.6.2; CHANGELOG updated under [0.6.2].

Checklist

  • Targets development
  • cargo fmt --all -- --check
  • cargo clippy --all-features -- -Drust-2018-idioms -Dwarnings
  • cargo clippy --all-features -- -Drust-2024-compatibility -Dwarnings
  • cargo test --all-features
  • MSRV verified at 1.85
  • CHANGELOG updated
  • AGENTS.md invariants documented

Closes #401, #402, #403, #404.

A (#402): edition 2021 -> 2024. cargo fix --edition made no auto-edits;
the only behavioral-adjacent change is approximate_eq rewritten to the
2024-safe tail-expr drop-order form (bind owned intermediate before the
borrowing iterator) -- numerical logic unchanged.

B (#403): #[allow] -> #[expect] where the lint still fires under
--all-features; #[allow] retained where the lint does not fire (avoids
unfulfilled_lint_expectations).

C (#404): declare rust-version = "1.85" in Cargo.toml and add an msrv
CI job that builds with dtolnay/rust-toolchain@1.85.0 to verify the
claim. Added [lints.rust] unexpected_cfgs check-cfg table for
cfg(coverage, coverage_nightly) and cfg(tarpaulin).

Also:
- AGENTS.md: document edition-2024 invariants (no RPIT, explicit
  dyn Trait + 'a, tail-expr drop-order, lint-suppression policy, unsafe
  stance) and the 'preserve bespoke numerical-system logic and
  performance' constraint for non-behavioral refactors.
- svm/svc.rs: drop two redundant let-and-return tails surfaced by the
  2024 clippy::let_and_return lint.
- preprocessing/categorical.rs: keep nested-if (annotated
  #[allow(clippy::collapsible_if)]) because collapsing to a let-chain
  needs let-chains, unstable until 1.88 -- incompatible with MSRV 1.85.
- cargo fmt normalization across the tree.

D (#368, unsafe -> split_at_mut) is deferred to its own PR per the
'preserve numerical logic' constraint.

Verified: fmt=0, clippy --all-features -Drust-2018-idioms
-Drust-2024-compatibility -Dwarnings=0, test --all-features=0 (474 unit
+ 68+3 doctests), cargo +1.85.0 build --all-features=0.

Bump 0.6.1 -> 0.6.2.
@Mec-iS Mec-iS added enhancement New feature or request rust Pull requests that update rust code labels Aug 9, 2026
Mec-iS added 2 commits August 9, 2026 14:58
- Install pin: ^0.5.3 -> ^0.6 (current is 0.6.2).
- Quick-start example: fix .unwrap -> .unwrap() (was a real typo that
  would not compile as a doctest).
- Roadmap: 'Move to Rust 2021 edition' -> 'Move to Rust 2024 edition
  (MSRV 1.85)'.
- Contributing: note MSRV 1.85 / edition 2024 alongside 'stable Rust'.
- Replace stale 'v0.4 API' prose references with version-agnostic
  'smartcore API' wording to stop the version label going stale again.
@Mec-iS

Mec-iS commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

Overall Assessment

PR #406 is a thorough, disciplined Rust 2024 migration. The scope is clearly bounded — A (edition flip), B (#[allow]#[expect]), C (MSRV + CI), plus cosmetic cargo fmt fallout — and the deferred D (368, unsafesplit_at_mut) is a correct call . All verification gates are green per the PR description.


A — Edition flip (#402)

linalg/basic/arrays.rs::approximate_eq — tail-expr drop order fix

The rewrite is correct and the #[expect(clippy::let_and_return, reason = "...")] annotation is exactly right . The reason field documents why this binding is load-bearing, which is crucial — without it, a future contributor would confidently remove what looks like a redundant local. One minor observation: the current form introduces a second redundant binding (let result = ...; result) that clippy::let_and_return would normally flag. The #[expect] suppresses the whole lint at the function level, so this is fine mechanically, but you could simplify to just:

let diff = self.sub(other);
diff.iterator(0).all(|v| v.abs() <= error)

This removes the need for #[expect] entirely, keeps the owned-intermediate binding that 2024 requires, and lets the return expression be idiomatic. Worth a follow-up cleanup.


B — #[allow]#[expect] (#403)

The split in lib.rs is correct: type_complexity, too_many_arguments, many_single_char_names, unnecessary_wraps move to #![expect] (lint fires); upper_case_acronyms and approx_constant stay as #![allow] (lint doesn't fire, avoiding unfulfilled_lint_expectations) . The site-by-site migration across cholesky.rs, evd.rs, lu.rs, qr.rs, svd.rs, mahalanobis.rs, lbfgs.rs, svm/mod.rs, svm/svc.rs, logistic_regression.rs, knn_regressor.rs, xgb_regressor.rs, cosinepair.rs, fastpair.rs, quick_sort.rs, cosine.rs, preprocessing/numerical.rs, model_selection/mod.rs, naive_bayes/mod.rs, dataset/mod.rs, matrix.rs is thorough .

One concern: logistic_regression.rs converts df's #[allow(clippy::ptr_arg)] to #[expect] but keeps the second #[allow(clippy::ptr_arg)] on the trait definition below it as #[allow] . This is intentional per the PR description (the second site apparently doesn't fire), but it creates a visual inconsistency — two ptr_arg suppressions on adjacent trait methods with different attributes. A comment explaining why the second one stays #[allow] would prevent future confusion.


C — MSRV declaration + CI (#404)

ci.yml — new msrv job

The job is correct and builds both --all-features and no-features at 1.85.0 . Two minor issues:

  • TZ: "/usr/share/zoneinfo/your/location" — this is a placeholder left in the CI job . It has no effect on the build but looks like an unfinished template copy-paste. Should be removed or replaced with a real timezone (or just dropped entirely; Rust builds don't depend on TZ).
  • The msrv job has no needs: dependency on the main ci job. This means it runs in parallel, which is fine for correctness, but if the main CI job fails early (e.g. cargo fmt), the MSRV job still consumes runner time. Adding needs: [ci] (or the equivalent matrix job name) would be a minor efficiency improvement, not a blocker.

preprocessing/categorical.rs — let-chains MSRV gate

The #[allow(clippy::collapsible_if, reason = "MSRV 1.85: let-chains unstable, cannot collapse")] is the right approach . The note in AGENTS.md records this constraint clearly. No issues.

numbers/floatnum.rs and numbers/realnum.rsln_1pe reformatting

cargo fmt collapsed the 4-line if/else bodies into single-line form . The math is unchanged — this is purely cosmetic formatting — but it's worth flagging that these 4 occurrences (f64 and f32 × 2 files) are slightly less readable in the one-liner form. Not a blocker, but if the codebase style preference is readability over compactness in numeric code, these could be reverted with a // rustfmt::skip annotation. Given the AGENTS.md constraint to "preserve bespoke numerical-system logic," keeping these expanded would be consistent with that spirit.


svm/svc.rs — redundant let svc = ...; svc removal

Clean . Both fit and fit_multiclass_component now return Self::optimize_and_fit(...) directly. No behavioral change.


README.md — second commit

The ^0.5.3^0.6 pin update and the .unwrap.unwrap() doctest typo fix are both correct and overdue . The version-agnostic "smartcore API" wording (replacing "v0.4 API") will stop the README from going stale again.


Summary of Actionable Notes

Location Severity Note
linalg/basic/arrays.rs::approximate_eq 🟡 Minor Simplify to let diff = self.sub(other); diff.iterator(0).all(...) — removes need for #[expect] entirely
logistic_regression.rs 🟡 Minor Second #[allow(clippy::ptr_arg)] on adjacent method lacks an explanation comment; add // lint does not fire here
.github/workflows/ci.yml 🟠 Fix TZ: "/usr/share/zoneinfo/your/location" is an unfilled placeholder — remove it
.github/workflows/ci.yml 🟡 Minor msrv job has no needs: — consider adding to avoid wasted runner time on upstream failures
numbers/floatnum.rs, numbers/realnum.rs 🟡 Style ln_1pe collapsed to one-liner by cargo fmt; consider #[rustfmt::skip] if numeric code readability is preferred

The TZ placeholder is the only item that should be fixed before merge — it's a broken template artifact. Everything else is polish. PR #406 is ready to merge once the TZ line is removed.

Sources
[1] chore(edition): port to Rust 2024 + declare MSRV 1.85 (#401) by Mec-iS · Pull Request #406 · smartcorelib/smartcore #406

Per PR #406 review feedback:
- ci.yml: remove the unfilled TZ template placeholders from all three
  jobs (broken template artifact; Rust builds don't depend on TZ).
- arrays.rs::approximate_eq: drop the redundant let result = ...; result
  + the #[expect(clippy::let_and_return)] — under edition 2024 the drop
  order changed so the simpler
  form compiles cleanly (no E0597, no clippy warning). Numerical logic
  unchanged.
@Mec-iS
Mec-iS merged commit ed49b31 into development Aug 9, 2026
13 checks passed
@Mec-iS
Mec-iS deleted the edition-2024 branch August 9, 2026 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request rust Pull requests that update rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A: flip to edition 2024 (cargo fix --edition, set edition="2024", update docs) Bump Rust edition to 2024 + adopt relative idioms

1 participant