Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer built-in sized impls (and only sized impls) for rigid types always #138176

Merged
merged 3 commits into from
Mar 31, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion compiler/rustc_middle/src/traits/select.rs
Original file line number Diff line number Diff line change
@@ -95,10 +95,16 @@ pub type EvaluationCache<'tcx, ENV> = Cache<(ENV, ty::PolyTraitPredicate<'tcx>),
/// parameter environment.
#[derive(PartialEq, Eq, Debug, Clone, TypeVisitable)]
pub enum SelectionCandidate<'tcx> {
/// 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.
///
/// 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 {
Original file line number Diff line number Diff line change
@@ -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,27 @@ 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(nested) => {
candidates
.vec
.push(SizedCandidate { has_nested: !nested.skip_binder().is_empty() });
}
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>,
Original file line number Diff line number Diff line change
@@ -40,6 +40,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
candidate: SelectionCandidate<'tcx>,
) -> Result<Selection<'tcx>, SelectionError<'tcx>> {
let mut impl_src = match candidate {
SizedCandidate { has_nested } => {
let data = self.confirm_builtin_candidate(obligation, has_nested);
ImplSource::Builtin(BuiltinImplSource::Misc, data)
}

BuiltinCandidate { has_nested } => {
let data = self.confirm_builtin_candidate(obligation, has_nested);
ImplSource::Builtin(BuiltinImplSource::Misc, data)
27 changes: 16 additions & 11 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
@@ -1801,17 +1801,21 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
return Some(candidates.pop().unwrap().candidate);
}

// 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
// We prefer `Sized` candidates over everything.
let mut sized_candidates =
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!(trivial_builtin.next(), None);
return Some(BuiltinCandidate { has_nested: false });
debug_assert_eq!(sized_candidates.next(), None);
// 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
@@ -1940,7 +1944,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 { has_nested: _ }
| BuiltinCandidate { has_nested: _ }
| TransmutabilityCandidate
| AutoImplCandidate
| ClosureCandidate { .. }
21 changes: 21 additions & 0 deletions tests/ui/sized/dont-incompletely-prefer-built-in.rs
Original file line number Diff line number Diff line change
@@ -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() {}
9 changes: 9 additions & 0 deletions tests/ui/traits/incomplete-infer-via-sized-wc.current.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0282]: type annotations needed
--> $DIR/incomplete-infer-via-sized-wc.rs:15:5
|
LL | is_sized::<MaybeSized<_>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^ 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`.
9 changes: 9 additions & 0 deletions tests/ui/traits/incomplete-infer-via-sized-wc.next.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0282]: type annotations needed
--> $DIR/incomplete-infer-via-sized-wc.rs:15:5
|
LL | is_sized::<MaybeSized<_>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^ 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`.
19 changes: 19 additions & 0 deletions tests/ui/traits/incomplete-infer-via-sized-wc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver

// Exercises change in <https://github.com/rust-lang/rust/pull/138176>.

struct MaybeSized<T: ?Sized>(T);

fn is_sized<T: Sized>() -> Box<T> { todo!() }

fn foo<T: ?Sized>()
where
MaybeSized<T>: Sized,
{
is_sized::<MaybeSized<_>>();
//~^ ERROR type annotations needed
Comment on lines +15 to +16
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting explicitly here that at least part of the point of the crater run was to check that nobody was relying on this inference behavior.

}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//@ check-pass
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver

// Exercises change in <https://github.com/rust-lang/rust/pull/138176>.

trait Trait<T>: Sized {}
impl<T> Trait<T> for T {}

fn is_sized<T: Sized>() {}

fn normal_ref<'a, 'b, T>()
where
&'a u32: Trait<T>,
{
is_sized::<&'b u32>();
}

struct MyRef<'a, U: ?Sized = ()>(&'a u32, U);
fn my_ref<'a, 'b, T>()
where
MyRef<'a>: Trait<T>,
{
is_sized::<MyRef<'b>>();
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error[E0308]: mismatched types
--> $DIR/lifetime-incomplete-prefer-sized-builtin-over-wc.rs:13: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: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: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 E0308`.
Original file line number Diff line number Diff line change
@@ -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`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver

// Exercises change in <https://github.com/rust-lang/rust/pull/138176>.

struct MyType<'a, T: ?Sized>(&'a (), T);

fn is_sized<T>() {}

fn foo<'a, T: ?Sized>()
where
(MyType<'a, T>,): Sized,
//[current]~^ ERROR mismatched types
//[next]~^^ ERROR lifetime bound not satisfied
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() {}
Loading