Use cfg_select! macro instead of cfg_if!#7636
Use cfg_select! macro instead of cfg_if!#7636ShaharNaveh wants to merge 1 commit intoRustPython:mainfrom
cfg_select! macro instead of cfg_if!#7636Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request replaces the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
67f5ebc to
8db3293
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/vm/src/stdlib/posix.rs (1)
1805-1820:⚠️ Potential issue | 🔴 CriticalRemove the stray
iffrom thecfg_select!arm.Line 1807 uses
if any(...) =>, but Rust 1.95cfg_select!arms use the cfg predicate directly without theifkeyword (any(...) =>). This is a compile-blocking syntax error. All othercfg_select!usages in the codebase follow the correct syntax withoutif.🐛 Proposed fix
// Note: POSIX_SPAWN_SETSID may not be available on all platforms cfg_select! { - if any( + any( target_os = "linux", target_os = "haiku", target_os = "solaris", target_os = "illumos", target_os = "hurd", ) => { flags.insert(nix::spawn::PosixSpawnFlags::from_bits_retain(libc::POSIX_SPAWN_SETSID));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/stdlib/posix.rs` around lines 1805 - 1820, The cfg_select! arm in the flags handling block incorrectly uses the `if` keyword ("if any(...) =>") which is invalid syntax; edit the cfg_select! invocation (the block that inserts nix::spawn::PosixSpawnFlags::from_bits_retain(libc::POSIX_SPAWN_SETSID) and returns a new_not_implemented_error) to remove the stray `if` so the arm uses the predicate directly (e.g., `any(...) =>`), leaving the flags.insert(...) and the `_ => { return Err(vm.new_not_implemented_error(...)); }` arm unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/stdlib/src/openssl.rs`:
- Around line 35-42: The PROBE static initializer's cfg_select! branches end
with trailing semicolons making each branch evaluate to () instead of returning
a LazyLock<ProbeResult>; remove the semicolons after
LazyLock::new(openssl_probe::probe) and LazyLock::new(|| ProbeResult {
cert_file: None, cert_dir: vec![] }) so each branch yields a
LazyLock<ProbeResult> (affecting the PROBE static, the cfg_select! macro,
LazyLock::new invocations, and the ProbeResult construction).
In `@crates/vm/src/stdlib/_thread.rs`:
- Around line 34-39: The cfg_select! arms for PYTHREAD_NAME currently list the
broader unix => Some("pthread") arm before the more specific any(target_os =
"solaris", target_os = "illumos") arm, so Solaris/illumos targets never reach
their intended match; fix by reordering the cfg_select! arms in the
PYTHREAD_NAME constant so the any(target_os = "solaris", target_os = "illumos")
arm appears before the unix arm, preserving the other arms (windows and default)
as-is.
In `@crates/vm/src/stdlib/sys.rs`:
- Around line 248-255: The PLATFORM constant in sys.rs currently maps target_os
"macos" and Windows to non-CPython values; update the cfg_select! mapping in the
pub const PLATFORM definition so that target_os = "macos" => "darwin" and
windows => "win32" (leave the other arms unchanged) to match CPython's
sys.platform values; modify the PLATFORM const in crates/vm/src/stdlib/sys.rs
accordingly and run the build/tests to verify.
---
Outside diff comments:
In `@crates/vm/src/stdlib/posix.rs`:
- Around line 1805-1820: The cfg_select! arm in the flags handling block
incorrectly uses the `if` keyword ("if any(...) =>") which is invalid syntax;
edit the cfg_select! invocation (the block that inserts
nix::spawn::PosixSpawnFlags::from_bits_retain(libc::POSIX_SPAWN_SETSID) and
returns a new_not_implemented_error) to remove the stray `if` so the arm uses
the predicate directly (e.g., `any(...) =>`), leaving the flags.insert(...) and
the `_ => { return Err(vm.new_not_implemented_error(...)); }` arm unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 3c4d5dab-4eac-460c-b598-031fda07a667
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
Cargo.tomlcrates/common/Cargo.tomlcrates/common/src/lock.rscrates/stdlib/Cargo.tomlcrates/stdlib/src/openssl.rscrates/stdlib/src/resource.rscrates/stdlib/src/socket.rscrates/vm/Cargo.tomlcrates/vm/src/builtins/type.rscrates/vm/src/macros.rscrates/vm/src/object/core.rscrates/vm/src/object/ext.rscrates/vm/src/object/payload.rscrates/vm/src/stdlib/_io.rscrates/vm/src/stdlib/_signal.rscrates/vm/src/stdlib/_thread.rscrates/vm/src/stdlib/os.rscrates/vm/src/stdlib/posix.rscrates/vm/src/stdlib/sys.rs
💤 Files with no reviewable changes (3)
- crates/common/Cargo.toml
- crates/vm/Cargo.toml
- crates/stdlib/Cargo.toml
| static PROBE: LazyLock<ProbeResult> = cfg_select! { | ||
| openssl_vendored => { | ||
| LazyLock::new(openssl_probe::probe); | ||
| } | ||
| _ => { | ||
| LazyLock::new(|| ProbeResult { cert_file: None, cert_dir: vec![] }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify cfg_select expression-context usage patterns and likely mismatch points.
set -euo pipefail
echo "== cfg_select! usages in expression contexts =="
rg -n -C2 '=\s*cfg_select!\s*\{' --type rust
echo
echo "== Branches ending with semicolon inside cfg_select! (quick heuristic) =="
rg -n -U --type rust 'cfg_select!\s*\{(?s).*?=>\s*\{\s*[^{}]*;\s*\}'Repository: RustPython/RustPython
Length of output: 50378
🏁 Script executed:
sed -n '33,42p' crates/stdlib/src/openssl.rsRepository: RustPython/RustPython
Length of output: 410
🏁 Script executed:
# Find cfg_select! macro definition or doc comments
rg -n 'macro_rules!\s*cfg_select' --type rust -A 5Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Check if code compiles
cargo check --manifest-path crates/stdlib/Cargo.toml 2>&1 | head -50Repository: RustPython/RustPython
Length of output: 6838
🏁 Script executed:
# Search for cfg_select! macro definition/use
rg -n 'cfg_select' crates/stdlib/src/openssl.rs | head -5Repository: RustPython/RustPython
Length of output: 167
🏁 Script executed:
# Find where cfg_select is imported from or defined
rg -n 'use.*cfg_select|macro.*cfg_select' --type rust -B 2 -A 2Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Check Cargo.toml for rustpython-compiler or similar dependency that provides cfg_select
fd Cargo.toml -path '*/stdlib/*' -exec grep -l 'cfg' {} \;Repository: RustPython/RustPython
Length of output: 233
🏁 Script executed:
# Search in all Cargo.toml for cfg_select or rustpython-compiler
rg -n 'cfg_select|rustpython.compiler' --type rust | head -20Repository: RustPython/RustPython
Length of output: 2016
🏁 Script executed:
# Find where cfg_select comes from - check imports at top of openssl.rs
head -30 crates/stdlib/src/openssl.rsRepository: RustPython/RustPython
Length of output: 675
🏁 Script executed:
# Search for cfg_select in compiler crate
fd 'lib.rs|mod.rs' -path '*/compiler/*' | head -5Repository: RustPython/RustPython
Length of output: 233
🏁 Script executed:
# Broader search for cfg_select definition
rg 'macro_rules.*cfg_select|pub.*cfg_select' -A 10 --type rustRepository: RustPython/RustPython
Length of output: 1451
🏁 Script executed:
# Look for cfg_select definition in rustpython-compiler
find . -name '*.rs' -path '*compiler*' -exec rg -l 'cfg_select' {} \;Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Check Cargo.toml for dependencies that provide cfg_select
cat crates/stdlib/Cargo.toml | grep -A 10 -B 10 cfg_selectRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Look for the macro in common or macros crate
find crates -name 'lib.rs' -o -name 'macros.rs' | xargs rg -l 'cfg_select' 2>/dev/nullRepository: RustPython/RustPython
Length of output: 88
🏁 Script executed:
# Compare with other correct cfg_select usage - check the exact pattern
rg -n 'cfg_select!\s*\{' -A 8 crates/vm/src/stdlib/sys.rs | head -20Repository: RustPython/RustPython
Length of output: 403
🏁 Script executed:
# Examine the cfg_select! macro definition
cat crates/vm/src/macros.rs | head -100Repository: RustPython/RustPython
Length of output: 3374
🏁 Script executed:
# Search for cfg_select macro definition specifically
rg -n 'cfg_select' crates/vm/src/macros.rs -A 20Repository: RustPython/RustPython
Length of output: 349
🏁 Script executed:
# Also check if it's imported from elsewhere
rg -n 'cfg_select' crates/vm/src/lib.rsRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Look for cfg_select in the entire codebase definition
rg 'macro_rules!\s*cfg_select' -A 30Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Search for cfg_select definition more broadly
rg 'cfg_select' --type rust -l | head -10Repository: RustPython/RustPython
Length of output: 357
🏁 Script executed:
# Check if cfg_select is from an external crate by looking at Cargo.lock or Cargo.toml
grep -r 'cfg_select' Cargo.lock 2>/dev/null | head -5Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Try to find it by looking at commonly available macros
rg 'macro_rules' crates/vm/src/macros.rs | grep cfgRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Check if cfg_select might be from rustpython_common or rustpython_compiler_core
find crates -name '*.rs' | xargs grep -l 'macro_rules.*cfg_select'Repository: RustPython/RustPython
Length of output: 49
🏁 Script executed:
# Check all Cargo.toml files for cfg_select or cfg-if dependencies
find crates -name Cargo.toml -exec grep -H 'cfg' {} \; | head -30Repository: RustPython/RustPython
Length of output: 1596
🏁 Script executed:
# Look for the macro by checking imports in macros.rs more thoroughly
head -20 crates/vm/src/macros.rsRepository: RustPython/RustPython
Length of output: 911
🏁 Script executed:
# Check what's in stdlib Cargo.toml dependencies
cat crates/stdlib/Cargo.toml | head -50Repository: RustPython/RustPython
Length of output: 1919
🏁 Script executed:
# Check rustpython-derive for macro definitions
find crates -path '*/rustpython-derive/*' -name '*.rs' | xargs rg 'cfg_select' 2>/dev/nullRepository: RustPython/RustPython
Length of output: 1552
🏁 Script executed:
# Look in rustpython-common
find crates -path '*/rustpython-common/*' -name '*.rs' | xargs rg 'cfg_select' 2>/dev/nullRepository: RustPython/RustPython
Length of output: 1552
🏁 Script executed:
# Try to compile a minimal test to see if code with semicolons actually fails
cat > /tmp/test_cfg_select.rs << 'EOF'
cfg_select! {
true => {
fn foo() { }
}
_ => {
fn foo() { }
}
}
EOFRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Check if cfg_select is maybe from a crate re-export
rg -n 'pub.*cfg_select|use.*cfg_select' crates/vm/src/lib.rs crates/stdlib/src/lib.rs 2>/dev/nullRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Look for it in external macro definition
rg 'macro_export.*cfg_select|cfg_select.*macro_export' crates -A 5Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Look at line 10 usage which is in a module context (statement position)
sed -n '10,25p' crates/stdlib/src/openssl.rsRepository: RustPython/RustPython
Length of output: 408
🏁 Script executed:
# Compare: look at a const assignment usage
sed -n '16,21p' crates/stdlib/src/resource.rsRepository: RustPython/RustPython
Length of output: 276
🏁 Script executed:
# Check if cfg_select comes from rustpython-derive crate
grep -r 'cfg_select' crates/derive/src 2>/dev/null || echo "Not in derive"Repository: RustPython/RustPython
Length of output: 78
🏁 Script executed:
# Look for it in common crate
grep -r 'cfg_select' crates/common/src 2>/dev/null | head -10Repository: RustPython/RustPython
Length of output: 144
🏁 Script executed:
# Check if it's maybe a re-export from a crate like cfg-if
cargo tree -p rustpython-stdlib 2>/dev/null | grep -i 'cfg\|select' || echo "Cannot check tree"Repository: RustPython/RustPython
Length of output: 151
🏁 Script executed:
# Look at crates/common/src/lock.rs to find cfg_select definition
cat crates/common/src/lock.rs | head -150Repository: RustPython/RustPython
Length of output: 5255
🏁 Script executed:
# Or search for the actual macro definition
grep -n 'macro_rules\|cfg_select' crates/common/src/lock.rs | head -20Repository: RustPython/RustPython
Length of output: 97
🏁 Script executed:
# Check the full resource.rs const to see the pattern
sed -n '16,30p' crates/stdlib/src/resource.rsRepository: RustPython/RustPython
Length of output: 538
🏁 Script executed:
# Now let's definitively check if cfg_select is even a valid macro by looking for its definition
find crates -name '*.rs' -exec grep -l 'macro_rules! cfg_select' {} \;Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Since it's being used everywhere, let's check if it's from an external crate like cfg-if or similar
grep -r 'pub macro' crates/common/src crates/vm/src crates/stdlib/src | grep cfgRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Last attempt - look in the prelude or macro definitions
rg 'cfg_select' crates/common/src/lib.rs crates/vm/src/lib.rs crates/stdlib/src/lib.rsRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Look at the actual context more - does cfg_select come from rustpython-compiler?
grep -r 'cfg_select' crates/compiler*/src 2>/dev/null || echo "Not in compiler"Repository: RustPython/RustPython
Length of output: 80
🏁 Script executed:
# Check if cfg_select might be from an external proc-macro or prelude import
head -50 crates/stdlib/src/lib.rs | grep -E 'use|mod'Repository: RustPython/RustPython
Length of output: 665
Remove trailing semicolons from cfg_select! branches in the PROBE initializer
Lines 37 and 40 have trailing semicolons that cause each branch to evaluate to () instead of LazyLock<ProbeResult>, breaking the static initializer type.
Suggested fix
static PROBE: LazyLock<ProbeResult> = cfg_select! {
openssl_vendored => {
- LazyLock::new(openssl_probe::probe);
+ LazyLock::new(openssl_probe::probe)
}
_ => {
- LazyLock::new(|| ProbeResult { cert_file: None, cert_dir: vec![] });
+ LazyLock::new(|| ProbeResult { cert_file: None, cert_dir: vec![] })
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/stdlib/src/openssl.rs` around lines 35 - 42, The PROBE static
initializer's cfg_select! branches end with trailing semicolons making each
branch evaluate to () instead of returning a LazyLock<ProbeResult>; remove the
semicolons after LazyLock::new(openssl_probe::probe) and LazyLock::new(||
ProbeResult { cert_file: None, cert_dir: vec![] }) so each branch yields a
LazyLock<ProbeResult> (affecting the PROBE static, the cfg_select! macro,
LazyLock::new invocations, and the ProbeResult construction).
| pub const PYTHREAD_NAME: Option<&str> = cfg_select! { | ||
| windows => Some("nt"), | ||
| unix => Some("pthread"), | ||
| any(target_os = "solaris", target_os = "illumos") => Some("solaris"), | ||
| _ => None, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Rust cfg_select first matching arm target_os solaris illumos cfg unix target_family
💡 Result:
In Rust, both Solaris (e.g., x86_64-pc-solaris) and illumos (e.g., x86_64-unknown-illumos, aarch64-unknown-illumos) targets have target_os = "solaris" for LLVM but are recognized distinctly in Rust's cfg system. Illumos uses base::illumos::opts, analogous to base::solaris::opts which explicitly sets target_family = ["unix"]. Custom illumos targets also specify "target-family": ["unix"]. Both match cfg(target_os = "solaris"), cfg(target_family = "unix"), and cfg(unix). The cfg_select! macro (stable since Rust 1.95) selects the first matching arm based on cfg predicates evaluated left-to-right. For a cfg_select prioritizing illumos over generic solaris/unix: cfg_select! { target_os = "solaris", target_os = "illumos" => { /* illumos-specific / }, target_os = "solaris" => { / Solaris-specific (excludes illumos) / }, target_family = "unix" => { / other unix / }, _ => { / fallback */ } } However, since illumos sets target_os = "illumos" in Rust (distinct from Solaris's "solaris"), target_os = "illumos" matches only illumos, while target_os = "solaris" matches only Solaris. If using target_os = "solaris" first, it matches neither, falling through. To match illumos first: target_os = "illumos" arm before target_os = "solaris". Both are under target_family = "unix".
Citations:
- 1: https://doc.rust-lang.org/rustc/platform-support/illumos.html
- 2: https://doc.rust-lang.org/beta/src/std/sys/fs/unix.rs.html
- 3: https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_target/spec/targets/x86_64_unknown_illumos.rs.html
- 4: https://stackoverflow.com/questions/76480789/rust-is-there-a-list-of-what-target-is-a-member-of-what-target-family
- 5: https://doc.rust-lang.org/std/macro.cfg_select.html
- 6: https://doc.rust-lang.org/beta/rustc/platform-support/illumos.html
- 7: https://doc.rust-lang.org/beta/nightly-rustc/src/rustc_target/spec/base/solaris.rs.html
- 8: https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_target/spec/targets/aarch64_unknown_illumos.rs.html
- 9: https://blog.openflowlabs.com/~/personal/Setting%20up%20a%20new%20Rust%20Target%20with%20a%20cross%20compiler/
🏁 Script executed:
cat -n crates/vm/src/stdlib/_thread.rs | sed -n '30,45p'Repository: RustPython/RustPython
Length of output: 751
Move the Solaris/illumos arm before the unix arm.
cfg_select! evaluates arms left-to-right and stops at the first match. Since both Solaris and illumos targets have target_family = "unix", they currently match the unix => Some("pthread") arm before reaching the more specific any(target_os = "solaris", target_os = "illumos") arm. Reorder to check the narrower condition first.
🔧 Proposed fix
pub const PYTHREAD_NAME: Option<&str> = cfg_select! {
windows => Some("nt"),
+ any(target_os = "solaris", target_os = "illumos") => Some("solaris"),
unix => Some("pthread"),
- any(target_os = "solaris", target_os = "illumos") => Some("solaris"),
_ => None,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub const PYTHREAD_NAME: Option<&str> = cfg_select! { | |
| windows => Some("nt"), | |
| unix => Some("pthread"), | |
| any(target_os = "solaris", target_os = "illumos") => Some("solaris"), | |
| _ => None, | |
| }; | |
| pub const PYTHREAD_NAME: Option<&str> = cfg_select! { | |
| windows => Some("nt"), | |
| any(target_os = "solaris", target_os = "illumos") => Some("solaris"), | |
| unix => Some("pthread"), | |
| _ => None, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/vm/src/stdlib/_thread.rs` around lines 34 - 39, The cfg_select! arms
for PYTHREAD_NAME currently list the broader unix => Some("pthread") arm before
the more specific any(target_os = "solaris", target_os = "illumos") arm, so
Solaris/illumos targets never reach their intended match; fix by reordering the
cfg_select! arms in the PYTHREAD_NAME constant so the any(target_os = "solaris",
target_os = "illumos") arm appears before the unix arm, preserving the other
arms (windows and default) as-is.
| pub const PLATFORM: &str = cfg_select! { | ||
| target_os = "linux" => "linux", | ||
| target_os = "android" => "android", | ||
| target_os = "macos" => "macos", | ||
| target_os = "ios" => "ios", | ||
| windows => "windows", | ||
| target_os = "wasi" => "wasi", | ||
| _ => "unknown" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find sys.platform checks that depend on CPython-compatible values.
rg -nP '(_sys|sys)\.platform|["'\''](darwin|win32|macos|windows)["'\'']' extra_testsRepository: RustPython/RustPython
Length of output: 2811
🏁 Script executed:
cat -n crates/vm/src/stdlib/sys.rs | sed -n '245,260p'Repository: RustPython/RustPython
Length of output: 654
🌐 Web query:
sys.platform CPython Python 3 documentation
💡 Result:
sys.platform is a string in the Python sys module that identifies the underlying platform (operating system) on which the Python interpreter is running, specifically for CPython. Official documentation is in the sys module: https://docs.python.org/3/library/sys.html#sys.platform It states: "Changed in version 3.14: On FreeBSD, sys.platform doesn’t contain the major version anymore. It is always 'freebsd', instead of 'freebsd13' or 'freebsd14'. See also os.name has a coarser granularity. os.uname gives system-dependent version information. The platform module provides detailed checks for the system’s identity." The official Python docs do not provide an exhaustive list of all possible values for sys.platform, as it is determined at compile time based on the platform Python was built for and can vary (e.g., 'linux' for Linux in Python 3.3+, 'win32' for Windows, 'darwin' for macOS, 'freebsd' on FreeBSD 14+, etc.). For common values in CPython: - Linux: 'linux' (since Python 3.3; previously 'linux2') - Windows: 'win32' (even on 64-bit) - macOS: 'darwin' - FreeBSD: 'freebsd' (recent versions) The platform module (https://docs.python.org/3/library/platform.html) provides more detailed and portable platform identification functions like platform.system, platform.platform, etc., and is recommended for detailed checks. In CPython source code (Python/OsPath/posixmodule.c or getplatform in platform.py), it is set during build/configuration (e.g., via configure script on Unix). Use sys.platform.startswith('linux') or similar for portable checks.
Citations:
- 1: https://docs.python.org/3/library/sys.html
- 2: https://docs.python.org/3/library/platform.html
- 3: https://docs.python.org/ko/3.5/library/sys.html
- 4: https://stackoverflow.com/questions/446209/possible-values-from-sys-platform
- 5: https://github.com/python/cpython/blob/main/Lib/platform.py
Change sys.platform values to match CPython standards.
Lines 251 and 253 return "macos" and "windows", but CPython uses "darwin" for macOS and "win32" for Windows. This breaks multiple test cases that check for CPython-compatible values:
stdlib_sys.pychecks for"darwin"and"win32"stdlib_select.pychecks for"win32"stdlib_ctypes.pychecks for"darwin"stdlib_datetime.pychecks for"win32"
Fix
pub const PLATFORM: &str = cfg_select! {
target_os = "linux" => "linux",
target_os = "android" => "android",
- target_os = "macos" => "macos",
+ target_os = "macos" => "darwin",
target_os = "ios" => "ios",
- windows => "windows",
+ windows => "win32",
target_os = "wasi" => "wasi",
_ => "unknown"
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub const PLATFORM: &str = cfg_select! { | |
| target_os = "linux" => "linux", | |
| target_os = "android" => "android", | |
| target_os = "macos" => "macos", | |
| target_os = "ios" => "ios", | |
| windows => "windows", | |
| target_os = "wasi" => "wasi", | |
| _ => "unknown" | |
| pub const PLATFORM: &str = cfg_select! { | |
| target_os = "linux" => "linux", | |
| target_os = "android" => "android", | |
| target_os = "macos" => "darwin", | |
| target_os = "ios" => "ios", | |
| windows => "win32", | |
| target_os = "wasi" => "wasi", | |
| _ => "unknown" | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/vm/src/stdlib/sys.rs` around lines 248 - 255, The PLATFORM constant in
sys.rs currently maps target_os "macos" and Windows to non-CPython values;
update the cfg_select! mapping in the pub const PLATFORM definition so that
target_os = "macos" => "darwin" and windows => "win32" (leave the other arms
unchanged) to match CPython's sys.platform values; modify the PLATFORM const in
crates/vm/src/stdlib/sys.rs accordingly and run the build/tests to verify.
c9d7635 to
d460f5c
Compare
d460f5c to
71e2fac
Compare
Since 1.95
cfg_select!macro is stabilizedhttps://doc.rust-lang.org/stable/std/macro.cfg_select.html
Summary by CodeRabbit
New Requirements
Breaking Changes
sys.PLATFORMstring values: macOS now returns"macos"(previously"darwin"), Windows now returns"windows"(previously"win32").