Skip to content

feat(io): impl Reader for std::io::Cursor#278

Merged
cpubot merged 1 commit intomasterfrom
std-io-cursor-read
Mar 30, 2026
Merged

feat(io): impl Reader for std::io::Cursor#278
cpubot merged 1 commit intomasterfrom
std-io-cursor-read

Conversation

@cpubot
Copy link
Copy Markdown
Contributor

@cpubot cpubot commented Mar 27, 2026

Implements Reader for Cursor<AsRef<[u8]>>, which is consistent with std's supported implementation.

@cpubot cpubot force-pushed the std-io-cursor-read branch from 3b2bd9c to c9876f3 Compare March 28, 2026 01:02
@cpubot cpubot requested a review from kskalski March 28, 2026 01:03
@cpubot cpubot force-pushed the std-io-cursor-read branch from c9876f3 to fd39bec Compare March 28, 2026 17:12
}

cursor.set_position(next_pos as u64);
let inner = cursor.get_ref().as_ref();
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.

I suppose re-borrow is typically avoided by the compiler, but maybe it would make sense to make non-mutating function returning (buf, new_pos) and let the callers set position? Maybe this will make copy_into_slice and take_array faster for non-trivial T impls

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

A function returning (buf, new_pos) would still require a reborrow. The reference to buf would be invalidated after calling set_position.

The codegen looks good here in the examples I tested. The bounds check was elided on &inner[pos..next_pos] and I didn't see anything that would warrant changing anything

@cpubot cpubot force-pushed the std-io-cursor-read branch from fd39bec to 31e1547 Compare March 29, 2026 17:19
@cpubot cpubot merged commit ba93f44 into master Mar 30, 2026
4 checks passed
@cpubot cpubot deleted the std-io-cursor-read branch March 30, 2026 00:33
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.

2 participants