Implement _stat module
#5847
Implement _stat module
#5847
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VM
participant stat_module
User->>VM: import _stat
VM->>stat_module: initialize via make_module
stat_module-->>VM: module instance with constants/functions
User->>stat_module: call filemode(mode)
stat_module-->>User: return permission string
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsvm/src/stdlib/stat.rs (10)🪛 Biome (1.9.4).cspell.json[error] 137-137: End of file expected Use an array for a sequence of values: (parse) ⏰ Context from checks skipped due to timeout of 90000ms (10)
🔇 Additional comments (5)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
moreal
force-pushed
the
impl-stat
branch
8 times, most recently
from
651f5f8 to
6fc099a
Compare
moreal
force-pushed
the
impl-stat
branch
9 times, most recently
from
85e1e01 to
08110ff
Compare
![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
🧹 Nitpick comments (1)
vm/src/stdlib/stat.rs (1)
310-321: Consider using const values directly instead of functions for constants.The
S_IMODE_methodandS_IFMT_methodfunctions use local const values that differ from the module-level constants. This could lead to confusion.const S_IFMT: Mode = 0o170000; - const S_IMODE: Mode = 0o7777; + const S_IMODE_MASK: Mode = 0o7777; #[pyfunction(name = "S_IMODE")] #[allow(non_snake_case)] fn S_IMODE_method(mode: Mode) -> Mode { - mode & S_IMODE + mode & S_IMODE_MASK }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.cspell.json(1 hunks)Lib/test/test_stat.py(0 hunks)vm/src/stdlib/mod.rs(2 hunks)vm/src/stdlib/stat.rs(1 hunks)
💤 Files with no reviewable changes (1)
- Lib/test/test_stat.py
🧰 Additional context used
🧠 Learnings (1)
vm/src/stdlib/stat.rs (3)
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.109Z
Learning: To add a Python module to the RustPython interpreter, use 'vm.add_native_module' with the module name and module factory.
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.109Z
Learning: When implementing a Python module in Rust for RustPython, use the #[pymodule] attribute and expose functions and classes using #[pyfunction], #[pyclass], and #[pymethod] as appropriate.
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.109Z
Learning: Use the macro system (such as 'pyclass', 'pymodule', 'pyfunction') when implementing Python functionality in Rust for RustPython.
🧬 Code Graph Analysis (1)
vm/src/stdlib/mod.rs (2)
vm/src/stdlib/os.rs (2)
stat(508-540)stat(886-896)vm/src/stdlib/stat.rs (1)
make_module(569-571)
🪛 Biome (1.9.4)
.cspell.json
[error] 137-137: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Ensure compilation on various targets
🔇 Additional comments (6)
vm/src/stdlib/mod.rs (2)
17-17: LGTM! Module declaration follows the established pattern.The stat module is properly declared following the same pattern as other standard library modules.
93-93: LGTM! Module registration follows the established pattern.The stat module is correctly registered in the module initialization map with the appropriate "_stat" name, matching Python's convention.
vm/src/stdlib/stat.rs (4)
1-14: LGTM! Module setup and type definitions are well-structured.The platform-specific type definitions for
Modecorrectly handle Unix (using libc::mode_t), Windows (u16), and fallback cases (u32). The conditional compilation attributes are appropriate.
135-154: Android-specific handling looks correct.The conditional compilation for
S_IWRITEandS_IEXECon Android is appropriate since these constants may not be available in Android's libc.
569-571: LGTM! Module creation function is implemented correctly.The
make_modulefunction follows the established pattern for creating Python modules in RustPython.
93-98: Verify that S_ENFMT should use the same value as S_ISGID.The
S_ENFMTconstant is defined aslibc::S_ISGIDon Unix and0o2000on non-Unix, which matchesS_ISGID. Please confirm this is intentionally the same value.#!/bin/bash # Description: Verify S_ENFMT definition in system headers and CPython # Expected: S_ENFMT should be the same as S_ISGID on most systems # Check system headers for S_ENFMT definition rg -A 2 -B 2 "S_ENFMT" /usr/include/ 2>/dev/null || echo "S_ENFMT not found in system headers" # Search for S_ENFMT in the CPython source to understand the expected behavior echo "Checking CPython's stat module implementation..."
Summary by CodeRabbit
New Features
Tests
Chores