no_std for common, pylib, codegen#7056
Conversation
Sorry, something went wrong.
📝 WalkthroughWalkthroughAdds optional no_std support across multiple crates by introducing std feature flags, applying crate-level no_std gating, switching std→core imports, and providing cfg-based alternative implementations for panic handling, static storage, and thread-local behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@crates/codegen/Cargo.toml`:
- Around line 11-13: The crate's Cargo.toml currently toggles a local "std"
feature but doesn't propagate it to dependencies, so dependencies like thiserror
and itertools will still enable std; either (A) update workspace dependency
entries for thiserror and itertools to set default-features = false (and add the
needed features like itertools' use_alloc) so they truly work in no_std, or (B)
add feature forwarding in this crate's Cargo.toml by making the std feature
enable the dependencies' std/use_std/use_alloc equivalents (e.g., map this
crate's "std" feature to "thiserror/std" and the appropriate itertools feature)
so disabling std here also disables std in those deps; apply this change to the
Cargo.toml features and/or workspace dependency declarations and ensure feature
names match the dependency crates (thiserror, itertools).
In `@crates/codegen/src/symboltable.rs`:
- Around line 325-341: The PopGuard using a raw *mut pointer is unsound because
calling f(self) re-borrows self and invalidates that pointer; replace the
raw-pointer Drop guard in with_append with a safe length/index-based approach:
record the previous length (or index) of self.v, push x.into(), call f(self) to
get the result, then after f returns pop the element by checking/restoring based
on the recorded length (or pop unconditionally if you know you always pushed
one). Remove the PopGuard type and raw pointer usage (references: with_append,
PopGuard, self.v, f(self)).
In `@crates/common/src/refcount.rs`:
- Around line 38-40: The refcount overflow check currently uses panic! which
weakens safety; change the overflow handling in the refcount increment paths
(the branch checking if old_size & Self::MASK == Self::MASK in the method(s)
that perform increments—e.g., the function implementing inc and inc_by) to call
std::process::abort() when the standard library is available and only use panic!
as a fallback in no_std builds; implement this via cfg(feature =
"std")/cfg(not(feature = "std")) or equivalent cfg!(std) guards so the overflow
always aborts in std builds and apply the exact same change to the inc_by
overflow check as well.
Sorry, something went wrong.
- Gate OS-dependent modules behind `#[cfg(feature = "std")]` - Replace `std::f64` with `core::f64` in float_ops - Replace `std::process::abort` with panic in refcount - Remove `thread_local` from levenshtein (stack buffer) - Split static_cell into threading/non_threading/no_std
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/common/src/str.rs`:
- Line 536: The left-edge initialization in the Levenshtein DP loop is off by
one: update the assignment that sets result for the current row (using variables
result, b_index, and constant MOVE_COST inside the DP routine) so that it uses
(b_index + 1) multiplied by MOVE_COST instead of b_index multiplied by
MOVE_COST, ensuring the row deletion cost corresponds to row b_index+1.
🧹 Nitpick comments (2)
crates/common/src/static_cell.rs (1)
123-126: Impreciseunsafesafety justification —no_stddoes not preclude threading.The comment "without std there can be no threads" is not accurate. Many
no_stdenvironments (RTOS, multi-core bare-metal) support threading. The actual safety invariant here is that thethreadingfeature is disabled, which is a user contract that concurrent access won't occur. Consider revising the comment to reflect the real guarantee:Suggested comment fix
- // unsync::OnceCell is !Sync, but without std there can be no threads. - struct SyncOnceCell<T>(OnceCell<T>); - // SAFETY: Without std, threading is impossible. - unsafe impl<T> Sync for SyncOnceCell<T> {} + // unsync::OnceCell is !Sync, but this path is only active when the + // `threading` feature is disabled, meaning the caller guarantees + // no concurrent access. + struct SyncOnceCell<T>(OnceCell<T>); + // SAFETY: The `threading` feature is off, so the user has opted out of + // multi-threaded use. No concurrent access to the inner cell can occur. + unsafe impl<T> Sync for SyncOnceCell<T> {}crates/common/Cargo.toml (1)
12-13: Thestdfeature doesn't forward to dependency feature gates, which prevents trueno_stdbuilds.If you plan to support
no_std, dependencies likeonce_cellanditertoolsneed conditional feature forwarding. The currentstd = []is inert—disabling it doesn't disablestdin transitive dependencies.Consider extending the feature once
no_stdis fully supported:std = ["once_cell/std", "itertools/use_std"](Note:
getrandomis already explicitly enabled withstdin workspace.dependencies.malachite-biginthas astdfeature but doesn't fully supportno_stdat the crate level, so forwarding it won't be sufficient for trueno_stdbuilds.)If this is an incomplete incremental step, a tracking issue would help clarify the scope.
Sorry, something went wrong.
youknowone
merged commit 570d50c
into
RustPython:main
Feb 9, 2026
Summary by CodeRabbit
New Features
Bug Fixes
Other