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`. 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() {}