Repository navigation
fix(kwin): replace unstable atomic_try_update with stable fetch_add - #252
Merged
Merged
Conversation
AtomicU64::try_update is still unstable on stable Rust and is only usable behind the unstable-library-feature gate, so this crate fails to build with E0658 on pinned toolchains that predate its stabilisation. fetch_add returns the previous value; checking it with checked_add reproduces exactly the overflow condition the try_update closure existed to detect, using stable primitives only. This is the sole unstable library usage in the crate. Verified on Ubuntu 26.04 with rustc 1.93.1: cargo build --release succeeds and cargo test --lib passes 433/433.
There was a problem hiding this comment.
This is an auto review done by revuto.
Revuto completed the review and found no evidence-backed concerns.
Collaborator
|
Reviewed 341f948 independently. No blocking findings. Reproduced E0658 on Rust 1.93.1. The proposed atomic code compiles there, and all-target compilation on 1.93.1 passed. Full local tests and formatting passed. CI passed and Revuto approved this head. Copilot hit its quota. The sequence now starts at 1, and overflow resets the counter. The comment overstates equivalence. Both differences are harmless for this private counter, which starts at zero and is paired with a random nonce. This does not block merging. Thanks @MubarakSec for the fix and the distro-toolchain validation. |
Merged
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.
Problem
cargo buildfails witherror[E0658]: use of unstable library feature 'atomic_try_update'atsrc/windowing/backends/kwin.rs:427.AtomicU64::try_updateis not yet stabilised on stable Rust, so the crate does not compile on any toolchain pinned below its stabilisation — including the rustc shipped by current Ubuntu LTS.Fix
fetch_addreturns the previous value, sochecked_addon it reproduces exactly the overflow condition thetry_updateclosure existed to detect. Stable primitives only, no feature gate, no behaviour change on any reachable input.This was the sole unstable-library usage in the crate.
Testing
Verified on Ubuntu 26.04 / GNOME 50 / Wayland, rustc 1.93.1:
cargo build --release— succeeds (previously failed)cargo test --locked --no-fail-fast— 433/433 passcargo clippy --locked --all-targets -- -D warnings— cleancargo fmt --all -- --check— cleanscripts/mcp_safety_check.py— passesNote on CI visibility
CI pins
dtolnay/rust-toolchain@stable, so this is invisible there — it only bites contributors building against distro Rust. Happy to instead add arust-toolchain.toml, or a CI job against a pinned older toolchain, if that would be preferable to patching the call site.