Report bytes out-of-range elements as CPython does - #8679
Conversation
`collect_bytes` is shared by the `bytes` and `bytearray` entry points and hardcoded one element error, so `bytes([256])` said "byte must be in range(0, 256)" where CPython says "bytes". Every `bytearray` path does say "byte", so only the one caller was wrong. The element error is now a parameter, alongside the type error this function already varies per caller. Verified against CPython 3.14 for all seven paths. Assisted-by: Claude Code:claude-opus-5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e07c6b5397
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ) | ||
|
|
||
|
|
||
| def out_of_range(fn, message): |
There was a problem hiding this comment.
Remove the prohibited snippet test additions
Remove this new helper and its associated assertions from extra_tests/snippets/builtin_bytes.py: the repository explicitly restricts test-file changes to adding or removing expected-failure markers, so adding new test logic and test data here violates the contribution rules.
AGENTS.md reference: AGENTS.md:L273-L279
Useful? React with 👍 / 👎.
| // PyBytes_FromObject | ||
| pub fn bytes_from_object(vm: &VirtualMachine, obj: &PyObject) -> PyResult<Vec<u8>> { | ||
| collect_bytes(vm, obj, true, |name| { | ||
| collect_bytes(vm, obj, true, BYTES_ELEMENT_ERROR, |name| { |
There was a problem hiding this comment.
Preserve the bytearray C-API error wording
Use a caller-specific element error instead of unconditionally selecting the bytes wording here. When an extension passes [256] to PyByteArray_FromObject, crates/capi/src/bytearrayobject.rs calls bytes_from_object, so this change makes that bytearray API raise ValueError: bytes must be in range(0, 256); CPython's PyByteArray_FromObject raises the singular byte must be in range(0, 256) for this input.
Useful? React with 👍 / 👎.
It called `bytes_from_object`, so the previous commit made it report "bytes must be in range(0, 256)" where CPython's `PyByteArray_FromObject` says "byte". It is the C-level bytearray constructor, so it now uses `bytearray_from_object`, which carries that wording and the constructor's unsized iteration. Assisted-by: Claude Code:claude-opus-5
|
the p2 one was right and it was a real regression, thanks. checking cpython for the other three callers, on p1 i would rather ask than assume. the test modification rules read to me as protecting existing test code, since the acceptable modifications listed are about Assisted-by: Claude Opus 5 (Claude Code). the text above is mine, not Roshan's. |

One of checkbox below must be checked.
Summary
bytes([256])said "byte must be in range(0, 256)". cpython says "bytes" there, and only there:collect_bytesis shared by all the constructor paths and hardcoded one message, so the bytearray ones were already right and onlybyteswas wrong. the element error is a parameter now, next to the type error that function already varies per caller.checked all seven against cpython 3.14. the snippet cases pass under cpython too, so the expectations are its behaviour rather than my reading, and they fail on main.
this is a follow up to #8659, which listed this as one of the divergences left over.
Summary by CodeRabbit
Bug Fixes
bytesandbytearrayobjects with integers outside the valid range.bytesnow reports “bytes must be in range(0, 256)”, whilebytearrayreports “byte must be in range(0, 256)”.Tests
256and-1across relevant operations.