GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||
GeezerVison is showing github.com — Add more abstract functions to c-api by bschoenmaeckers · Pull Request #8202 · RustPython/RustPython Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughSix new C-ABI exports are added in ChangesC-ABI abstract protocol exports
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/capi/src/abstract_.rs`:
- Around line 191-199: PyObject_Format currently dereferences format_spec
unconditionally, which crashes when callers pass NULL. Update the
PyObject_Format function to detect a null format_spec and treat it as an empty
PyStr before calling try_downcast_ref::<PyStr>(vm), while leaving the obj
handling and vm.format call unchanged.
- Around line 139-147: PyVectorcall_Call is currently routed through
PyObject_Call, which bypasses the vectorcall fast path and can recurse when used
as the callable’s call bridge. Update PyVectorcall_Call in abstract_.rs to
dispatch via the vectorcall entrypoint instead of the normal call slot, using
the existing vectorcall-related helpers/types in this module so the tp_call
bridge preserves vectorcall behavior.
Fix all unresolved CodeRabbit comments on this PR:
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: fc3c192e-d650-46b7-b380-4fa319477111
crates/capi/src/abstract_.rs
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/capi/src/abstract_.rs`:
- Around line 225-228: In PyObject_Format within abstract_.rs, the current
try_downcast_ref::<PyStr> path returns a generic downcast error for non-string
format_spec values, but this C API should map that case to SystemError to match
CPython. Update the handling around format_spec.as_ref() so non-str specs are
explicitly translated into a SystemError before continuing, while keeping the
existing empty_str fallback for None.
Fix all unresolved CodeRabbit comments on this PR:
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 273e1473-5959-4bf5-9667-93d87a2fb932
crates/capi/src/abstract_.rs
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
4c4cc0d to
71d44f0
Compare
71d44f0 to
3301c67
Compare
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.