Add symtable.py from 3.13.5#6048
Conversation
Sorry, something went wrong.
WalkthroughThe changes update the symbol table module in the standard library. The module's registration key is renamed from "symtable" to "_symtable". The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate 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. 🪧 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
Documentation and Community
|
Sorry, something went wrong.
youknowone
left a comment
youknowone
left a comment
There was a problem hiding this comment.
Did we have test_symtable but no symtable.py? How did it work before?
Sorry, something went wrong.
Correct
We had implemented |
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: 1
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Lib/test/test_symtable.pyis excluded by!Lib/**
📒 Files selected for processing (1)
vm/src/stdlib/symtable.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style (cargo fmtto format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your 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:
vm/src/stdlib/symtable.rs
🧠 Learnings (1)
vm/src/stdlib/symtable.rs (3)
Learnt from: moreal
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.
Learnt from: moreal
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: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to **/*.rs : Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust
🔇 Additional comments (4)
vm/src/stdlib/symtable.rs (4)
5-11: LGTM!The import additions are appropriate for the new functionality -
PyDictRefsupports thesymbolsproperty and the explicitcompilerimport improves code clarity.
13-105: Excellent alignment with CPython's symtable constants.The comprehensive set of constants mirrors CPython's internal symtable definitions, ensuring compatibility. The bit-shifting patterns and flag combinations are mathematically correct and follow established conventions.
144-184: Excellent refactoring to Python-style properties.The conversion from explicit methods to properties using
#[pygetset]aligns perfectly with Python conventions. The use of#[pygetset(name = "type")]properly handles Rust keyword conflicts.
186-194: Clean implementation of symbols property.The new
symbolsproperty correctly creates a dictionary mapping symbol names to their flag bits, providing a proper Python-style interface to the symbol information.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
vm/src/stdlib/symtable.rs (1)
181-184: Security concern with id() method exposing raw memory addresses.The
id()method returns raw memory addresses, which could potentially be used for memory layout attacks or information disclosure in security-sensitive contexts.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Lib/test/test_symtable.pyis excluded by!Lib/**
📒 Files selected for processing (1)
vm/src/stdlib/symtable.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style (cargo fmtto format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your 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:
vm/src/stdlib/symtable.rs
🧠 Learnings (3)
📚 Learning: in rustpython's stat module implementation, platform-specific constants like sf_supported and sf_syn...
Learnt from: moreal
PR: RustPython/RustPython#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:
vm/src/stdlib/symtable.rs
📚 Learning: in rustpython's stat module implementation, platform-specific constants like sf_supported and sf_syn...
Learnt from: moreal
PR: RustPython/RustPython#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:
vm/src/stdlib/symtable.rs
📚 Learning: applies to **/*.rs : use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implement...
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
Applied to files:
vm/src/stdlib/symtable.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run tests under miri
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Run rust tests (ubuntu-latest)
🔇 Additional comments (5)
vm/src/stdlib/symtable.rs (5)
149-152: LGTM! Method to property conversion aligns with feedback.Converting
get_nameto anameproperty using#[pygetset]aligns well with the previous review feedback to use getset instead of deleting methods.
171-179: LGTM! Children property correctly replaces get_children method.The
childrenproperty properly converts sub-tables to Python objects and aligns with the goal of using properties instead of explicit methods.
186-195: LGTM! Identifiers property conversion is appropriate.Converting
get_identifiersto anidentifiersproperty maintains the same functionality while following the property-based interface pattern.
207-210: LGTM! Nested property appropriately replaces is_nested method.The
nestedproperty correctly exposes the symbol table's nested status, following the pattern established by other property conversions.
15-110: Constants section matches CPython internal definitionsThe bit shifts and composite values (e.g.
DEF_BOUND,SCOPE_MASK) exactly mirror those in CPython’spycore_symtable.h. No changes needed here.
Sorry, something went wrong.
youknowone
left a comment
youknowone
left a comment
There was a problem hiding this comment.
Thank you!
Sorry, something went wrong.
youknowone
merged commit 7f1fc36
into
RustPython:main
Aug 1, 2025
Summary by CodeRabbit
New Features
Refactor
PySymbolTableinterface by replacing several explicit methods with Python-style properties.Chores