add a QueryManyUniqueIter

[Victoronz]

Objective

Requires #13476.

Currently we cannot iterate mutably over QueryManyIter even if all entities we pass are unique.

Solution

Add a QueryManyUniqueIter wrapper around QueryManyIter for full Iterator trait iteration.
It can be constructed via either the dedup_entities or entities_all_unique method on QueryManyIter.
dedup_entities deduplicates the internal entity_iter, retaining order.
entities_all_unique takes self, checks for uniqueness, returns QueryManyUniqueIter on success, and QueryManyIter (Self) on failure.
Since QueryManyUniqueIter can iterate over mutable QueryData , it does not expose a fetch_next method.

Testing

The example/doc test shows the method in use, also making sure expect can be called on the return Result.

Changelog

Added dedup_entities and entities_all_unique methods on QueryManyIter.
Added QueryManyUniqueIter struct.

[chescock]

This duplicates the code in QueryManyIter::fetch_next_aliased_unchecked(). Could you share the implementation by having QueryManyUniqueIter wrap a whole QueryManyIter, and then implement Iterator::next() by calling fetch_next_aliased_unchecked() on the inner QueryManyIter?

[Victoronz]

IIRC, I was not sure whether the uniqueness property could simplify the next() call, so I kept them separate, but looking at it again, you're right, this can just be wrapper type! Good idea.

[chescock]

It might be useful to have methods to construct a QueryManyUniqueIter directly from a few iterator types that can already guarantee uniqueness, such as HashSet. That could let users skip the overhead of the uniqueness check.

It might also be useful to have a method that removes duplicates instead of returning Err, by doing something like

entity_iter: self
    .entity_iter
    .map(|e| *e.borrow())
    .collect::<EntityHashSet>()
    .into_iter(),

(Those could always be done in future PRs, of course.)

[Victoronz]

I agree with the methods to construct from unique iterators. What other source iterator types would you suggest?

Deduplication can already be done on the iterator itself with itertools, so isn't as important to have as the others. Still a nice to have though.
I wonder whether a sort + dedup would be faster than your HashSet version.
Edit: Actually just collecting into a BTreeSet might be simpler and faster. None of these retain input order however, that would be more expensive.

I think I'll make these separate follow-up PRs, since I have to keep these changes in line with #13443.

[Victoronz]

I have added dedup_entities on QueryManyIter to this PR anyway.

[chescock]

This is good cleanup, but could probably be a separate PR.

[Victoronz]

Fair, I'll put it in a different PR.

[Victoronz]

I wonder whether it might be easier to have a direct Query::iter_many_unique method instead of these roundabout conversions on QueryManyIter.

[Victoronz]

I have come up with a more powerful, general solution, which I think I will put in a different PR. I'll keep this open as the simpler alternative.

This PR would also need to be updated with a TrustedEq: Eq trait, because even though we only use it for Entity, unsafe code must not rely on an Eq trait impl for correctness, because Eq itself does not contain a logical correctness guarantee.
That being said, this would be a simple unsafe marker trait impl on Entity stating the safety contract.

[Victoronz]

This PR is now superseded by the concept of #16547, though the actual conversion API would be a follow-up to that PR.

[Victoronz]

#16547 has merged.