Report bytes out-of-range elements as CPython does by rawsun007 · Pull Request #8679 · RustPython/RustPython · GitHub
Skip to content

Report bytes out-of-range elements as CPython does - #8679

Open
rawsun007 wants to merge 2 commits into
RustPython:mainfrom
rawsun007:bytes-element-error
Open

Report bytes out-of-range elements as CPython does#8679
rawsun007 wants to merge 2 commits into
RustPython:mainfrom
rawsun007:bytes-element-error

Conversation

@rawsun007

@rawsun007 rawsun007 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
  • Closes #xxxx

One of checkbox below must be checked.

  • I did not use AI to write the code of this patch.
  • This PR follows our AI policy

Summary

bytes([256]) said "byte must be in range(0, 256)". cpython says "bytes" there, and only there:

bytes([256])                 bytes must be in range(0, 256)
bytes(iter([256]))           bytes must be in range(0, 256)
bytearray([256])             byte must be in range(0, 256)
bytearray(iter([256]))       byte must be in range(0, 256)
bytearray().extend([256])    byte must be in range(0, 256)
ba[0:1] = [256]              byte must be in range(0, 256)
ba[0] = 256                  byte must be in range(0, 256)

collect_bytes is shared by all the constructor paths and hardcoded one message, so the bytearray ones were already right and only bytes was 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

    • Improved error messages when creating or extending bytes and bytearray objects with integers outside the valid range.
    • bytes now reports “bytes must be in range(0, 256)”, while bytearray reports “byte must be in range(0, 256)”.
    • Fixed bytearray creation from source objects to use the correct bytearray conversion behavior.
  • Tests

    • Added coverage for out-of-range values such as 256 and -1 across relevant operations.

`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
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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):

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 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 👍 / 👎.

Comment thread crates/vm/src/byte.rs
// 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| {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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
@rawsun007

Copy link
Copy Markdown
Contributor Author

the p2 one was right and it was a real regression, thanks. PyByteArray_FromObject calls bytes_from_object, so my change made the c-level bytearray constructor say "bytes". it now uses bytearray_from_object, which carries the singular wording and the constructor's unsized iteration, in abd0e65. i could not run that path here: bytearrayobject::tests::bytearray_from_object segfaults on clean main in my environment too, single threaded, so i am going by reading rather than a run.

checking cpython for the other three callers, int.from_bytes([256], 'big') also says "bytes must be in range(0, 256)", so builtins/int.rs is right to keep the new wording, and mmap.write([256]) raises a typeerror before any element conversion, so it never reaches either message.

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 @unittest.expectedFailure markers, which only exist in the vendored cpython suite. #8652 and #8659 both added cases to extra_tests/snippets and were merged. if the rule is meant to cover new snippet cases too then i have it wrong twice over and will drop them, just say so.

Assisted-by: Claude Opus 5 (Claude Code). the text above is mine, not Roshan's.

@rawsun007

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant