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

rawsun007 · 2026-09-09T09:52:28Z

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 · 2026-09-09T09:52:48Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: 32c82807-f439-4d18-b27d-f9e1b5a55b71

📥 Commits

Reviewing files that changed from the base of the PR and between e07c6b5 and abd0e65.

📒 Files selected for processing (1)

crates/capi/src/bytearrayobject.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

Byte conversion paths now report distinct out-of-range messages for bytes and bytearray. The C API bytearray constructor uses the bytearray conversion path. Tests cover construction, iterator conversion, and extension.

Changes

Byte range errors

Layer / File(s)	Summary
Conversion message routing `crates/vm/src/byte.rs`	`collect_bytes` accepts a caller-supplied element error message. `bytes` uses the plural message, while `bytearray` uses the singular message.
C API bytearray routing `crates/capi/src/bytearrayobject.rs`	`PyByteArray_FromObject` now converts sources with `bytearray_from_object`.
Range error validation `extra_tests/snippets/builtin_bytes.py`	Tests verify the expected `ValueError` messages for out-of-range values in construction, iteration, and extension.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to abd0e

Bytes now reports the CPython-compatible plural range error while bytearray retains singular wording across its conversion paths, including the C API constructor. The covered behavior change is ready to merge.

Suggested reviewers: bschoenmaeckers

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: reporting out-of-range byte elements with CPython-compatible errors.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector

💡 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".

chatgpt-codex-connector · 2026-09-09T09:55:47Z

 )
+
+
+def out_of_range(fn, message):


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

chatgpt-codex-connector · 2026-09-09T09:55:47Z

 // 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| {


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 · 2026-09-09T13:53:50Z

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 · 2026-09-09T15:03:24Z

the red check is the windows runner, not the diff. in "check whats_left is not broken" the release build failed with failed to remove file target\release\rustpython.exe: Access is denied. (os error 5), which is a file lock on the binary it was replacing. snippets and cpython tests passed on ubuntu and macos, and rust tests passed on windows. i cannot rerun it myself, needs admin rights.

Assisted-by: Claude Opus 5 (Claude Code).

chatgpt-codex-connector Bot reviewed Sep 9, 2026

View reviewed changes

Navigation Menu

Uh oh!

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

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

rawsun007 commented Sep 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Sep 9, 2026

Uh oh!

chatgpt-codex-connector Bot Sep 9, 2026

Uh oh!

rawsun007 commented Sep 9, 2026

Uh oh!

rawsun007 commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

rawsun007 commented Sep 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

rawsun007 commented Sep 9, 2026

Uh oh!

rawsun007 commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

rawsun007 commented Sep 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading