GeezerVison is showing github.com — compiler-core: implement DerefMut and IndexMut for Constants by youknowone · Pull Request #8557 · RustPython/RustPython
Conversation
youknowone
commented
Aug 19, 2026
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Constants<C> is a newtype over Box<[C]> that exposes Deref and
Index<oparg::ConstIdx> but neither mutable counterpart. An owner of a
CodeObject can therefore read a constant but cannot replace one in place —
the field is public, but Constants' own .0 is not, so the only way to
change 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 its
IndexMut), so this makes ConstIdx symmetric with it.
No behaviour change: both impls are new trait implementations over the
existing private field, and nothing in the tree currently mutates a
Constants value.
Motivation: an out-of-tree consumer of rustpython-compiler-core that owns
its CodeObjects rewrites nested code constants (the _imp._fix_co_filename
path) 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_filenames reaches its nested code objects as already
wrapped PyRef<PyCode> and mutates them through interior mutability — so this
is an API-completeness change rather than a fix for in-tree code.
— commented by Claude
Summary by CodeRabbit
- New Features
- Added support for modifying values through mutable dereferencing.
- Added support for updating constant entries by index.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
`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
coderabbitai
Bot
commented
Aug 19, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 29f946a4-cc14-4c6d-9d3e-ad8dc907ea14
📥 Commits
Reviewing files that changed from the base of the PR and between 86d9407 and 4349d4a.
📒 Files selected for processing (1)
crates/compiler-core/src/bytecode.rs
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
📝 Walkthrough
Walkthrough
Constants<C> now supports mutable access to its underlying constant slice and individual constants through oparg::ConstIdx.
Changes
Mutable constant access
| Layer / File(s) |
Summary |
Add mutable constant access
crates/compiler-core/src/bytecode.rs |
Imports DerefMut and implements mutable slice access and mutable oparg::ConstIdx indexing for Constants<C>. |
Estimated code review effort: 1 (Trivial) | ~5 minutes
Merge Risk: ⚪ Minimal · up to 4349d
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: shaharnaveh
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name |
Status |
Explanation |
| Description Check |
✅ Passed |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check |
✅ Passed |
The title clearly and concisely identifies the two implementations added for Constants and matches the main pull request change. |
| Docstring Coverage |
✅ Passed |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check |
✅ Passed |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check |
✅ Passed |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with 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.
❤️ Share
Comment @coderabbitai help to get the list of available commands.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
youknowone
added a commit
that referenced
this pull request
Sep 16, 2026
`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
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.