From fbf38028960b5b1d9e33571e81d9f305d8a64377 Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Sat, 14 Mar 2026 05:10:11 +0000 Subject: [PATCH 1/2] Fix marker trait winnowing depending on impl order The `TypeOutlives` handler in `evaluate_predicate_recursively` means to check whether a type in a `T: 'a` predicate has free regions, bound regions, non-region inference variables, or non-region generic parameters -- and if so, to return `EvaluatedToOkModuloRegions` rather than `EvaluatedToOk`. Correspondingly, the comment says "no free lifetimes or generic parameters". But the code was mistakenly checking `has_non_region_infer()` twice instead of checking both `has_non_region_infer()` and `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 impl beats another when there are multiple candidates. 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. This fixes Rust issue 109481. See PR 153847. This same symptom was originally filed as issue 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. Around this same time, issue 109481 was filed. It noted the connection to PR 102472 -- that the bug only appeared after it -- but the duplicate condition was not noticed. --- .../src/traits/select/mod.rs | 2 +- .../overlapping-impl-1-modulo-regions.rs | 14 +++++++------- .../overlapping-impl-1-modulo-regions.stderr | 17 ----------------- 3 files changed, 8 insertions(+), 25 deletions(-) delete mode 100644 tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index eadc937639f20..699fef1b67f07 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -758,7 +758,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { if pred.0.has_free_regions() || pred.0.has_bound_regions() || pred.0.has_non_region_infer() - || pred.0.has_non_region_infer() + || pred.0.has_non_region_param() { Ok(EvaluatedToOkModuloRegions) } else { diff --git a/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs index b69b893ecc4f1..000826c678658 100644 --- a/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs +++ b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs @@ -1,12 +1,12 @@ -//@ known-bug: #109481 +//@ check-pass +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver // -// While the `T: Copy` is always applicable when checking -// that the impl `impl F for T {}` is well formed, -// the old trait solver can only approximate this by checking -// that there are no inference variables in the obligation and -// no region constraints in the evaluation result. +// Regression test for #109481 and #84917. See #153847. // -// Because of this we end up with ambiguity here. +// A bug previously made marker trait winnowing order-dependent, +// producing a spurious E0310 here. #![feature(marker_trait_attr)] #[marker] diff --git a/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr deleted file mode 100644 index daee4e66ad102..0000000000000 --- a/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr +++ /dev/null @@ -1,17 +0,0 @@ -error[E0310]: the parameter type `T` may not live long enough - --> $DIR/overlapping-impl-1-modulo-regions.rs:14:21 - | -LL | impl F for T {} - | ^ - | | - | the parameter type `T` must be valid for the static lifetime... - | ...so that the type `T` will meet its required lifetime bounds - | -help: consider adding an explicit lifetime bound - | -LL | impl F for T {} - | +++++++++ - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0310`. From 9a122c98e37f5eb9f2d260bcdac75b28544decdd Mon Sep 17 00:00:00 2001 From: Travis Cross Date: Sun, 17 May 2026 17:36:56 +0000 Subject: [PATCH 2/2] Add tests for marker trait winnowing It's harder than one would imagine to demonstrate that the `has_non_region_infer()` check in the `TypeOutlives` handler is load bearing (even though the check seems right analytically). Fortunately, we did find a way to show this. Let's add that test. In working that out, we found two other interesting ways of showing that the `has_non_region_param()` check matters. Let's add those too. Though we're concerned here with code in the old solver, we test against both. --- ...verlapping-impl-assoc-ty-binding-static.rs | 79 +++++++++++++++++++ ...verlapping-impl-infer-resolved-to-param.rs | 52 ++++++++++++ .../overlapping-impl-projection-static.rs | 41 ++++++++++ 3 files changed, 172 insertions(+) create mode 100644 tests/ui/marker_trait_attr/overlapping-impl-assoc-ty-binding-static.rs create mode 100644 tests/ui/marker_trait_attr/overlapping-impl-infer-resolved-to-param.rs create mode 100644 tests/ui/marker_trait_attr/overlapping-impl-projection-static.rs diff --git a/tests/ui/marker_trait_attr/overlapping-impl-assoc-ty-binding-static.rs b/tests/ui/marker_trait_attr/overlapping-impl-assoc-ty-binding-static.rs new file mode 100644 index 0000000000000..7fbb63219acc4 --- /dev/null +++ b/tests/ui/marker_trait_attr/overlapping-impl-assoc-ty-binding-static.rs @@ -0,0 +1,79 @@ +//@ check-pass +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver +// +// This demonstrates that the `has_non_region_infer()` +// disjunctive check in the `TypeOutlives` handler in +// `evaluate_predicate_recursively` is load bearing with respect to +// marker trait winnowing. +// +// It's harder than one might imagine to demonstrate this. The outer +// obligation must be a marker trait predicate, otherwise winnowing +// gives up on any nontrivial multi-candidate case. This prevents us +// from simply making the outer obligation a type-outlives obligation. +// +// But the outer obligation must also be free of nonregion infer +// vars. Otherwise, when we're considering candidates, each +// call to `prefer_lhs_over_victim` will return `false` and +// we'll get ambiguity. +// +// Together, these imply that the infer var must come from the +// impl. It can't come from our starting obligation. So we need a +// subobligation, produced by candidate evaluation, that is both 1) a +// `TypeOutlives` predicate and 2) has an infer var. +// +// To make the would-be bug visible, we need 1) the candidate with the +// infer var to be picked as the winner, 2) the winner's WCs to fail, +// and 3) at least one other candidate in the winnowing set that gets +// marked `EvaluatedToOK`. +// +// How do we introduce an inference variable from an impl? All +// inference variables used in the trait ref (the trait and its +// generic arguments, including `Self`) will have been resolved +// and substituted by this point. We can't simply constrain a type +// parameter only by a lifetime -- that will produce an error about +// the parameter not being constrained (E0207). Fortunately, if +// we bind that parameter to a trait associated type it will be +// treated as constrained. Then, we can add a type-outlives bound +// on it. This then survives as an infer var all the way to the +// `TypeOutlives` handler. +// +// Without the `has_non_region_infer()` check in the `TypeOutlives` +// handler, this `?U: 'static` would produce `EvaluatedToOk` rather +// than `EvaluatedToOkModuloRegions`. Because this impl appears +// second, marker trait winnowing would pick it as the winner and we'd +// register the `?U: 'static` obligation. That would then fail, after +// we substitute `?U = X`, and we'd get a spurious "may not live long +// enough" (E0310) error. +// +// With the check, the second impl gets `EvaluatedToOkModuloRegions`, +// so the first impl correctly wins. +// +// See #153847. +#![feature(marker_trait_attr)] + +trait Assoc { + type Ty; +} +impl Assoc for T { + type Ty = T; +} + +#[marker] +trait Marker {} + +impl Marker for T where T: Assoc {} +impl Marker for T +where + T: Assoc, + U: 'static, +{ +} + +fn requires() {} +fn use_marker() { + requires::(); +} + +fn main() {} diff --git a/tests/ui/marker_trait_attr/overlapping-impl-infer-resolved-to-param.rs b/tests/ui/marker_trait_attr/overlapping-impl-infer-resolved-to-param.rs new file mode 100644 index 0000000000000..49b2249f109ee --- /dev/null +++ b/tests/ui/marker_trait_attr/overlapping-impl-infer-resolved-to-param.rs @@ -0,0 +1,52 @@ +//@ check-pass +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver +// +// This demonstrates that the `has_non_region_param()` +// disjunctive check in the `TypeOutlives` handler in +// `evaluate_predicate_recursively` is load bearing with respect to +// marker trait winnowing. +// +// Conversely, this does *not* show that the `has_non_region_infer()` +// check is load bearing -- you could be forgiven for thinking that +// it would. The `f::` will result in a `u8: Marker` +// obligation; candidate evaluation will produce a `?U: 'static` +// subobligation. The infer var *will* reach the check in +// question. But then, in `prefer_lhs_over_victim`, we check for +// `!has_non_region_infer` (there's a long comment there describing +// the subtle reason why). This causes us to defer making a +// selection. At a later point, when we check again, we'll have +// equated and substituted `?U = X`, so there won't be an infer var, +// but there will be a type parameter. +// +// Without the `has_non_region_param()` check in the +// `TypeOutlives` handler, the substituted `X: 'static` obligation +// from the impl would produce `EvaluatedToOk` rather than +// `EvaluatedToOkModuloRegions`. Because this impl appears second, +// marker trait winnowing would pick it as the winner and we'd +// register the `X: 'static` obligation. That would then fail, and +// we'd get a spurious "may not live long enough" (E0310) error. +// +// With the check, the second impl gets `EvaluatedToOkModuloRegions`, +// so the first impl correctly wins. +// +// See #153847. +#![feature(marker_trait_attr)] + +#[marker] +trait Marker {} + +impl Marker for u8 {} +impl Marker for u8 {} + +fn f, U>() -> U { + loop {} +} + +fn g() { + let x = f::(); + let _: X = x; +} + +fn main() {} diff --git a/tests/ui/marker_trait_attr/overlapping-impl-projection-static.rs b/tests/ui/marker_trait_attr/overlapping-impl-projection-static.rs new file mode 100644 index 0000000000000..d23fc1657b94e --- /dev/null +++ b/tests/ui/marker_trait_attr/overlapping-impl-projection-static.rs @@ -0,0 +1,41 @@ +//@ check-pass +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver +// +// This demonstrates that the `has_non_region_param()` +// disjunctive check in the `TypeOutlives` handler in +// `evaluate_predicate_recursively` is load bearing with respect to +// marker trait winnowing. +// +// This test is interesting because the projection reaches the +// `TypeOutlives` handler unnormalized. +// +// If not for the `has_non_region_param()` disjunct, the +// `::Ty: 'static` would get `EvaluatedToOk` rather +// than `EvaluatedToOkModuloRegions` and cause winnowing to select +// the second impl. The `'static` bound then gets registered as +// a region obligation that borrowck can't discharge, causing a +// spurious "may not live long enough" (E0310) error to be reported +// against the first impl. +// +// With the check, the second impl gets `EvaluatedToOkModuloRegions`, +// so the first impl correctly wins. +// +// See #153847. +#![feature(marker_trait_attr)] + +#[marker] +trait Marker {} + +trait Assoc { + type Ty; +} +impl Assoc for T { + type Ty = T; +} + +impl Marker for T where ::Ty: Copy {} +impl Marker for T where ::Ty: 'static {} + +fn main() {}