Skip to content

StorageVec Iterator#284

Open
davebryson wants to merge 2 commits into
OffchainLabs:mainfrom
davebryson:storagevec-iter
Open

StorageVec Iterator#284
davebryson wants to merge 2 commits into
OffchainLabs:mainfrom
davebryson:storagevec-iter

Conversation

@davebryson

Copy link
Copy Markdown

Description

Added support for Iterator/IntoIterator to StorageVec

Checklist

  • I have documented these changes where necessary.
  • I have read the DCO and ensured that these changes comply.
  • I assign this work under its open source licensing.

@rory-ocl

Copy link
Copy Markdown
Collaborator

Took a look at this. The approach is reasonable and works correctly for common value types, but there are a few things that would need addressing before merge:

Must fix:

  • The slot/offset calculation in Iter::next() duplicates the logic from StorageVec::index_slot(). This should reuse the existing method or extract a shared helper — if the packing logic is ever updated in one place, the other will silently diverge.

Should fix:

  • Missing size_hint() / ExactSizeIterator implementation. The iterator knows its exact length upfront (self.len - self.idx), so .collect::<Vec<_>>() can't pre-allocate without this.
  • The Iter struct should be re-exported from the storage module for discoverability.

Minor:

  • The into_iter test name is slightly misleading — it tests &StorageVec iteration via auto-ref, not a consuming iterator.

Not including this in the upcoming 0.10.3 release, but happy to revisit once these are addressed.

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