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 — StatelessIncrementalEncoder, sys attributes by youknowone · Pull Request #7195 · RustPython/RustPython Conversation
|
📝 WalkthroughWalkthroughThis pull request introduces stateless codec encoder/decoder wrapper classes as fallback mechanisms in the 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
🧪 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 |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
📦 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:
|
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify 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.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| #[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() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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())📝 Committable suggestion
‼️ 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.
| #[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() | |
| } |
🤖 Prompt for AI Agents
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.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| #[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) {} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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())🤖 Prompt for AI Agents
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.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Uh oh!
There was an error while loading. Please reload this page.
StatelessIncrementalEncoder, sys attributes
StatelessIncrementalEncoder, sys attributes
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
1 participant
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.