Skip to content

Iter rework#20

Merged
jounathaen merged 10 commits intohermit-os:masterfrom
an-owl:iter-rework
Jan 23, 2026
Merged

Iter rework#20
jounathaen merged 10 commits intohermit-os:masterfrom
an-owl:iter-rework

Conversation

@an-owl
Copy link
Contributor

@an-owl an-owl commented Nov 14, 2025

pr for #19

  • Defined AddrIter to panic when iterating over non-canonical addresses.
  • Implemented DoubleEndedIterator and ExactSizeIterator
  • Added a number of method implementations for iterator traits to reduce unnecessary calls to Iterator::next()
  • Added an inclusive variant of the iterator. This is required because it cannot return the last address in a range without it. (e.g it will never return 0x7FFFFFFFFFFF it will stop at 0x7FFFFFFFFFFE).
  • Added From<Range> and From<RangeInclusive> for AddrIter

8637ae6 requires that MemoryAddress::RAW implements TryFrom<usize>, I don't really like further restricting it but it already implements TryInto<usize> so it probably wont be an issue.

There may be bugs where iterating to a non-canonical address when add/sub_checked() returns None, by assuming its an overflow, when it should be checked. I think I caught them all but there might be more.

Comment on lines +158 to +161
trait IterInclusivity: 'static {
fn exhausted<T: Ord>(start: &T, end: &T) -> bool;
}
pub enum NonInclusive {}
Copy link
Member

Choose a reason for hiding this comment

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

Minor nitpicks: Could you please add a doc comment to these pub items? Also, the line spacing is a little inconsistent.

Copy link
Member

@jounathaen jounathaen left a comment

Choose a reason for hiding this comment

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

Very nice! Thank you for the contribution!

@an-owl
Copy link
Contributor Author

an-owl commented Nov 16, 2025

Sorry for the wait. I've added docs to the pub items with some doctests too which managed to catch a bug.

I also added some code comments where I thought they were necessary.

@jounathaen
Copy link
Member

Very nice! Once Clippy is satisfied, I'm as well 😉

I assume you are interested in a release after the merge?

@an-owl
Copy link
Contributor Author

an-owl commented Nov 17, 2025

I assume you are interested in a release after the merge?

Not yet there are still a couple of things I want to add.

@jounathaen
Copy link
Member

@an-owl, can you fix the Clippy issue? Then I'd be happy to merge this :-)

@jounathaen jounathaen merged commit f5d13bb into hermit-os:master Jan 23, 2026
5 checks passed
@jounathaen
Copy link
Member

Ah, I didn't notice the fix. Thank you for your contribution!

@jounathaen
Copy link
Member

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