Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Dec 31, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Improved memory management by eliminating reference cycles in exception handling during function returns.
    • Corrected file attribute assignment to properly distinguish between real file paths and pseudo-paths in code execution.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 31, 2025

📝 Walkthrough

Walkthrough

Two targeted optimizations to the VM: exception traceback cleanup in the frame unwinding path to prevent reference cycles, and conditional file attribute setting that only applies to real file paths, skipping pseudo-paths like <string>.

Changes

Cohort / File(s) Summary
Exception Traceback Cleanup
crates/vm/src/frame.rs
Modified unwinding path for Returning to clear tracebacks of exceptions stored in fastlocals when their traceback's top frame references the current frame. Iterates fastlocals, preserves return value and active exception, removes potentially leaked references.
file Attribute Conditional Setting
crates/vm/src/vm/compile.rs
Added conditional logic to run_code_string to set __file__ attribute only for real file paths; pseudo-paths like <string> are now skipped, preventing module globals modification for embedded/synthetic sources.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit's ode to memory freed:
Tracebacks cleared in cycles freed,
False paths skip the file decree,
Cleaner stacks and reference trees! 🌳
No leaks persist where frames once be.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes both main changes: clearing tracebacks in exception handling and conditional file setting based on path type.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 020ff30 and fc800b7.

⛔ Files ignored due to path filters (2)
  • Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**
  • Lib/test/test_unittest/test_case.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • crates/vm/src/frame.rs
  • crates/vm/src/vm/compile.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to 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/frame.rs
  • crates/vm/src/vm/compile.rs
🧬 Code graph analysis (2)
crates/vm/src/frame.rs (1)
crates/vm/src/exceptions.rs (6)
  • obj (221-221)
  • obj (225-225)
  • obj (395-395)
  • obj (403-403)
  • value (426-426)
  • value (590-590)
crates/vm/src/vm/compile.rs (1)
crates/compiler/src/lib.rs (1)
  • source_path (73-78)
🔇 Additional comments (3)
crates/vm/src/frame.rs (2)

1767-1772: LGTM: Precise frame equality check.

The pointer equality check correctly identifies exceptions whose traceback references this specific frame, ensuring only local reference cycles are broken while preserving tracebacks for exceptions from outer frames.


1742-1777: Add context for the performance impact and clarify the frame pointer comparison safety.

The exception traceback cleanup logic is sound and properly safeguarded. Three important notes:

  1. Frame pointer comparison: The core::ptr::eq::<Py<Frame>>(&*tb.frame, self.object) safely compares frame pointers—self.object is a valid current frame reference, and pointer equality comparison is safe in Rust.

  2. Reference cycle prevention: The logic correctly breaks cycles by only clearing tracebacks where the exception's traceback top frame is THIS frame, preserving both the return value and active exception.

  3. Performance consideration: This adds O(n) iteration on every function return where n = number of local variables. While most functions have few locals (minimal impact), functions with many locals or in hot paths may see measurable overhead. Consider documenting the expected performance characteristics or add a comment noting this is acceptable for correctness given RustPython's lack of tracing GC.

crates/vm/src/vm/compile.rs (1)

118-125: Correctly filters pseudo-paths when setting __file__.

The conditional check that skips __file__ for pseudo-paths like <string>, <stdin>, and <embedded> is correct and aligns with CPython's behavior. The simple string-based pattern !(source_path.starts_with('<') && source_path.ends_with('>')) effectively distinguishes synthetic sources from real file paths.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone youknowone force-pushed the __file__ branch 2 times, most recently from 5d20fd5 to 789fc1a Compare January 1, 2026 02:19
@youknowone youknowone marked this pull request as ready for review January 1, 2026 07:27
@youknowone youknowone merged commit a445a22 into RustPython:main Jan 1, 2026
13 checks passed
@youknowone youknowone deleted the __file__ branch January 1, 2026 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant