Fix marker trait winnowing depending on impl order

[traviscross]

Story: I was looking through this code for an unrelated reason and happened to notice the duplicate condition. That seemed obviously wrong, especially in light of the comment, so I worked backward to what it affected, wrote a test that failed, and then found that the test matched an existing known-bug test, which pointed me to #109481.

---

The TypeOutlives handler in evaluate_predicate_recursively checks whether a type in a T: 'a predicate has free regions, bound regions, or inference variables -- and if so, returns EvaluatedToOkModuloRegions rather than EvaluatedToOk. The comment says "no free lifetimes or generic parameters", but the code checked has_non_region_infer() twice instead of checking has_non_region_param().

This meant that TypeOutlives(T, 'static) where T is a type parameter returned EvaluatedToOk -- claiming the result holds unconditionally -- when the correct answer is EvaluatedToOkModuloRegions.

The distinction matters during marker trait winnowing in prefer_lhs_over_victim, which uses
must_apply_considering_regions() (true only for EvaluatedToOk) to decide whether one overlapping impl beats another. With the bug, a T: 'static-bounded impl appeared equally as strong as an unrestricted impl, making the winner depend on source order. This caused spurious E0310 errors when the more-constrained impl happened to appear after the less-constrained one.

Fixes #109481.

This same symptom was originally filed as #84917 and fixed in PR #88139. Then PR #102472 rewrote the TypeOutlives handler, introducing the duplicate has_non_region_infer() and losing the param check, regressing this. Shortly after, #109481 was filed. It noted the connection to #102472 -- that it was only relevant after it -- but the duplicate condition was not noticed.

r? @lcnr

cc @oli-obk @nikomatsakis

[lcnr]

Good catch. We actually need to check for both.

If we have ?x: 'a, ?x could later be inferred to be T after all

[traviscross]

Good catch. We actually need to check for both.

If we have ?x: 'a, ?x could later be inferred to be T after all

That makes sense to me. On the other hand, I commented out the pred.0.has_non_region_infer() check and all the tests still pass. I timed out in trying to come up with a test that would fail. I think maybe things are being equated and normalized before we get here (but you'd know better on this). Even still, it would seem more correct to me to not lean on that and to check for both.

[lcnr]

I timed out in trying to come up with a test that would fail. I think maybe things are being equated and normalized before we get here (but you'd know better on this). Even still, it would seem more correct to me to not lean on that and to check for both.

We don't even try to prove things if the self type is an inference variable, so you need sth like

#![feature(marker_trait_attr)]
#[marker]
trait Marker<T> {}

impl<T: 'static> Marker<T> for u32 {}
impl<T> Marker<T> for u32 {} 

fn foo<T: Marker<U>, U>() -> U { todo!() }

fn bar<'a>() {
    let x = foo::<u32, _>();
    1i32.abs(); // guarantees we do trait solving before constraining `U`
    let _: *mut &'a () = x;
}

would need to look at the logging in select/mod.rs to see whether this does what u need