Update to edition 2024, MSRV 1.85#231
Conversation
- Replace `extern "C" {}` doc-include hack with `#[doc = include_str!(...)]`
- Fix edition 2024 binding mode change in `min_by_key` closure
- Apply `is_none_or` (stabilized 1.82) replacing `map_or(true, ...)`
- Update CI to modern actions (checkout@v4, dtolnay/rust-toolchain)
- Sync clippy.toml MSRV
Verified: all 39 tests pass, `cargo msrv verify` confirms 1.85.0.
- Add resolver = "2" to avoid edition 2024's v3 resolver default - Delete clippy.toml; clippy reads rust-version from Cargo.toml since 1.64
This isn't true. You still need unsafe, it just gets hidden/abstracted. I was in the process of writing up a long response, but realized there's no point in arguing with what's probably an LLM hallucination. |
197g
left a comment
There was a problem hiding this comment.
I'll take this one with the version it really needs, i.e. as-is. With rust-version it makes little difference if we bump it now or later, so I'd rather do that on actual demand than on preemptive philosophistry.
Re: simd and llm debate: I've already looked at archmage before, it's at least is an interesting approach; fn(Token, ..) is a kind of newtype wrapper choice to annotate an original unsafe fn with; which is the direction I would have liked std's cpu-flag macro to have gone in from the start yet it has a huge design space. So it's good to explore one very clear direction here. There's a bit of duplication though and this together with almost everything happening inside proc-macros does, by-design, not aid the argument that off-loading unsafe into the crate makes the safety more reviewable. (Which is the important part since if we depended on cve-rs we could also forbid() but it'd be disastrous).
|
I think there is an argument that we can trust is_*_feature_detected(x), and that you can make a provably sound abstraction on top of that which is simple enough to audit. That said, it does generate a trampoline, which does contain an unsafe block to call the inner function - the function itself is not unsafe annotated, though, so the only unsafety is the target_feature hop/switch. Rust itself computes the set math at compile time to determine if a call is safe or if you will be required to get a new capability token. Archmage takes the generate-everything approach to ensure token types never get mismatched, and most of the complexity has been around how to group features into pragmatic supersets, name things, make such code testable, and ergonomics. Using a cpu instruction a cpu doesn't support is unsound, true, but in reality is quite like exit() rather than something exploitable, AFAIK. I haven’t had any such instructions slip through the cracks yet. The dependency safe_unaligned_simd crate is more load-bearing, but trivial to audit in a different way - it's just checking slices-to-pointer logic. I don't own that crate, but I trust it. Given archmage+forbid unsafe, I feel comfortable letting LLVMs prototype with intrinsics all day and explore every idea I can come up with. Magetypes, on the other hand, takes an approach similar to fearless_simd (something I wish I had looked at earlier), and uses generics and associated types to make simd use 95% as fast as raw intrinsics. It is much harder to audit, so I parse the rust source code to build a db of which intrinsics rust considers safe per feature token, then use that for generating and validating the simd types. It's labeled experimental, but it's actually working better than I hoped. I haven’t run into any soundness holes with either crate yet. |
|
The design problem of generating There are more convincing ways to write a proc-macro; the derives of |
Summary
actions/checkout@v4,dtolnay/rust-toolchain)resolver = "2"to avoid edition 2024's v3 resolver defaultclippy.toml(clippy readsrust-versionfromCargo.tomlsince 1.64)Code changes required by edition 2024
Doc-include macro: The
extern "C" {}hack for embedding README doctests doesn't work in edition 2024 (unsafe externis now required, which conflicts with#![forbid(unsafe_code)]). Replaced with the standard#[doc = include_str!("../README.md")] mod _readme {}pattern.Binding mode: Edition 2024 changes implicit reference binding in closures.
|(_, &value)|becomes|&(_, &value)|inmin_by_key(one occurrence incommon.rs).is_none_or: Applied clippy's suggestion to replacemap_or(true, ...)withis_none_or(...)(stabilized in 1.82, two occurrences).All 39 tests pass.
cargo msrv verifyconfirms 1.85.0 compatibility.Discussion: consider going to 1.88 or 1.89
Since this is a large MSRV jump anyway (1.62 → 1.85), it may be worth going further. The
imagecrate is already on MSRV 1.88, so any user pulling ingifthroughimagealready needs 1.88+.Case for 1.88
[T]::as_chunks/as_rchunks— process pixel data in fixed-width chunks (3-byte RGB, 4-byte RGBA) without bounds checks or manual index math. Directly applicable tofrom_rgba_speed,from_rgb_speed,from_grayscale_with_alpha.letchains (edition 2024 only) —if let Some(x) = a && let Some(y) = breplaces nestedif letladders in parser code.imageis already there —image 0.25.10requires 1.88, so the primary downstream consumer already demands it.Case for 1.89
1.89 is where Rust's SIMD story came together across four releases:
#[target_feature]functions can be safe — composing SIMD code no longer needsunsafeat every call site between matching feature contexts.std::archintrinsics (arithmetic, shuffle, compare, bitwise) are now safe inside#[target_feature]functions. Only pointer-based ops (loadu/storeu) remainunsafe.as_chunksgives zero-cost fixed-size array views into slices — exactly what safe SIMD loads need.is_x86_feature_detected!("avx512f")) works on stable. The entire AVX-512 family was previously locked behind nightly-only feature gates.Together, these mean a crate on MSRV 1.89 can use
#![forbid(unsafe_code)]while still doing SIMD dispatch — nounsafe, no nightly, no feature gates. This matters for the broader image-rs ecosystem as codecs move toward safe SIMD. See archmage's MSRV rationale for the full breakdown.This PR targets 1.85 as the conservative baseline. Happy to bump to 1.88 or 1.89 if preferred.
Test plan
cargo test --all-features— 39 tests passcargo clippy --all-features— clean (one pre-existingif_same_then_elsewarning)cargo msrv verify— confirms 1.85.0cargo fmt --check— cleancargo-copter— no regressions from the edition change itself