Conversation
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/stdlib/src/gc.rs`:
- Around line 5-13: The module attribute definition for gc.garbage uses
#[pyattr] which recreates an empty list on each access; change the attribute to
#[pyattr(once)] on the garbage function so the created list is stored/persistent
across accesses (matching CPython semantics) — update the attribute macro on the
garbage function (fn garbage(vm: &VirtualMachine) -> PyObjectRef) to
#[pyattr(once)] so gc.garbage returns the same mutable list instance each time.
crates/stdlib/src/gc.rs
Outdated
| use crate::vm::{PyObjectRef, PyResult, VirtualMachine, function::FuncArgs}; | ||
|
|
||
| /// A list of objects which the collector found to be unreachable | ||
| /// but could not be freed. RustPython doesn't have cyclic GC, | ||
| /// so this is always an empty list. | ||
| #[pyattr] | ||
| fn garbage(vm: &VirtualMachine) -> PyObjectRef { | ||
| vm.ctx.new_list(vec![]).into() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the #[pyattr] macro definition
fd -t f "*.rs" | xargs rg -l "#\[pyattr\]" | head -5Repository: RustPython/RustPython
Length of output: 515
🏁 Script executed:
# Search for how #[pyattr] macro is defined
rg -t rs "macro.*pyattr|impl.*pyattr" --max-count=20Repository: RustPython/RustPython
Length of output: 91
🏁 Script executed:
# Look at the gc module context around this function
cat -n crates/stdlib/src/gc.rsRepository: RustPython/RustPython
Length of output: 2976
🏁 Script executed:
# Check if there's any module initialization or attribute setting
rg -t rs "garbage" crates/stdlib/src/gc.rs -B 5 -A 5Repository: RustPython/RustPython
Length of output: 91
🏁 Script executed:
# Look for gc.garbage tests in CPython test suite or RustPython tests
fd -t f "test_gc.py" -o "gc_test*" -o "*gc*.py" | xargs rg -l "garbage" 2>/dev/null | head -10Repository: RustPython/RustPython
Length of output: 467
🏁 Script executed:
# Search for #[pyattr] macro definition and implementation
fd "lib.rs" crates | xargs grep -l "pyattr" | head -5Repository: RustPython/RustPython
Length of output: 144
🏁 Script executed:
# Look for pyattr macro in derive or macro crates
fd "*.rs" crates | xargs grep -l "macro.*pyattr\|#\[proc_macro"Repository: RustPython/RustPython
Length of output: 350
🏁 Script executed:
# Search for how pyattr is used elsewhere in stdlib
fd "*.rs" crates/stdlib/src | xargs grep -A 3 "#\[pyattr\]" | head -40Repository: RustPython/RustPython
Length of output: 348
🏁 Script executed:
# Check for any cached or persistent attribute patterns in the module system
rg "pyattr|PyAttr" crates/pymodule --max-count=30 -B 2 -A 2Repository: RustPython/RustPython
Length of output: 120
🏁 Script executed:
# Search test directory for gc.garbage related tests
fd "test.*gc\|gc.*test" Lib/test 2>/dev/null || fd "test" Lib | grep gcRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Look directly for gc.garbage in test files
rg "garbage" Lib/test -l 2>/dev/null | head -5Repository: RustPython/RustPython
Length of output: 213
🏁 Script executed:
# Read the pyattr macro definition
head -200 crates/derive/src/lib.rs | tail -100Repository: RustPython/RustPython
Length of output: 3916
🏁 Script executed:
# Get more of the derive lib file to find pyattr
rg "pyattr" crates/derive/src/lib.rs -B 5 -A 15 | head -80Repository: RustPython/RustPython
Length of output: 2635
🏁 Script executed:
# Check how pyattr is used in hashlib
rg "#\[pyattr\]" crates/stdlib/src/hashlib.rs -B 2 -A 5Repository: RustPython/RustPython
Length of output: 725
🏁 Script executed:
# Check how pyattr is used in zlib
rg "#\[pyattr\]" crates/stdlib/src/zlib.rs -B 2 -A 5Repository: RustPython/RustPython
Length of output: 1542
🏁 Script executed:
# Search for pyattr in entire codebase with more context
rg "pyattr" --max-count=50 -B 3 -A 3 cratesRepository: RustPython/RustPython
Length of output: 50378
🏁 Script executed:
# Check if there are any RustPython tests for gc.garbage
fd "gc" Lib/test --type f | head -10Repository: RustPython/RustPython
Length of output: 255
🏁 Script executed:
# Look for how gc module is initialized or used in tests
rg "gc\.garbage" Lib 2>/dev/null | head -20Repository: RustPython/RustPython
Length of output: 1114
Use #[pyattr(once)] to make gc.garbage persistent across accesses.
The #[pyattr] macro without (once) recomputes the value on each attribute access, creating a new empty list every time. This breaks compatibility with CPython, where gc.garbage is a mutable list that persists between reads. Code that modifies it (e.g., gc.garbage.clear() in tests) will lose those changes. Use #[pyattr(once)] instead to cache the result in a static cell, matching the pattern used elsewhere in RustPython for persistent module attributes.
🤖 Prompt for AI Agents
In `@crates/stdlib/src/gc.rs` around lines 5 - 13, The module attribute definition
for gc.garbage uses #[pyattr] which recreates an empty list on each access;
change the attribute to #[pyattr(once)] on the garbage function so the created
list is stored/persistent across accesses (matching CPython semantics) — update
the attribute macro on the garbage function (fn garbage(vm: &VirtualMachine) ->
PyObjectRef) to #[pyattr(once)] so gc.garbage returns the same mutable list
instance each time.
Lib/test/libregrtest/single.py
Outdated
| if gc.garbage: | ||
| support.environment_altered = True | ||
| print_warning(f"{test_name} created {len(gc.garbage)} " | ||
| f"uncollectable object(s)") | ||
|
|
||
| # # move the uncollectable objects somewhere, # TODO: RUSTPYTHON, build a functional garbage collector into the interpreter | ||
| # # so we don't see them again # TODO: RUSTPYTHON, build a functional garbage collector into the interpreter | ||
| # GC_GARBAGE.extend(gc.garbage) # TODO: RUSTPYTHON, build a functional garbage collector into the interpreter | ||
| # gc.garbage.clear() # TODO: RUSTPYTHON, build a functional garbage collector into the interpreter | ||
| # move the uncollectable objects somewhere, | ||
| # so we don't see them again | ||
| GC_GARBAGE.extend(gc.garbage) | ||
| gc.garbage.clear() |
youknowone
force-pushed
the
regrtest
branch
2 times, most recently
from
eff49b4 to
6c17e07
Compare
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.