-
Notifications
You must be signed in to change notification settings - Fork 0
Inference hazard due to lazy alias relate: Break it in the old solver? #168
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
Comments
(cannot remember if this is already tracked as an issue lol) |
not sure 😅 doesn't really seem like it... definitely a known issue and I don't see how we can easily change this behavior in the old solver |
The parameter `In` of `call_inner` is completely unconstrained by its arguments and return type. We are only able to infer it by assuming that the only associated type equal to `In::Param<'_>` is `In::Param<'_>` itself. It could just as well be some other associated type which only normalizes to `In::Param<'_>`. This will change with the next-generation trait solver and was encountered by a crater run rust-lang/rust#133502 cc rust-lang/trait-system-refactor-initiative#168 I couldn't think of a cleaner alternative here. I first tried to just provide `In` as an explicit type parameter. This is also kinda ugly as I need to provide a variable number of them and `${ignore(..)}` is currently still unstable rust-lang/rust#83527. Sorry for the inconvenience. Also fun that this function exists to avoid a separate solver bug in the first place 😅
The parameter `In` of `call_inner` is completely unconstrained by its arguments and return type. We are only able to infer it by assuming that the only associated type equal to `In::Param<'_>` is `In::Param<'_>` itself. It could just as well be some other associated type which only normalizes to `In::Param<'_>`. This will change with the next-generation trait solver and was encountered by a crater run rust-lang/rust#133502 cc rust-lang/trait-system-refactor-initiative#168 I couldn't think of a cleaner alternative here. I first tried to just provide `In` as an explicit type parameter. This is also kinda ugly as I need to provide a variable number of them and `${ignore(..)}` is currently still unstable rust-lang/rust#83527. Sorry for the inconvenience. Also fun that this function exists to avoid a separate solver bug in the first place 😅
Thinking about this, esp wrt to the actual breakage to bevy-ecs bevyengine/bevy#18840, I expect that pretty much all use of this will be due to a function call. So instead of trying to detect cases where the old solver incorrectly relates arguments for higher ranked projections, we could instead future compat lint function definitions which can result in such patterns. I think the link should fire if:
|
So I'd like to note that the only reason that Bevy was triggering this bug was b/c of a redundant where clause: trait SystemInput {
type Param<'a>;
type Inner<'a>;
fn wrap(x: Self::Inner<'_>) -> Self::Param<'_>;
}
struct World;
fn run<In: SystemInput, Out>(
s: fn(In::Param<'_>, &mut World) -> Out,
world: &mut World,
input: In::Inner<'_>,
) -> Out
/// !!!!!!!!!! ///
where
fn(In::Param<'_>, &mut World) -> Out: Fn(In::Param<'_>, &mut World) -> Out,
/// !!!!!!!!!! ///
{
fn call_inner<In: SystemInput, Out>(
mut f: impl FnMut(In::Param<'_>, &mut World) -> Out,
input: In::Inner<'_>,
world: &mut World,
) -> Out {
f(In::wrap(input), world)
}
call_inner(s, input, world)
} Without it, we end up confirming an I'm not totally certain how common this case is, though, so I still think the major place this happens is |
I don't see a where-clause in the diff in bevyengine/bevy#18840 🤔 am I missing something that's hidden by the macro? |
No I was mistaken, it wasn't |
Also impacts trait Filter<Args> {}
trait FunctionArgs<'a> {
type Output;
}
fn callee<F, Args>(_: F)
where
Args: for<'a> FunctionArgs<'a>,
F: Filter<Args>,
F: for<'a> Filter<<Args as FunctionArgs<'a>>::Output>,
{
}
fn caller<F, Args>(x: F)
where
Args: for<'a> FunctionArgs<'a>,
F: Filter<Args>,
F: for<'a> Filter<<Args as FunctionArgs<'a>>::Output>,
{
callee(x)
} |
Lazy alias relate causes us to no longer (incompletely) infer alias args when we don't normalize aliases to infer vars. That causes this code to (rightfully, IMO) fail:
I'd like to prevent this from being more of an issue, but nothing comes to mind for a good scheme to weaken this inference in the old solver. Ideas?
The text was updated successfully, but these errors were encountered: