Use `cfg_select!` macro instead of `cfg_if!` by ShaharNaveh · Pull Request #7636 · RustPython/RustPython
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 14c68ac5-93be-4151-b777-2ddf8a8913e9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
crates/stdlib/Cargo.toml
💤 Files with no reviewable changes (1)
- crates/stdlib/Cargo.toml
📝 Walkthrough
Walkthrough
Removed cfg-if workspace dependency and replaced multiple cfg_if! conditional blocks with cfg_select! across crates; bumped workspace rust-version from 1.93.0 to 1.95.0. No public API signatures or runtime behavior were changed.
Changes
| Cohort / File(s) | Summary |
|---|---|
Workspace manifests Cargo.toml, crates/common/Cargo.toml, crates/stdlib/Cargo.toml, crates/vm/Cargo.toml |
Removed cfg-if dependency entries from manifests; updated root/workspace rust-version from 1.93.0 → 1.95.0. |
Common lock logic crates/common/src/lock.rs |
Replaced cfg_if! with cfg_select! when selecting RawMutex/RawRwLock/RawThreadId and LazyLock implementation; no API changes. |
Stdlib: OpenSSL & resource crates/stdlib/src/openssl.rs, crates/stdlib/src/resource.rs |
Switched OpenSSL module selection, PROBE/set_default_verify_paths, and RLIM_NLIMITS constant selection from cfg_if! to cfg_select! (Android #[expect(deprecated)] preserved). |
Stdlib: socket crates/stdlib/src/socket.rs |
Converted platform conditionals for proto selection and fileno invalid checks to cfg_select!; behavior unchanged. |
VM manifests & concurrency impls crates/vm/Cargo.toml, crates/vm/src/builtins/type.rs |
Removed cfg-if from vm manifest; replaced cfg_if! with cfg_select! to gate unsafe impl Send/Sync for PyType. |
VM macros crates/vm/src/macros.rs |
Unified py_dyn_fn! macro and used cfg_select! inside expansion to choose trait-object bounds (adds Send + Sync only when feature = "threading"). |
VM object types crates/vm/src/object/core.rs, crates/vm/src/object/ext.rs, crates/vm/src/object/payload.rs |
Rewrote multiple cfg_if!-guarded unsafe impl Send/Sync blocks and PyThreadingConstraint definition to use cfg_select!; semantics preserved. |
VM stdlib modules crates/vm/src/stdlib/_io.rs, crates/vm/src/stdlib/_signal.rs, crates/vm/src/stdlib/_thread.rs, crates/vm/src/stdlib/os.rs, crates/vm/src/stdlib/posix.rs, crates/vm/src/stdlib/sys.rs |
Replaced many platform/feature cfg_if! branches with cfg_select! for constants, type selections, and platform strings (e.g., sys::PLATFORM now via cfg_select!; "macos" → "darwin", "windows" → "win32"). |
Stdlib sqlite3/threading impls crates/stdlib/src/_sqlite3.rs |
Replaced cfg_if! with cfg_select! around unsafe impl Send trait implementations; added explicit fallback arm. |
Shell helper src/shell/helper.rs |
Replaced cfg_if! controlling rustyline trait impls with cfg_select!, adding an explicit fallback arm for wasm32. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
🐰 I hopped through cfg-if's tangled lair,
swapped branches neat with cfg_select flair.
Same shapes, same bounds, one tiny version leap—
I nibble old macros, then cuddle up to sleep. 🥕✨
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly summarizes the main change: replacing cfg_if! conditional compilation with cfg_select! macro throughout the codebase. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 @coderabbitai help to get the list of available commands and usage tips.