Repository navigation
Validate mmap assignments against current storage #9071
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
71956aa
399e5b0
f165f99
6acb8d6
d93e32d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 3.14.7 | ||
| 3.14.8 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Changing the repository target here leaves Useful? React with 👍 / 👎. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1455,17 +1455,15 @@ mod mmap { | |
| value: &PyObject, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult<()> { | ||
| let i: usize = i | ||
| .wrapped_at(self.__len__()) | ||
| .ok_or_else(|| vm.new_index_error("mmap index out of range"))?; | ||
|
|
||
| let b = value_from_object(vm, value)?; | ||
|
|
||
| self.try_writable(vm, |mmap| { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The commit's own scope limits state that the full workspace suites were not run, while AGENTS.md reference: AGENTS.md:L31-L35 Useful? React with 👍 / 👎. |
||
| let i = i | ||
| .wrapped_at(mmap.len()) | ||
| .ok_or_else(|| vm.new_index_error("mmap index out of range"))?; | ||
| mmap[i] = b; | ||
| })?; | ||
|
|
||
| Ok(()) | ||
| Ok(()) | ||
| })? | ||
| } | ||
|
|
||
| fn setitem_by_slice( | ||
|
|
@@ -1474,43 +1472,37 @@ mod mmap { | |
| value: &PyObject, | ||
| vm: &VirtualMachine, | ||
| ) -> PyResult<()> { | ||
| let (range, step, slice_len) = slice.adjust_indices(self.__len__()); | ||
|
|
||
| let bytes = bytes_from_object(vm, value)?; | ||
|
|
||
| if bytes.len() != slice_len { | ||
| return Err(vm.new_index_error("mmap slice assignment is wrong size")); | ||
| } | ||
| self.try_writable(vm, |mmap| { | ||
| let (range, step, slice_len) = slice.adjust_indices(mmap.len()); | ||
|
|
||
| if slice_len == 0 { | ||
| // do nothing | ||
| Ok(()) | ||
| } else if step == 1 { | ||
| self.try_writable(vm, |mmap| { | ||
| if bytes.len() != slice_len { | ||
| return Err(vm.new_index_error("mmap slice assignment is wrong size")); | ||
| } | ||
|
|
||
| if slice_len == 0 { | ||
| // do nothing | ||
| } else if step == 1 { | ||
| (&mut mmap[range]) | ||
| .write(&bytes) | ||
| .map_err(|err| err.to_pyexception(vm))?; | ||
| Ok(()) | ||
| })? | ||
| } else { | ||
| let mut bi = 0; // bytes index | ||
| if step.is_negative() { | ||
| for i in range.rev().step_by(step.unsigned_abs()) { | ||
| self.try_writable(vm, |mmap| { | ||
| mmap[i] = bytes[bi]; | ||
| })?; | ||
| bi += 1; | ||
| } | ||
| } else { | ||
| for i in range.step_by(step.unsigned_abs()) { | ||
| self.try_writable(vm, |mmap| { | ||
| let mut bi = 0; // bytes index | ||
| if step.is_negative() { | ||
| for i in range.rev().step_by(step.unsigned_abs()) { | ||
| mmap[i] = bytes[bi]; | ||
| bi += 1; | ||
| } | ||
| } else { | ||
| for i in range.step_by(step.unsigned_abs()) { | ||
| mmap[i] = bytes[bi]; | ||
| })?; | ||
| bi += 1; | ||
| bi += 1; | ||
| } | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
| })? | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,3 +77,41 @@ | |
| with assert_raises(ValueError): | ||
| mapped.resize(4) | ||
| assert mapped.closed | ||
|
|
||
|
|
||
| # Assignment validates current storage after argument conversion, even if empty. | ||
| with mmap.mmap(-1, 4) as mapped: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
AGENTS.md reference: AGENTS.md:L277-L279 Useful? React with 👍 / 👎. |
||
| with assert_raises(ValueError): | ||
| mapped[4] = 256 | ||
| mapped[0:0] = b"" | ||
| with assert_raises(ValueError): | ||
| mapped[0:0] = b"" | ||
|
|
||
| with mmap.mmap(-1, 4, access=mmap.ACCESS_READ) as mapped: | ||
| with assert_raises(TypeError): | ||
| mapped[0:0] = b"" | ||
|
|
||
|
|
||
| class CloseOnIndex: | ||
| def __init__(self, mapped): | ||
| self.mapped = mapped | ||
|
|
||
| def __index__(self): | ||
| self.mapped.close() | ||
| return 0 | ||
|
|
||
|
|
||
| for close_from in ("key", "value", "slice"): | ||
| mapped = mmap.mmap(-1, 4) | ||
| closer = CloseOnIndex(mapped) | ||
| try: | ||
| with assert_raises(ValueError): | ||
| if close_from == "key": | ||
| mapped[closer] = 0 | ||
| elif close_from == "value": | ||
| mapped[0] = closer | ||
| else: | ||
| mapped[closer:0] = b"" | ||
| assert mapped.closed | ||
| finally: | ||
| mapped.close() | ||
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.
The reviewed commit is authored by
Codex, but its commit message has noAssisted-by: AGENT_NAME:MODEL_VERSIONtrailer.AGENTS.mdmakes that disclosure mandatory for every AI-assisted contribution, so add the required trailer before merge.AGENTS.md reference: AGENTS.md:L20-L23
Useful? React with 👍 / 👎.