Skip to content

Raise ValueError consistently for negative int -> unsigned conversions#6196

Open
teddytennant wants to merge 4 commits into
PyO3:mainfrom
teddytennant:fix-negative-unsigned-valueerror
Open

Raise ValueError consistently for negative int -> unsigned conversions#6196
teddytennant wants to merge 4 commits into
PyO3:mainfrom
teddytennant:fix-negative-unsigned-valueerror

Conversation

@teddytennant

Copy link
Copy Markdown

Extracting a negative Python int into an unsigned Rust integer previously raised an inconsistent exception type depending on the width and the Python version: OverflowError for u8/u16/u32 (try_from after PyLong_AsLong) and u64 (PyLong_AsUnsignedLongLong), but ValueError for u128 on 3.13+.

This standardizes on ValueError for the negative case across every unsigned width (u8u128, usize, and the NonZeroU* variants), matching the convention of newer CPython APIs such as PyLong_AsUInt64, as suggested in the issue. Genuine magnitude overflow of an in-range-sign value continues to raise OverflowError.

The remap is gated on <$rust_type>::MIN == 0 (and, in the 128-bit path, the existing $is_signed), and only fires on the already-cleared error path after confirming the value is negative, so signed types and too-large positive values are unaffected.

Adds the test_negative case to test_common! so it runs for all integer widths (signed types still accept -1; unsigned types must raise ValueError).

cargo test -p pyo3 --lib conversions::std::num

Fixes #6116.

Copilot AI review requested due to automatic review settings July 8, 2026 00:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codspeed-hq

codspeed-hq Bot commented Jul 8, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 12.26%

❌ 5 regressed benchmarks
✅ 135 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
iter_tuple 5.1 ms 6 ms -14.89%
tuple_get_borrowed_item_unchecked 2.9 ms 3.3 ms -13.45%
tuple_get_borrowed_item 3.5 ms 4 ms -11.19%
tuple_get_item_unchecked 3.6 ms 4.1 ms -10.89%
list_get_item_unchecked 3.7 ms 4.1 ms -10.82%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing teddytennant:fix-negative-unsigned-valueerror (fddb340) with main (8fcf8fc)

Open in CodSpeed

The negative-int-to-unsigned conversion now raises ValueError instead of
OverflowError, so update the expected traceback in test_conversion_error.
Negative values for unsigned time components now raise ValueError
(consistent with newer CPython int-conversion APIs), so rename
test_invalid_time_fails_overflow accordingly.
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.

negative-to-unsigned conversions are inconsistent between ValueError and OverflowError

2 participants