From aebbd424607b7797f231bc09f44226fcd3040d7e Mon Sep 17 00:00:00 2001 From: Michael Goulet <michael@errs.io> Date: Mon, 10 Mar 2025 16:50:29 +0000 Subject: [PATCH] Only prefer Sized candidates, and only if they certainly hold --- compiler/rustc_middle/src/traits/select.rs | 7 +++-- .../src/traits/select/candidate_assembly.rs | 6 ++-- .../src/traits/select/confirmation.rs | 4 +-- .../src/traits/select/mod.rs | 28 ++++++++----------- .../dont-incompletely-prefer-built-in.rs | 21 ++++++++++++++ ...incomplete-prefer-sized-builtin-over-wc.rs | 19 +++++++++++++ ...mplete-prefer-sized-builtin-over-wc.stderr | 27 ++++++++++++++++++ 7 files changed, 89 insertions(+), 23 deletions(-) create mode 100644 tests/ui/sized/dont-incompletely-prefer-built-in.rs create mode 100644 tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.rs create mode 100644 tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.stderr diff --git a/compiler/rustc_middle/src/traits/select.rs b/compiler/rustc_middle/src/traits/select.rs index 749f3c31bf8..aa2ee756bc5 100644 --- a/compiler/rustc_middle/src/traits/select.rs +++ b/compiler/rustc_middle/src/traits/select.rs @@ -95,8 +95,11 @@ pub type EvaluationCache<'tcx, ENV> = Cache<(ENV, ty::PolyTraitPredicate<'tcx>), /// parameter environment. #[derive(PartialEq, Eq, Debug, Clone, TypeVisitable)] pub enum SelectionCandidate<'tcx> { - /// UwU - SizedCandidate, + /// A built-in implementation for the `Sized` trait. This is preferred + /// over all other candidates. + SizedCandidate { + has_nested: bool, + }, /// A builtin implementation for some specific traits, used in cases /// where we cannot rely an ordinary library implementations. diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 9c0efec2e6c..316198f9e01 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -1062,8 +1062,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { candidates: &mut SelectionCandidateSet<'tcx>, ) { match self.sized_conditions(obligation) { - BuiltinImplConditions::Where(_) => { - candidates.vec.push(SizedCandidate); + BuiltinImplConditions::Where(nested) => { + candidates + .vec + .push(SizedCandidate { has_nested: !nested.skip_binder().is_empty() }); } BuiltinImplConditions::None => {} BuiltinImplConditions::Ambiguous => { diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 349eab2cbe3..29e0b833665 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -40,8 +40,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { candidate: SelectionCandidate<'tcx>, ) -> Result<Selection<'tcx>, SelectionError<'tcx>> { let mut impl_src = match candidate { - SizedCandidate => { - let data = self.confirm_builtin_candidate(obligation, true); + SizedCandidate { has_nested } => { + let data = self.confirm_builtin_candidate(obligation, has_nested); ImplSource::Builtin(BuiltinImplSource::Misc, data) } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 674ff30786b..956417b122c 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -1803,25 +1803,19 @@ impl<'tcx> SelectionContext<'_, 'tcx> { // We prefer `Sized` candidates over everything. let mut sized_candidates = - candidates.iter().filter(|c| matches!(c.candidate, SizedCandidate)); - if let Some(_sized_candidate) = sized_candidates.next() { + candidates.iter().filter(|c| matches!(c.candidate, SizedCandidate { has_nested: _ })); + if let Some(sized_candidate) = sized_candidates.next() { // There should only ever be a single sized candidate // as they would otherwise overlap. debug_assert_eq!(sized_candidates.next(), None); - return Some(SizedCandidate); - } - - // We prefer trivial builtin candidates, i.e. builtin impls without any nested - // requirements, over all others. This is a fix for #53123 and prevents winnowing - // from accidentally extending the lifetime of a variable. - let mut trivial_builtin = candidates - .iter() - .filter(|c| matches!(c.candidate, BuiltinCandidate { has_nested: false })); - if let Some(_trivial) = trivial_builtin.next() { - // There should only ever be a single trivial builtin candidate - // as they would otherwise overlap. - debug_assert_eq!(trivial_builtin.next(), None); - return Some(BuiltinCandidate { has_nested: false }); + // Only prefer the built-in `Sized` candidate if its nested goals are certain. + // Otherwise, we may encounter failure later on if inference causes this candidate + // to not hold, but a where clause would've applied instead. + if sized_candidate.evaluation.must_apply_modulo_regions() { + return Some(sized_candidate.candidate.clone()); + } else { + return None; + } } // Before we consider where-bounds, we have to deduplicate them here and also @@ -1950,7 +1944,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> { // Don't use impl candidates which overlap with other candidates. // This should pretty much only ever happen with malformed impls. if candidates.iter().all(|c| match c.candidate { - SizedCandidate + SizedCandidate { has_nested: _ } | BuiltinCandidate { has_nested: _ } | TransmutabilityCandidate | AutoImplCandidate diff --git a/tests/ui/sized/dont-incompletely-prefer-built-in.rs b/tests/ui/sized/dont-incompletely-prefer-built-in.rs new file mode 100644 index 00000000000..f5bf0c8915e --- /dev/null +++ b/tests/ui/sized/dont-incompletely-prefer-built-in.rs @@ -0,0 +1,21 @@ +//@ check-pass +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver + +struct W<T: ?Sized>(T); + +fn is_sized<T: Sized>(x: *const T) {} + +fn dummy<T: ?Sized>() -> *const T { todo!() } + +fn non_param_where_bound<T: ?Sized>() +where + W<T>: Sized, +{ + let x: *const W<_> = dummy(); + is_sized::<W<_>>(x); + let _: *const W<T> = x; +} + +fn main() {} diff --git a/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.rs b/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.rs new file mode 100644 index 00000000000..cc3303dccd5 --- /dev/null +++ b/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.rs @@ -0,0 +1,19 @@ +struct MyType<'a, T: ?Sized>(&'a (), T); + +fn is_sized<T>() {} + +fn foo<'a, T: ?Sized>() +where + (MyType<'a, T>,): Sized, + //~^ ERROR mismatched types + MyType<'static, T>: Sized, +{ + // Preferring the builtin `Sized` impl of tuples + // requires proving `MyType<'a, T>: Sized` which + // can only be proven by using the where-clause, + // adding an unnecessary `'static` constraint. + is_sized::<(MyType<'a, T>,)>(); + //~^ ERROR lifetime may not live long enough +} + +fn main() {} diff --git a/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.stderr b/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.stderr new file mode 100644 index 00000000000..a54574f743f --- /dev/null +++ b/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.stderr @@ -0,0 +1,27 @@ +error[E0308]: mismatched types + --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:7:23 + | +LL | (MyType<'a, T>,): Sized, + | ^^^^^ lifetime mismatch + | + = note: expected trait `<MyType<'a, T> as Sized>` + found trait `<MyType<'static, T> as Sized>` +note: the lifetime `'a` as defined here... + --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:5:8 + | +LL | fn foo<'a, T: ?Sized>() + | ^^ + = note: ...does not necessarily outlive the static lifetime + +error: lifetime may not live long enough + --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:15:5 + | +LL | fn foo<'a, T: ?Sized>() + | -- lifetime `'a` defined here +... +LL | is_sized::<(MyType<'a, T>,)>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ requires that `'a` must outlive `'static` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`.