stdlib_module_names #6568
stdlib_module_names #6568
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds Frame.f_builtins getter exposing a frame's builtins dict and adds sys.stdlib_module_names attribute returning a frozen set of standard-library module names; also updates cspell dictionary with four new words. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/vm/src/stdlib/sys.rs (1)
446-452: Consider using.expect()instead of.unwrap()for better diagnostics.The implementation correctly creates a frozen set of standard library module names. The
.unwrap()call is safe in this context since all items are strings (which are hashable), but using.expect()with a descriptive message would provide better diagnostics if this ever fails unexpectedly.🔎 Optional improvement
#[pyattr(once)] fn stdlib_module_names(vm: &VirtualMachine) -> PyObjectRef { let names = STDLIB_MODULE_NAMES .iter() .map(|&n| vm.ctx.new_str(n).into()); - PyFrozenSet::from_iter(vm, names).unwrap().to_pyobject(vm) + PyFrozenSet::from_iter(vm, names) + .expect("stdlib_module_names should always be hashable") + .to_pyobject(vm) }
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Lib/test/test_sys.pyis excluded by!Lib/**
📒 Files selected for processing (2)
crates/vm/src/builtins/frame.rscrates/vm/src/stdlib/sys.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/vm/src/builtins/frame.rscrates/vm/src/stdlib/sys.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.011Z
Learning: RustPython is a Python 3 interpreter written in Rust implementing Python 3.13.0+ compatibility with complete Python environment in Rust (not CPython bindings), clean implementation without compatibility hacks, and cross-platform support including WebAssembly compilation
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
📚 Learning: 2025-06-27T14:47:28.810Z
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.
Applied to files:
crates/vm/src/stdlib/sys.rs
📚 Learning: 2025-06-27T14:47:28.810Z
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
Applied to files:
crates/vm/src/stdlib/sys.rs
🪛 GitHub Check: Check Rust code with clippy
crates/vm/src/stdlib/sys.rs
[warning] 218-218:
Unknown word (pyrepl)
[warning] 215-215:
Unknown word (pydecimal)
[warning] 203-203:
Unknown word (multibytecodec)
[warning] 199-199:
Unknown word (lsprof)
🔇 Additional comments (3)
crates/vm/src/builtins/frame.rs (1)
44-47: LGTM! Clean implementation of frame.f_builtins.The getter correctly exposes the frame's builtins dictionary and follows the same pattern as
f_globals. This aligns with Python's frame object interface.crates/vm/src/stdlib/sys.rs (2)
12-13: LGTM! Necessary import for stdlib_module_names implementation.Adding
PyFrozenSetto the imports is required for the newstdlib_module_namesattribute.
152-444: LGTM! Comprehensive standard library module list.The constant list matches CPython's standard library modules. Note that the static analysis warnings about "unknown words" (pyrepl, pydecimal, multibytecodec, lsprof) are false positives—these are valid Python standard library module names.
e1362df to
0e42435
Compare
0e42435 to
2538f7b
Compare
related to #6548
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.