Sitelet https://github.com/starkware-libs/cairo/pull/10051
Skip to content

bugfix(corelib): Fixed ByteSpan::get OOB empty range. - #10051

Merged
orizi merged 1 commit into
mainfrom
orizi/06-07-bugfix_corelib_fixed_bytespan_get_oob_empty_range
Jun 7, 2026
Merged

orizi merged 1 commit into
mainfrom
orizi/06-07-bugfix_corelib_fixed_bytespan_get_oob_empty_range

Conversation

@orizi

@orizi orizi commented Jun 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

ByteSpan::get with a range where start == end previously always returned Some(Default::default()) (an empty slice), even when the range was entirely out of bounds. It now returns None when range.start > self.len().


Type of change

Please check one:

  • Bug fix (fixes incorrect behavior)
  • New feature
  • Performance improvement
  • Documentation change with concrete technical impact
  • Style, wording, formatting, or typo-only change

⚠️ Note:
To keep maintainer workload sustainable, we generally do not accept PRs that
are only minor wording, grammar, formatting, or style changes.
Such PRs may be closed without detailed review.


Why is this change needed?

An empty range (start == end) that falls beyond the end of the span was silently returning a valid empty slice instead of signaling that the index is out of bounds. This is inconsistent with how non-empty out-of-bounds ranges are handled (they return None) and with the general contract that get returns None for invalid indices.


What was the behavior or documentation before?

span.get(n..n) returned Some("") for any value of n, regardless of whether n was within the bounds of the span.


What is the behavior or documentation after?

span.get(n..n) returns Some("") only when n <= span.len(), and returns None when n > span.len(), matching the behavior of out-of-bounds non-empty range queries.


Related issue or discussion (if any)


Additional context

Tests were added for out-of-bounds empty-range queries on spans of various lengths (including spans backed by 30-byte and 31-byte ByteArrays) to cover the boundary conditions around word alignment.

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

orizi commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@orizi
orizi marked this pull request as ready for review June 7, 2026 11:10
@cursor

cursor Bot commented Jun 7, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Small, localized change to corelib slicing semantics with added tests; callers that relied on OOB empty ranges returning Some may see different behavior.

Overview
Fixes ByteSpan range get when start == end: empty slices at indices within the span still return Some (default empty ByteSpan), but empty ranges whose start is past the end now return None instead of a bogus empty slice.

This matches the existing docs (“out of bounds: returns None”) and affects only the early-return path for zero-length ranges; non-empty ranges are unchanged. Tests in test_span_slice_is_empty cover boundary cases (e.g. 5..5 vs 6..6) and multi–31-byte layouts.

Reviewed by Cursor Bugbot for commit 1a61329. Bugbot is set up for automated code reviews on this repo. Configure here.

@TomerStarkware TomerStarkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

:lgtm:

@TomerStarkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).

@orizi
orizi added this pull request to the merge queue Jun 7, 2026
Merged via the queue into main with commit 20fb35b Jun 7, 2026
54 checks passed
@orizi
orizi deleted the orizi/06-07-bugfix_corelib_fixed_bytespan_get_oob_empty_range branch June 7, 2026 13:16
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.

3 participants