Skip to content

Conversation

@kirillsemyonkin
Copy link
Collaborator

@kirillsemyonkin kirillsemyonkin commented Mar 7, 2025

This PR is a variant of implementing IntoIterator via self-reference to the respective collections (IArray, IMap).

These implementations are done with the help of the ouroboros crate. This means it uses unsafe (in the macro-generated code, I believe) to achieve self-reference. This may be undesired if fully safe code is wanted.

From an API point, this solution seems ideal - any struct will be taken by ownership and allows for these IntoIter structs to outlive scope they are made in.

The IArray implementation is rewritten to the same approach for consistency. No breaking changes.

Closes #68 because the two approaches are disjoint.

@kirillsemyonkin
Copy link
Collaborator Author

looks at PR number

Nice.

@cecton
Copy link
Contributor

cecton commented Mar 8, 2025

I prefer this approach because the end result is more usable for the end user. It might use unsafe internally in the ouroboros dependency but not in here and the code is perfectly safe right? Even std uses unsafe so I don't see the problem.

The only thing that bothers me is all the complexity introduced seems to be mostly related to handling IMap which I'm not sure is any useful at all. I've used implicit clone with yew for so long and never ever cared about IMap honestly. Do you know if it's used anywhere?

@kirillsemyonkin
Copy link
Collaborator Author

but not in here and the code is perfectly safe right?

I believe macros generate code with unsafe{} blocks, but it somehow passes #![deny(unsafe_code)] so I guess its fine.

Do you know if it's used anywhere?

It's used in my own code, same for #58. As for Yew, I believe Attributes needs some love.

@kirillsemyonkin kirillsemyonkin merged commit bf2d6a7 into yewstack:main Mar 10, 2025
6 checks passed
@kirillsemyonkin kirillsemyonkin deleted the self-referential-into-iters branch March 10, 2025 17:03
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