Skip to content

Convert an __index__ key before refusing item assignment or deletion #9516

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.

Sign up for GitHub

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

Merged
youknowone merged 2 commits into from
Nov 1, 2026
Merged

Convert an __index__ key before refusing item assignment or deletion #9516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c009022
Convert an index key before refusing item assignment
jseop-lim Oct 31, 2026
da3a3c4
Read the type name after converting the deletion key
jseop-lim Nov 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion crates/vm/src/protocol/object.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@ use crate::{
types::{Constructor, PyComparisonOp},
};

/// Converts an index key the way a sequence does before it refuses the
/// operation, so an error from `__index__` or the size check wins.
fn sequence_index_key(key: &PyObject, vm: &VirtualMachine) -> PyResult<Option<isize>> {
let Some(index) = key.try_index_opt(vm) else {
return Ok(None);
};
index?.try_to_primitive(vm).map(Some).map_err(|_| {
vm.new_index_error(format!(
"cannot fit '{}' into an index-sized integer",
key.class().name()
))
})
}

// RustPython doesn't need these items
// PyObject *Py_NotImplemented
// Py_RETURN_NOTIMPLEMENTED
Expand Down Expand Up @@ -812,6 +826,10 @@ impl PyObject {
return f(seq, i, Some(value), vm);
}

// Every heap type carries a sequence table.
if seq.slots().has_any() || self.class().heaptype_ext().is_some() {
sequence_index_key(&needle.to_pyobject(vm), vm)?;
}
Err(vm.new_type_error(format!(
"'{}' object does not support item assignment",
self.class().slot_name()
Expand All @@ -836,8 +854,11 @@ impl PyObject {

// A type carrying a sequence table turns the deletion down in
// PySequence_DelItem's words instead; every heap type carries one.
let index_key = (seq.slots().has_any() || self.class().heaptype_ext().is_some())
&& sequence_index_key(&needle.to_pyobject(vm), vm)?.is_some();
// Read the name only now: the key's __index__ may have renamed the type.
let name = self.class().slot_name();
let msg = if seq.slots().has_any() || self.class().heaptype_ext().is_some() {
let msg = if index_key {
format!("'{name}' object doesn't support item deletion")
} else {
format!("'{name}' object does not support item deletion")
Expand Down
55 changes: 55 additions & 0 deletions extra_tests/snippets/protocol_index_bad.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Test that indexing ops don't hang when an object with a mutating
__index__ is used."""

import operator
from array import array

from testutils import assert_raises
Expand Down Expand Up @@ -33,3 +34,57 @@ def run_delslice():
for e in instances:
run_setslice()
run_delslice()


# An exception from __index__ propagates before the operation is refused.
class RaisingIndex:
def __index__(self):
raise ZeroDivisionError


class Plain:
pass


t = ()
with assert_raises(ZeroDivisionError):
t[RaisingIndex()] = 0
Comment on lines +50 to +51

@chatgpt-codex-connector chatgpt-codex-connector Bot Nov 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Revert the disallowed edits to the existing test snippet

These additions introduce new test logic and assertions into an existing test file, but root AGENTS.md explicitly limits the “only acceptable modifications to test files” to adding/removing @unittest.expectedFailure decorators and removing associated TODO comments. The appended regression cases and rewritten docstring fall outside that allowed set; restore the existing snippet content or obtain an explicit exception to the repository's test-edit policy.

AGENTS.md reference: AGENTS.md:L280-L282

Useful? React with 👍 / 👎.

@jseop-lim jseop-lim Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That section covers the CPython tests in Lib/test. For extra_tests/snippets, AGENTS.md asks for new tests in the most closely related existing file (L119-L120), which is what this does. I did restore the original docstring in da3a3c4, since the new cases didn't need it rewritten.

with assert_raises(ZeroDivisionError):
del t[RaisingIndex()]
with assert_raises(ZeroDivisionError):
operator.setitem(b"", RaisingIndex(), 0)
with assert_raises(ZeroDivisionError):
operator.delitem("", RaisingIndex())
with assert_raises(ZeroDivisionError):
Plain()[RaisingIndex()] = 0
with assert_raises(ZeroDivisionError):
del Plain()[RaisingIndex()]
with assert_raises(IndexError):
t[2**70] = 0
with assert_raises(IndexError):
del t[2**70]

with assert_raises(TypeError) as cm:
del t[0]
assert str(cm.exception) == "'tuple' object doesn't support item deletion"
with assert_raises(TypeError) as cm:
del t["a"]
assert str(cm.exception) == "'tuple' object does not support item deletion"
with assert_raises(TypeError) as cm:
del object()[RaisingIndex()]
assert str(cm.exception) == "'object' object does not support item deletion"


class Renamed:
pass


class RenamingIndex:
def __index__(self):
Renamed.__name__ = "Other"
return 0


with assert_raises(TypeError) as cm:
del Renamed()[RenamingIndex()]
assert str(cm.exception) == "'Other' object doesn't support item deletion"
Loading