Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement DequeView on top of #486 #490

Merged
merged 3 commits into from
Jul 1, 2024

Conversation

sosthene-nitrokey
Copy link
Contributor

Efforts have been made to avoid breaking changes:

  • Since capacity and len were const, they have been kept on Deque, and the generic version on DequeInner are prefixed by storage and are not const
  • Iter<'a, T, const N: usize> is kept, and a new IterView type is added with conversions to and from both iter types. (I also simplified significantly the implementation of the Iterator, by using the standard library's iterator combinators).

By accepting a backward incompatible change, these APIs could be made more intuitive.

@Dirbaio
Copy link
Member

Dirbaio commented Jul 1, 2024

Iter, IterMut weren't public before becuase mod deque wasn't, so I think changing them is not breaking, I think? I'd just do the changes to them directly, keeps the code simpler.

The const len/capacity thing is unfortunate. I guess we can live with it for 0.8.x then clean it up for 0.9 which should be soon anyway (for example i'd like to make all per-container mods public and remove all the reexports at the crate root. the docs are getting hard to read with everything mixed up).

@sosthene-nitrokey
Copy link
Contributor Author

Oh, that's right for the iterators!

I would go even further and return directly iter::Chain<slice::Iter, _> in the methods. That would reduce the code we need to implement, and give us for free all the "goodies" that iterators can have (double ended, size_hint, trustedsize, skip...). Do you think it's a good idea?

@sosthene-nitrokey
Copy link
Contributor Author

sosthene-nitrokey commented Jul 1, 2024

Should I create a issue to track breaking changes for the future 0.9 release?

@Dirbaio
Copy link
Member

Dirbaio commented Jul 1, 2024

I would go even further and return directly iter::Chain<slice::Iter, _> in the methods.

perhaps that's a bit too far? we'd be leaking the details on how it's implemented, so we can't change it later if we find it doesn't work for some reason (can't think of why that'd be, but you never know...).

an alternative is RPIT but then the user can't name the iterator. this'd be the perfect use case for TAIT when it's stable... for now I think a wrapper struct is best even if it's a bit verbose.

Should I create a issue to track breaking changes for the future 0.9 release?

why not, that'd be great!

@Dirbaio Dirbaio added this pull request to the merge queue Jul 1, 2024
Merged via the queue into rust-embedded:main with commit 02cc494 Jul 1, 2024
22 checks passed
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