Skip to content

we avoid assembling impls shadowed by where-bounds #226

@lcnr

Description

@lcnr

cc rust-lang/rust#144732

This does not result in observable behavior differences. However, it is a 5% in many crates and used as a necessary optimization to avoid hangs, e.g. in rayon.

If we stabilize the new solver with this behavior, it will be more difficult to change our shadowing rules later on as considering more impls may result in unacceptable performance regressions.

Building on my reasoning on zulip:

Why could this make it harder to change our shadowing rules in the future

  • the perf cost of considering impls again is simply too large for code people are already writing today. This should mean that the perf gain of not considering impls is also worth it right now.
  • not considering impls for a while will cause people to write code which is currently prohibitively slow.
    • If such code is 'desirable', then that's just a reason to not consider impls.
    • Ignoring impls may allow people to write code which would otherwise hang/be very slow while not providing any benefits over otherwise possible code, i.e. users may rely on the new behavior by accident.
  • we'd have to spend a significant effort on improving perf before landing such a change. If we never take the easy way out by ignoring unused impl candidates we frontload this effort, making future changes easier. This does not feel like a good reason to me as blocking the stabilization of the new trait solver on this feels very undesirable :3

The only 'real' reason to not do this performance optimization is that people rely on possible behavior by accident. This feels unlikely to me. If people write impls which pass coherence and these impls currently get shadowed by where-clauses, then we shouldn't hang in these cases. if there are impls which cause hangs or large performance issues this is pretty much never something we'd accept. So even if people rely on such an impl due to this performance optimization, we should instead fix the hang for such a setup regardless of whether it's necessary to unblock shadowing changes.

What kind of changes to shadowing rules may be desirable

Avoid inference guidance due to where-bounds

If applying the ParamEnv candidates result in inference constraints, and there are also applicable impls, bail with ambiguity. This would cause the following to compile.

fn foo<T>(x: u32) -> (T, u64)
where
    u32: Into<T>,
{
    (x.into(), x.into())
}

Doing this relies on assembling impl candidates if the ParamEnv candidate has any inference constraints.

Always consider always applicable impls when proving trait goals, only shadow for normalization

When proving trait goals, always use impl candidates if they result in no constraints at all, this would allow the following to compile

fn foo<'a, 'b>(x: &'a str)
where
    &'b str: Copy,
{
    let _ = (x, x);
}

Doing this relies on assembling impl candidates if the ParamEnv candidate has any inference constraints.

Treat where-bounds as global if they are superseded by an impl

The idea would be that a where-bound is global (and therefore does not shadow impls) if there exists an impl which is able to prove the where-bound. This would then allow normalizing via this impl even if a where-bound exists. The exact way to implement this is unclear, and it may rely on always assembling impl candidates, even if there's a trivially applicable where-bound.

Metadata

Metadata

Assignees

No one assigned

    Labels

    stabilization-report-relevantRelevant for stabilization report for new solver (new breakage, new capabilities)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions