StatelessIncrementalEncoder, sys attributes - #7195
Merged
youknowone merged 3 commits intoFeb 21, 2026
Merged
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
📝 Walkthrough
WalkthroughThis pull request introduces stateless codec encoder/decoder wrapper classes as fallback mechanisms in the _io module, refactors the sys module by removing module parameters from pyclass definitions and dynamically computing flag properties, and updates VM initialization logic for codec registration and stdio setup with refined feature gating and error handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem🚥 Pre-merge checks | ✅ 3 ✅ Passed checks (3 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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_compile.py (TODO: 24) dependencies: dependent tests: (no tests depend on compile) [x] test: cpython/Lib/test/test_sys.py (TODO: 8) dependencies: dependent tests: (213 tests)
Legend:
|
Sorry, something went wrong.
youknowone
marked this pull request as ready for review
February 21, 2026 14:46
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agentsVerify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/vm/src/stdlib/io.rs`:
- Around line 2610-2653: The encode wrapper in StatelessIncrementalEncoder
currently ignores the second tuple item from the codec result; update the encode
method to validate that res.try_into_value(vm)? yields a 2-tuple where the
second element is an integer (non-negative if appropriate to your codec
contract) and raise a TypeError via vm.new_type_error when it is missing or not
an int (reusing the existing error message "encoder must return a tuple (object,
integer)"). Locate the encode method and the tuple variable, convert/validate
tuple[1] as an integer (e.g., via try_into_value or checking .isinstance), and
only then return tuple[0].clone(), ensuring malformed codec outputs surface as
errors.
- Around line 2656-2694: In StatelessIncrementalDecoder::decode, validate that
the second tuple element is an integer and raise a TypeError if not; after
converting res to PyTupleRef and checking tuple.len(), try to convert tuple[1]
to an integer (using the VM's integer/index conversion helpers) and return
Err(vm.new_type_error("decoder must return a tuple (object, integer)")) when
conversion fails, otherwise proceed to return tuple[0].clone() as before. Ensure
this check is applied in the decode method to mirror the encoder wrapper's
guard.
Sorry, something went wrong.
Comment thread
crates/vm/src/stdlib/io.rs
Comment on lines
+2610
to
+2653
| #[pyclass(module = "_io", name, no_attr)] | ||
| #[derive(Debug, PyPayload)] | ||
| struct StatelessIncrementalEncoder { | ||
| encode: PyObjectRef, | ||
| errors: Option<PyStrRef>, | ||
| name: Option<PyStrRef>, | ||
| } | ||
|
|
||
| #[pyclass] | ||
| impl StatelessIncrementalEncoder { | ||
| #[pymethod] | ||
| fn encode( | ||
| &self, | ||
| input: PyObjectRef, | ||
| _final: OptionalArg<bool>, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult { | ||
| let mut args: Vec<PyObjectRef> = vec![input]; | ||
| if let Some(errors) = &self.errors { | ||
| args.push(errors.to_owned().into()); | ||
| } | ||
| let res = self.encode.call(args, vm)?; | ||
| let tuple: PyTupleRef = res.try_into_value(vm)?; | ||
| if tuple.len() != 2 { | ||
| return Err(vm.new_type_error("encoder must return a tuple (object, integer)")); | ||
| } | ||
| Ok(tuple[0].clone()) | ||
| } | ||
|
|
||
| #[pymethod] | ||
| fn reset(&self) {} | ||
|
|
||
| #[pymethod] | ||
| fn setstate(&self, _state: PyObjectRef) {} | ||
|
|
||
| #[pymethod] | ||
| fn getstate(&self, vm: &VirtualMachine) -> PyObjectRef { | ||
| vm.ctx.new_int(0).into() | ||
| } | ||
|
|
||
| #[pygetset] | ||
| fn name(&self) -> Option<PyStrRef> { | ||
| self.name.clone() | ||
| } |
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
⚠️ Potential issue | 🟡 Minor
Validate the consumed-length element from codec output.
The wrapper ignores the second tuple item entirely; if it’s not an integer, malformed codecs won’t surface errors. Consider validating it to match the codec contract.
🛠️ Suggested guard for the consumed-length slot let tuple: PyTupleRef = res.try_into_value(vm)?;
if tuple.len() != 2 {
return Err(vm.new_type_error("encoder must return a tuple (object, integer)"));
}
+ let _consumed: isize = isize::try_from_object(vm, tuple[1].clone()).map_err(|_| {
+ vm.new_type_error("encoder must return a tuple (object, integer)")
+ })?;
Ok(tuple[0].clone())‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[pyclass(module = "_io", name, no_attr)] | |
| #[derive(Debug, PyPayload)] | |
| struct StatelessIncrementalEncoder { | |
| encode: PyObjectRef, | |
| errors: Option<PyStrRef>, | |
| name: Option<PyStrRef>, | |
| } | |
| #[pyclass] | |
| impl StatelessIncrementalEncoder { | |
| #[pymethod] | |
| fn encode( | |
| &self, | |
| input: PyObjectRef, | |
| _final: OptionalArg<bool>, | |
| vm: &VirtualMachine, | |
| ) -> PyResult { | |
| let mut args: Vec<PyObjectRef> = vec![input]; | |
| if let Some(errors) = &self.errors { | |
| args.push(errors.to_owned().into()); | |
| } | |
| let res = self.encode.call(args, vm)?; | |
| let tuple: PyTupleRef = res.try_into_value(vm)?; | |
| if tuple.len() != 2 { | |
| return Err(vm.new_type_error("encoder must return a tuple (object, integer)")); | |
| } | |
| Ok(tuple[0].clone()) | |
| } | |
| #[pymethod] | |
| fn reset(&self) {} | |
| #[pymethod] | |
| fn setstate(&self, _state: PyObjectRef) {} | |
| #[pymethod] | |
| fn getstate(&self, vm: &VirtualMachine) -> PyObjectRef { | |
| vm.ctx.new_int(0).into() | |
| } | |
| #[pygetset] | |
| fn name(&self) -> Option<PyStrRef> { | |
| self.name.clone() | |
| } | |
| #[pyclass(module = "_io", name, no_attr)] | |
| #[derive(Debug, PyPayload)] | |
| struct StatelessIncrementalEncoder { | |
| encode: PyObjectRef, | |
| errors: Option<PyStrRef>, | |
| name: Option<PyStrRef>, | |
| } | |
| #[pyclass] | |
| impl StatelessIncrementalEncoder { | |
| #[pymethod] | |
| fn encode( | |
| &self, | |
| input: PyObjectRef, | |
| _final: OptionalArg<bool>, | |
| vm: &VirtualMachine, | |
| ) -> PyResult { | |
| let mut args: Vec<PyObjectRef> = vec![input]; | |
| if let Some(errors) = &self.errors { | |
| args.push(errors.to_owned().into()); | |
| } | |
| let res = self.encode.call(args, vm)?; | |
| let tuple: PyTupleRef = res.try_into_value(vm)?; | |
| if tuple.len() != 2 { | |
| return Err(vm.new_type_error("encoder must return a tuple (object, integer)")); | |
| } | |
| let _consumed: isize = isize::try_from_object(vm, tuple[1].clone()).map_err(|_| { | |
| vm.new_type_error("encoder must return a tuple (object, integer)") | |
| })?; | |
| Ok(tuple[0].clone()) | |
| } | |
| #[pymethod] | |
| fn reset(&self) {} | |
| #[pymethod] | |
| fn setstate(&self, _state: PyObjectRef) {} | |
| #[pymethod] | |
| fn getstate(&self, vm: &VirtualMachine) -> PyObjectRef { | |
| vm.ctx.new_int(0).into() | |
| } | |
| #[pygetset] | |
| fn name(&self) -> Option<PyStrRef> { | |
| self.name.clone() | |
| } |
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/stdlib/io.rs` around lines 2610 - 2653, The encode wrapper in StatelessIncrementalEncoder currently ignores the second tuple item from the codec result; update the encode method to validate that res.try_into_value(vm)? yields a 2-tuple where the second element is an integer (non-negative if appropriate to your codec contract) and raise a TypeError via vm.new_type_error when it is missing or not an int (reusing the existing error message "encoder must return a tuple (object, integer)"). Locate the encode method and the tuple variable, convert/validate tuple[1] as an integer (e.g., via try_into_value or checking .isinstance), and only then return tuple[0].clone(), ensuring malformed codec outputs surface as errors.
Sorry, something went wrong.
Comment thread
crates/vm/src/stdlib/io.rs
Comment on lines
+2656
to
+2694
| #[pyclass(module = "_io", name, no_attr)] | ||
| #[derive(Debug, PyPayload)] | ||
| struct StatelessIncrementalDecoder { | ||
| decode: PyObjectRef, | ||
| errors: Option<PyStrRef>, | ||
| } | ||
|
|
||
| #[pyclass] | ||
| impl StatelessIncrementalDecoder { | ||
| #[pymethod] | ||
| fn decode( | ||
| &self, | ||
| input: PyObjectRef, | ||
| _final: OptionalArg<bool>, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult { | ||
| let mut args: Vec<PyObjectRef> = vec![input]; | ||
| if let Some(errors) = &self.errors { | ||
| args.push(errors.to_owned().into()); | ||
| } | ||
| let res = self.decode.call(args, vm)?; | ||
| let tuple: PyTupleRef = res.try_into_value(vm)?; | ||
| if tuple.len() != 2 { | ||
| return Err(vm.new_type_error("decoder must return a tuple (object, integer)")); | ||
| } | ||
| Ok(tuple[0].clone()) | ||
| } | ||
|
|
||
| #[pymethod] | ||
| fn getstate(&self, vm: &VirtualMachine) -> (PyBytesRef, u64) { | ||
| (vm.ctx.empty_bytes.to_owned(), 0) | ||
| } | ||
|
|
||
| #[pymethod] | ||
| fn setstate(&self, _state: PyTupleRef, _vm: &VirtualMachine) {} | ||
|
|
||
| #[pymethod] | ||
| fn reset(&self) {} | ||
| } |
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
⚠️ Potential issue | 🟡 Minor
Validate the consumed-length element from codec output.
Same concern as the encoder wrapper: the second tuple element should be an integer; otherwise invalid codec implementations pass silently.
🛠️ Suggested guard for the consumed-length slot let tuple: PyTupleRef = res.try_into_value(vm)?;
if tuple.len() != 2 {
return Err(vm.new_type_error("decoder must return a tuple (object, integer)"));
}
+ let _consumed: isize = isize::try_from_object(vm, tuple[1].clone()).map_err(|_| {
+ vm.new_type_error("decoder must return a tuple (object, integer)")
+ })?;
Ok(tuple[0].clone())Verify each finding against the current code and only fix it if needed.
In `@crates/vm/src/stdlib/io.rs` around lines 2656 - 2694, In
StatelessIncrementalDecoder::decode, validate that the second tuple element is
an integer and raise a TypeError if not; after converting res to PyTupleRef and
checking tuple.len(), try to convert tuple[1] to an integer (using the VM's
integer/index conversion helpers) and return Err(vm.new_type_error("decoder must
return a tuple (object, integer)")) when conversion fails, otherwise proceed to
return tuple[0].clone() as before. Ensure this check is applied in the decode
method to mirror the encoder wrapper's guard.
Sorry, something went wrong.
youknowone
added a commit
to youknowone/RustPython
that referenced
this pull request
Mar 22, 2026
StatelessIncrementalEncoder, sys attributes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features