compiler-core: implement DerefMut and IndexMut for Constants - #8557
Conversation
`Constants<C>` exposed `Deref` and `Index<ConstIdx>` but neither mutable counterpart, so an owner of a `CodeObject` could read a constant but not replace one in place and had to rebuild the whole boxed slice. `[T]` already carries both halves of the `VarNum` pair in this file. Assisted-by: Claude
|
No actionable comments were generated in the recent review. π βΉοΈ Recent review infoβοΈ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: π Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. π WalkthroughWalkthrough
ChangesMutable constant access
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: βͺ Minimal Β· up to This localized API addition enables mutable constant access without evidence of a correctness or production-impact risk at the current head; no actionable merge-blocking risk remains after normal checks. 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 |
Constants<C>is a newtype overBox<[C]>that exposesDerefandIndex<oparg::ConstIdx>but neither mutable counterpart. An owner of aCodeObjectcan therefore read a constant but cannot replace one in place βthe field is public, but
Constants' own.0is not, so the only way tochange a single entry is to map the whole slice and rebuild it with
FromIterator.This adds the two missing halves. The file already carries both sides of the
same pair for
VarNum(impl<T> Index<oparg::VarNum> for [T]and itsIndexMut), so this makesConstIdxsymmetric with it.No behaviour change: both impls are new trait implementations over the
existing private field, and nothing in the tree currently mutates a
Constantsvalue.Motivation: an out-of-tree consumer of
rustpython-compiler-corethat ownsits
CodeObjects rewrites nested code constants (the_imp._fix_co_filenamepath) and today has to rebuild the constants array to do it, which also moves
every entry and invalidates pointers into it. RustPython itself does not need
this β
update_code_filenamesreaches its nested code objects as alreadywrapped
PyRef<PyCode>and mutates them through interior mutability β so thisis an API-completeness change rather than a fix for in-tree code.
β commented by Claude
Summary by CodeRabbit