Lifetime bounds differ between WorldQuery and SystemParam, making rustc grumpy

[alice-i-cecile]

The problem

Currently, SystemParam is only implemented for Query types that are 'static: these types cannot contain any temporary references.

The key code is:

impl<Q: WorldQuery + 'static, F: ROWorldQuery + 'static> SystemParam for Query<'_, '_, Q, F>{
 // Actual code
}

By contrast, no equivalent bound exists on WorldQuery.

The net effect of this is that:

fn foo<'a>() {
  let _: Query<&'a Transform>;
}

compiles, since &'a Transform implements WorldQuery, no matter what the actual lifetime 'a is.

However, this is not a valid SystemParam, unless that lifetime is actually static (aka 'a: 'static).

Why this matters

So, we've seen users run into pain here within Bevy: see #7447 and #8192. Avoiding this footgun would be inherently good!

But the much larger problem comes when Rust wants to improve how implied bounds are computed. This, in a truly unprecedented fashion, breaks Bevy and effectively nothing else, because of the extremely normal things we do to the type system.

To explain their proposed changes:

1. When writing Rust programs, you don't always have to explicitly write out the exact lifetimes that are needed.
2. Instead, the compiler can sometimes infer what lifetimes must exist in order for your program to function: this is called "implied bounds".
3. However, the current approach to doing this is pretty ad-hoc, and underspecified.
4. In particular, the existing design uses trait solver flavored logic to do this in some cases (which Bevy hits) by examining the trait impls used.
5. This is both sketchy, and is in conflict with some more sensible implied bounds work that is currently missing.
6. So they want to change how it works!

The consequence of that change (if both Bevy and rustc hold their courses) is that Bevy users will get a very confusing, un-silenceable and unactionable lint (at times). In the future, this would be on the path to become a true compiler error.

What times? Well, @BoxyUwU did some digging, and discovered that the lint fires in exactly two places in our code base: both on our two internal uses of ParamSet. Experimenting more, this triggers on any use of ParamSet that involves queries with a Q or F type that have lifetimes that are not known to be static. In Bevy, that means &T and &mut T` query types.

The reason for this gets back to the problem at the top of this issue. Somewhere, we're currently relying on these implied bounds to effectively transfer those 'static lifetime requirements down into the Query, via the power of trait magic. So when rustc stops relying on trait information for implied bounds, generic types that combine WorldQuery and SystemParam with unconstrained lifetimes fall afoul of the new rules.

How can we fix this?

As @BoxyUwU and I see it, there are two fundamental approaches by which we could fix this.

1. Make WorldQuery more 'static, by adding bounds everywhere.
2. Make WorldQuery's SystemParam impl not require 'static

Either way, the discrepancy disappears, and we stop relying on implied bounds from trait impls to get the two parts to play nice.

Approach 1 is likely to improve end user ergonomics when working with custom SystemParam. There's a small chance that it regresses ergonomics in end user code (very bad!), by for example requiring users to write out &'static Transform in their queries. It also feels "more correct": using non static lifetimes in WorldQuery types doesn't seem to ever be correct: we're just type-punning with references.

This also may not work without help from rustc: trait WorldQuery: 'static in combination with Query<Q: WorldQuery, F: WorldQuery> should imply that Q and F are always 'static, but it's not clear that it currently does.

Approach 2 would be nicely targeted, and bring the impl into line with other implementations of SystemParam and how we implement WorldQuery (none of which requires 'static). However, it may not work, or require complex unsafe code to get working.

How do I test if my fix worked?

You will need to:

1. Create a branch of Bevy with your proposed fix.
2. Get the correct version of rustc, with the proposed PR included.
3. Set up your rustup toolchain so then it links to your local rustc build, following the contributing guide below.
4. Build your branch with cargo +stage1 build.

To get the correct version of rustc, follow their contributing guide. Alternatively, you may be able to pull in a cached version more easily using[rustup-toolchain-install-master] (https://github.com/kennytm/rustup-toolchain-install-master).

@lcnr warns me that this PR is somewhat stale (from May 2023), and should probably be rebased. If that happens, you'll want to test with the rebased version instead to get more accurate results.

[BD103]

As a small update, rust-lang/rust#118553 has landed stable Rust 1.77. If there were any uncaught errors from nightly users, they will likely surface now.

[janhohenheim]

Worth linking the following tracking issue: rust-lang/rust#119956