Added Query::for_each_combinations(_mut), Updated Query::for_each(_mut)

[bushrat011899]

Also updated function signatures of Query::for_each(_mut) to use a more restrictive lifetime on their borrows. This is a breaking change, but I believe it is the more "correct" API only recently made possible.

I also don't believe this breaking change is significant, as I don't believe there are any circumstances where you would need to escape a borrow out of a closure inside a for_each statement. iter().map(...) should provide all the required functionality in those cases anyway.

Objective

• Fixes Query::for_each_combinations(_mut) #4704

Solution

• Added Query::for_each_combinations and Query::for_each_combinations_mut with appropriate documentation.
• Updated Query::for_each and Query::for_each_mut to use closure-specific lifetime through for<'item>

---

Changelog

• Query::for_each and Query::for_each_mut provide references to their closures that no longer have the same lifetime as &self, and instead only last as long as the execution of the closure through the use of for<'item>. This is a breaking change.

Migration Guide

Code which used Query::for_each or Query::for_each_mut to borrow data from a query should instead use Query::iter or Query::iter_mut with .map(...) to achieve the same result. In practice, I don't believe there are many instances where this breaking change would actually affect real projects, as it appears to be a poor way to borrow data from a Query.

// Now fails to compile.
{
    let mut borrows: Vec<&Foo> = Vec::new();
    query.for_each(|data| borrows.push(data));
}
/*
error[E0521]: borrowed data escapes outside of closure
  --> tests/ui/query_lifetime_safety.rs:93:35
   |
 2 |     let mut borrows: Vec<&Foo> = Vec::new();
   |         ----------- `borrows` declared here, outside of the closure body
 3 |     query.for_each(|data| borrows.push(data));
   |                     ----  ^^^^^^^^^^^^^^^^^^ `data` escapes the closure body here
   |                     |
   |                     `data` is a reference that is only valid in the closure body
*/

// Proposed replacement.
{
    let _borrows: Vec<&Foo> = query.iter().collect();
}

[joseph-gio]

What's the motivation for making the lifetimes more restrictive for the old functions? I don't think it's the end of the world if they become more restrictive, but why not leave it as-is?

[bushrat011899]

What's the motivation for making the lifetimes more restrictive for the old functions? I don't think it's the end of the world if they become more restrictive, but why not leave it as-is?

Because for_each_combinations(_mut) requires the narrow lifetime, and it's so similar in functionality to for_each(_mut), I felt it was more beneficial for them to have matching narrow lifetimes, instead of mismatched broad ones. If the API change isn't worth it, I'm totally open to removing it from this PR, I just personally felt it made sense to combine these changes.

[hymm]

I don't think we should change the lifetime on for_each. The lifetimes are mostly transparent to users, but it could break existing code.