From f9696dda6e9d88a88c057e93fbc854104996bc33 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 7 Mar 2025 17:52:17 +0000 Subject: [PATCH 1/3] Prefer built-in sized impls for rigid types always --- compiler/rustc_middle/src/traits/select.rs | 5 +++- .../src/traits/select/candidate_assembly.rs | 24 +++++++++++++++---- .../src/traits/select/confirmation.rs | 5 ++++ .../src/traits/select/mod.rs | 13 +++++++++- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_middle/src/traits/select.rs b/compiler/rustc_middle/src/traits/select.rs index 811bd8fb4588a..749f3c31bf8db 100644 --- a/compiler/rustc_middle/src/traits/select.rs +++ b/compiler/rustc_middle/src/traits/select.rs @@ -95,10 +95,13 @@ 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 builtin implementation for some specific traits, used in cases /// where we cannot rely an ordinary library implementations. /// - /// The most notable examples are `sized`, `Copy` and `Clone`. This is also + /// The most notable examples are `Copy` and `Clone`. This is also /// used for the `DiscriminantKind` and `Pointee` trait, both of which have /// an associated type. BuiltinCandidate { 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 a8d8003ead6ea..9c0efec2e6cf6 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -86,10 +86,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // `Pointee` is automatically implemented for every type. candidates.vec.push(BuiltinCandidate { has_nested: false }); } else if tcx.is_lang_item(def_id, LangItem::Sized) { - // Sized is never implementable by end-users, it is - // always automatically computed. - let sized_conditions = self.sized_conditions(obligation); - self.assemble_builtin_bound_candidates(sized_conditions, &mut candidates); + self.assemble_builtin_sized_candidate(obligation, &mut candidates); } else if tcx.is_lang_item(def_id, LangItem::Unsize) { self.assemble_candidates_for_unsizing(obligation, &mut candidates); } else if tcx.is_lang_item(def_id, LangItem::Destruct) { @@ -1059,6 +1056,25 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { /// Assembles the trait which are built-in to the language itself: /// `Copy`, `Clone` and `Sized`. #[instrument(level = "debug", skip(self, candidates))] + fn assemble_builtin_sized_candidate( + &mut self, + obligation: &PolyTraitObligation<'tcx>, + candidates: &mut SelectionCandidateSet<'tcx>, + ) { + match self.sized_conditions(obligation) { + BuiltinImplConditions::Where(_) => { + candidates.vec.push(SizedCandidate); + } + BuiltinImplConditions::None => {} + BuiltinImplConditions::Ambiguous => { + candidates.ambiguous = true; + } + } + } + + /// Assembles the trait which are built-in to the language itself: + /// e.g. `Copy` and `Clone`. + #[instrument(level = "debug", skip(self, candidates))] fn assemble_builtin_bound_candidates( &mut self, conditions: BuiltinImplConditions<'tcx>, diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 4cd6781ab8905..349eab2cbe39b 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -40,6 +40,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { candidate: SelectionCandidate<'tcx>, ) -> Result, SelectionError<'tcx>> { let mut impl_src = match candidate { + SizedCandidate => { + let data = self.confirm_builtin_candidate(obligation, true); + ImplSource::Builtin(BuiltinImplSource::Misc, data) + } + BuiltinCandidate { 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 e1adabbeaa662..674ff30786bc3 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -1801,6 +1801,16 @@ impl<'tcx> SelectionContext<'_, 'tcx> { return Some(candidates.pop().unwrap().candidate); } + // 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() { + // 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. @@ -1940,7 +1950,8 @@ 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 { - BuiltinCandidate { has_nested: _ } + SizedCandidate + | BuiltinCandidate { has_nested: _ } | TransmutabilityCandidate | AutoImplCandidate | ClosureCandidate { .. } From aebbd424607b7797f231bc09f44226fcd3040d7e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 10 Mar 2025 16:50:29 +0000 Subject: [PATCH 2/3] 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 749f3c31bf8db..aa2ee756bc502 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 9c0efec2e6cf6..316198f9e012f 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 349eab2cbe39b..29e0b833665b7 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, 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 674ff30786bc3..956417b122c6a 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 0000000000000..f5bf0c8915e76 --- /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); + +fn is_sized(x: *const T) {} + +fn dummy() -> *const T { todo!() } + +fn non_param_where_bound() +where + W: Sized, +{ + let x: *const W<_> = dummy(); + is_sized::>(x); + let _: *const W = 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 0000000000000..cc3303dccd570 --- /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() {} + +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 0000000000000..a54574f743f8e --- /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 ` as Sized>` + found trait ` 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`. From c0230f4a2904e968f4afd833e44db1f4ebe29b21 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 13 Mar 2025 21:17:46 +0000 Subject: [PATCH 3/3] Flesh out tests --- ...complete-infer-via-sized-wc.current.stderr | 9 ++++++ .../incomplete-infer-via-sized-wc.next.stderr | 9 ++++++ .../traits/incomplete-infer-via-sized-wc.rs | 19 +++++++++++++ ...complete-prefer-sized-builtin-over-wc-2.rs | 28 +++++++++++++++++++ ...efer-sized-builtin-over-wc.current.stderr} | 6 ++-- ...e-prefer-sized-builtin-over-wc.next.stderr | 25 +++++++++++++++++ ...incomplete-prefer-sized-builtin-over-wc.rs | 9 +++++- 7 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 tests/ui/traits/incomplete-infer-via-sized-wc.current.stderr create mode 100644 tests/ui/traits/incomplete-infer-via-sized-wc.next.stderr create mode 100644 tests/ui/traits/incomplete-infer-via-sized-wc.rs create mode 100644 tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc-2.rs rename tests/ui/traits/{lifetime-incomplete-prefer-sized-builtin-over-wc.stderr => lifetime-incomplete-prefer-sized-builtin-over-wc.current.stderr} (85%) create mode 100644 tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.next.stderr diff --git a/tests/ui/traits/incomplete-infer-via-sized-wc.current.stderr b/tests/ui/traits/incomplete-infer-via-sized-wc.current.stderr new file mode 100644 index 0000000000000..f4930bf890c2b --- /dev/null +++ b/tests/ui/traits/incomplete-infer-via-sized-wc.current.stderr @@ -0,0 +1,9 @@ +error[E0282]: type annotations needed + --> $DIR/incomplete-infer-via-sized-wc.rs:15:5 + | +LL | is_sized::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `is_sized` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/tests/ui/traits/incomplete-infer-via-sized-wc.next.stderr b/tests/ui/traits/incomplete-infer-via-sized-wc.next.stderr new file mode 100644 index 0000000000000..f4930bf890c2b --- /dev/null +++ b/tests/ui/traits/incomplete-infer-via-sized-wc.next.stderr @@ -0,0 +1,9 @@ +error[E0282]: type annotations needed + --> $DIR/incomplete-infer-via-sized-wc.rs:15:5 + | +LL | is_sized::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `is_sized` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/tests/ui/traits/incomplete-infer-via-sized-wc.rs b/tests/ui/traits/incomplete-infer-via-sized-wc.rs new file mode 100644 index 0000000000000..9dcddea3551d1 --- /dev/null +++ b/tests/ui/traits/incomplete-infer-via-sized-wc.rs @@ -0,0 +1,19 @@ +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver + +// Exercises change in . + +struct MaybeSized(T); + +fn is_sized() -> Box { todo!() } + +fn foo() +where + MaybeSized: Sized, +{ + is_sized::>(); + //~^ ERROR type annotations needed +} + +fn main() {} diff --git a/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc-2.rs b/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc-2.rs new file mode 100644 index 0000000000000..8a8f7b933b539 --- /dev/null +++ b/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc-2.rs @@ -0,0 +1,28 @@ +//@ check-pass +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver + +// Exercises change in . + +trait Trait: Sized {} +impl Trait for T {} + +fn is_sized() {} + +fn normal_ref<'a, 'b, T>() +where + &'a u32: Trait, +{ + is_sized::<&'b u32>(); +} + +struct MyRef<'a, U: ?Sized = ()>(&'a u32, U); +fn my_ref<'a, 'b, T>() +where + MyRef<'a>: Trait, +{ + is_sized::>(); +} + +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.current.stderr similarity index 85% rename from tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.stderr rename to tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.current.stderr index a54574f743f8e..dd9393fae853d 100644 --- a/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.stderr +++ b/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.current.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:7:23 + --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:13:23 | LL | (MyType<'a, T>,): Sized, | ^^^^^ lifetime mismatch @@ -7,14 +7,14 @@ LL | (MyType<'a, T>,): Sized, = note: expected trait ` as Sized>` found trait ` as Sized>` note: the lifetime `'a` as defined here... - --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:5:8 + --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:11: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 + --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:22:5 | LL | fn foo<'a, T: ?Sized>() | -- lifetime `'a` defined here diff --git a/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.next.stderr b/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.next.stderr new file mode 100644 index 0000000000000..05861877d413b --- /dev/null +++ b/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.next.stderr @@ -0,0 +1,25 @@ +error[E0478]: lifetime bound not satisfied + --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:13:23 + | +LL | (MyType<'a, T>,): Sized, + | ^^^^^ + | +note: lifetime parameter instantiated with the lifetime `'a` as defined here + --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:11:8 + | +LL | fn foo<'a, T: ?Sized>() + | ^^ + = note: but lifetime parameter must outlive the static lifetime + +error: lifetime may not live long enough + --> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:22: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 E0478`. 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 index cc3303dccd570..ae7a6c9bba335 100644 --- a/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.rs +++ b/tests/ui/traits/lifetime-incomplete-prefer-sized-builtin-over-wc.rs @@ -1,3 +1,9 @@ +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver + +// Exercises change in . + struct MyType<'a, T: ?Sized>(&'a (), T); fn is_sized() {} @@ -5,7 +11,8 @@ fn is_sized() {} fn foo<'a, T: ?Sized>() where (MyType<'a, T>,): Sized, - //~^ ERROR mismatched types + //[current]~^ ERROR mismatched types + //[next]~^^ ERROR lifetime bound not satisfied MyType<'static, T>: Sized, { // Preferring the builtin `Sized` impl of tuples