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

compiler-errors · 2025-03-07T17:52:50Z

This PR changes the confirmation of Sized obligations to unconditionally prefer the built-in impl, even if it has nested obligations. This also changes all other built-in impls (namely, Copy/Clone/DiscriminantKind/Pointee) to not prefer built-in impls over param-env impls. This aligns the old solver with the behavior of the new solver.

In the old solver, we register many builtin candidates with the BuiltinCandidate { has_nested: bool } candidate kind. The precedence this candidate takes over other candidates is based on the has_nested field. We only prefer builtin impls over param-env candidates if has_nested is false

rust/compiler/rustc_trait_selection/src/traits/select/mod.rs

Lines 1804 to 1866 in 2b4694a

    
           // 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 }); 
        
           } 
        
           // Before we consider where-bounds, we have to deduplicate them here and also 
        
           // drop where-bounds in case the same where-bound exists without bound vars. 
        
           // This is necessary as elaborating super-trait bounds may result in duplicates. 
        
           'search_victim: loop { 
        
               for (i, this) in candidates.iter().enumerate() { 
        
                   let ParamCandidate(this) = this.candidate else { continue }; 
        
                   for (j, other) in candidates.iter().enumerate() { 
        
                       if i == j { 
        
                           continue; 
        
                       } 
        
                       let ParamCandidate(other) = other.candidate else { continue }; 
        
                       if this == other { 
        
                           candidates.remove(j); 
        
                           continue 'search_victim; 
        
                       } 
        
                       if this.skip_binder().trait_ref == other.skip_binder().trait_ref 
        
                           && this.skip_binder().polarity == other.skip_binder().polarity 
        
                           && !this.skip_binder().trait_ref.has_escaping_bound_vars() 
        
                       { 
        
                           candidates.remove(j); 
        
                           continue 'search_victim; 
        
                       } 
        
                   } 
        
               } 
        
               break; 
        
           } 
        
           // The next highest priority is for non-global where-bounds. However, while we don't 
        
           // prefer global where-clauses here, we do bail with ambiguity when encountering both 
        
           // a global and a non-global where-clause. 
        
           // 
        
           // Our handling of where-bounds is generally fairly messy but necessary for backwards 
        
           // compatibility, see #50825 for why we need to handle global where-bounds like this. 
        
           let is_global = |c: ty::PolyTraitPredicate<'tcx>| c.is_global() && !c.has_bound_vars(); 
        
           let param_candidates = candidates 
        
               .iter() 
        
               .filter_map(|c| if let ParamCandidate(p) = c.candidate { Some(p) } else { None }); 
        
           let mut has_global_bounds = false; 
        
           let mut param_candidate = None; 
        
           for c in param_candidates { 
        
               if is_global(c) { 
        
                   has_global_bounds = true; 
        
               } else if param_candidate.replace(c).is_some() { 
        
                   // Ambiguity, two potentially different where-clauses 
        
                   return None; 
        
               } 
        
           }

Preferring param-env candidates when the builtin candidate has nested obligations still ends up leading to detrimental inference guidance, like:

fn hello<T>() where (T,): Sized {
    let x: (_,) = Default::default();
    // ^^ The `Sized` obligation on the variable infers `_ = T`.
    let x: (i32,) = x;
    // We error here, both a type mismatch and also b/c `T: Default` doesn't hold.
}

Therefore this PR adjusts the candidate precedence of Sized obligations by making them a distinct candidate kind and unconditionally preferring them over all other candidate kinds.

Special-casing Sized this way is necessary as there are a lot of traits with a Sized super-trait bound, so a &'a str: From<T> where-bound results in an elaborated &'a str: Sized bound. People tend to not add explicit where-clauses which overlap with builtin impls, so this tends to not be an issue for other traits.

We don't know of any tests/crates which need preference for other builtin traits. As this causes builtin impls to diverge from user-written impls we would like to minimize the affected traits. Otherwise e.g. moving impls for tuples to std by using variadic generics would be a breaking change. For other builtin impls it's also easier for the preference of builtin impls over where-bounds to result in issues.

There are two ways preferring builtin impls over where-bounds can be incorrect and undesirable:

applying the builtin impl results in undesirable region constraints. E.g. if only MyType<'static> implements Copy then a goal like (MyType<'a>,): Copy would require 'a == 'static so we must not prefer it over a (MyType<'a>,): Copy where-bound
- this is mostly not an issue for Sized as all Sized impls are builtin and don't add any region constraints not already required for the type to be well-formed
- however, even with Sized this is still an issue if a nested goal also gets proven via a where-bound: playground
if the builtin impl has associated types, we should not prefer it over where-bounds when normalizing that associated type. This can result in normalization adding more region constraints than just proving trait bounds. candidate selection for normalization and trait goals disagree #133044
- not an issue for Sized as it doesn't have associated types.

r? lcnr

compiler-errors · 2025-03-07T17:53:03Z

@bors try @rust-timer queue

(and crater)

…try> Prefer built-in sized obligations for rigid types always r? lcnr

bors · 2025-03-07T17:54:14Z

⌛ Trying commit 49e4fbc with merge 51b1964...

bors · 2025-03-07T19:53:23Z

☀️ Try build successful - checks-actions
Build commit: 51b1964 (51b1964e35e5e26f8cd938940f0da6f7b5cc54fc)

compiler-errors · 2025-03-07T19:54:25Z

@craterbot check

craterbot · 2025-03-07T19:54:39Z

👌 Experiment pr-138176 created and queued.
🤖 Automatically detected try build 51b1964
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2025-03-07T19:55:33Z

🚧 Experiment pr-138176 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

rust-timer · 2025-03-07T23:38:49Z

Finished benchmarking commit (51b1964): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.1%, 0.4%]	6
Regressions ❌ (secondary)	0.4%	[0.2%, 0.9%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	0.3%	[0.1%, 0.4%]	6

Max RSS (memory usage)

Results (primary 1.0%, secondary -1.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.0%	[1.0%, 1.0%]	1
Regressions ❌ (secondary)	4.1%	[3.3%, 4.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.8%	[-6.1%, -2.7%]	5
All ❌✅ (primary)	1.0%	[1.0%, 1.0%]	1

Cycles

Results (primary 2.7%, secondary 1.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.7%	[2.7%, 2.7%]	1
Regressions ❌ (secondary)	2.6%	[1.4%, 3.9%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.5%	[-1.5%, -1.5%]	1
All ❌✅ (primary)	2.7%	[2.7%, 2.7%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 766.551s -> 766.651s (0.01%)
Artifact size: 362.09 MiB -> 362.13 MiB (0.01%)

craterbot · 2025-03-08T17:07:07Z

🎉 Experiment pr-138176 is completed!
📊 4 regressed and 2 fixed (593768 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

lcnr · 2025-03-10T15:00:42Z

compiler/rustc_trait_selection/src/traits/select/mod.rs

+            debug_assert_eq!(sized_candidates.next(), None);
+            return Some(SizedCandidate);
+        }
+


can you also try to remove the precedence for Builtin { nested: false }, i.e. exclusively prefer Sized. This would then match the new solver, if it causes breakage, we should instead hcange the list of "trivial builtin traits/impls"

i.e. exclusively prefer Sized

Do you mean stop preferring other trivial built-in impls, like Copy and Clone?

Yeah, let me re-crater that then.

feeling conflicted here. Preferring trivial builtin impls for traits with assoc types results in #133044. This also leaks whether impls are builtin or not. E.g. going from a builtin impl for fn-ptrs to an blanket impl for F: FnPtr is a subtle breaking change.

In general:

we prefer trivial builtin impls over where-bounds as the impl can never add any undesirable constraints. Where-bounds may add undesirable constraints, especially if the where-bound gets added by elaborating trait Sub: Trait.

if the builtin impl defines an associated type which references either regions or some param, we should use the where-bound as it's otherwise observable that we ignore the bound in case it's assoc type differs 🤔

if the builtin impl has nested obligations, using it may result in undesirable errors, e.g. if you check (T, T): Copy and the environment contains (T, T): Copy using the builtin impl for tuples causes this to error.

winnowing first evaluates all nested obligations of each candidate, so this is only an issue if proving the nested obligations is still ambig (due to inference vars instead of T), or if proving the nested obligations only cause unsatisfied region constraints

For Sized we don't need to worry about region constraints from the builtin impl, even if it has nested obligations as Sized impls never have region constraints not already required by well-formedness.

We do have the issue when proving Sized for types still involving infer vars, i.e. the following should error with this PR

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; }

proving W<?x>: Sized prefers the builtin impl over the where-bound as the nested obligations of that impl are still ambig. We then constrain ?x to T in which case proving the nested obligations fails.

Can you change the preference for Sized to bail with ambiguity if the self type still has non-region infer vars?

compiler-errors · 2025-03-10T15:17:26Z

@bors try

…try> Prefer built-in sized impls (and only sized impls) for rigid types always This PR changes the confirmation of `Sized` obligations to unconditionally prefer the built-in obligation, even if it has nested obligations. In the old solver, we register many builtin candidates with the `BuiltinCandidate { has_nested: bool }` candidate kind. The precedence this candidate takes over other candidates is based on the `has_nested` field. If it's false, then we prefer it over param-env candidates: https://github.com/rust-lang/rust/blob/2b4694a69804f89ff9d47d1a427f72c876f7f44c/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1804-L1815 Otherwise we prefer param-env candidates over it: https://github.com/rust-lang/rust/blob/2b4694a69804f89ff9d47d1a427f72c876f7f44c/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1847-L1866 I assume that the motivation for this behavior is to prefer built-in candidates if they have no nested obligations, but we can't as confidently prefer built-in candidates when they have where-clauses since we have no guarantee that the nested obligations for the built-in candidate are actually satisfyable (especially considering regions). However, for `Sized` candidates in particular, unlike the example above we never end up bottoming-out in a user-written impl, since *all* `Sized` impls are built-in. Thus, we don't have this problem, and preferring param-env candidates actually ends up leading to detrimental inference guidance, like: ```rust fn hello<T>() where (T,): Sized { let x: (_,) = Default::default(); // ^^ The `Sized` obligation on the variable infers `_ = T`. let x: (i32,) = x; // We error here, both a type mismatch and also b/c `T: Default` doesn't hold. } ``` Therefore this PR adjusts the candidate precedence of `Sized` obligations by making them a distinct candidate kind and unconditionally preferring them over all other candidate kinds. r? lcnr

bors · 2025-03-10T15:18:39Z

⌛ Trying commit ee3359a with merge 8a84d47...

compiler-errors · 2025-03-11T00:00:43Z

@bors try

lcnr · 2025-03-13T08:56:49Z

updated the PR description to be ready for an FCP, however, I do think there's still one way we ignore where-bounds with this change:

however, even with Sized this is still an issue if a nested goal also gets proven via a where-bound: playground

struct MyType<'a, T: ?Sized>(&'a (), T);
fn is_sized<T>() {}
fn foo<'a, T: ?Sized>()
where
    (MyType<'a, T>,): Sized,
    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>,)>();
}

@compiler-errors pls add this as a test, check my changes to the PR description and then start a types FCP :3

compiler-errors · 2025-03-13T18:56:52Z

Thanks for fleshing out the PR description @lcnr. I think this is a pretty desirable change personally and seems like lcnr does too, so let's start the FCP.

@rfcbot fcp merge

rfcbot · 2025-03-13T18:56:54Z

Team member @compiler-errors has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

compiler-errors · 2025-03-13T18:57:13Z

Let's get an updated perf too

@bors try @rust-timer queue

…try> Prefer built-in sized impls (and only sized impls) for rigid types always This PR changes the confirmation of `Sized` obligations to unconditionally prefer the built-in impl, even if it has nested obligations. This also changes all other built-in impls (namely, `Copy`/`Clone`/`DiscriminantKind`/`Pointee`) to *not* prefer built-in impls over param-env impls. This aligns the old solver with the behavior of the new solver. --- In the old solver, we register many builtin candidates with the `BuiltinCandidate { has_nested: bool }` candidate kind. The precedence this candidate takes over other candidates is based on the `has_nested` field. We only prefer builtin impls over param-env candidates if `has_nested` is `false` https://github.com/rust-lang/rust/blob/2b4694a69804f89ff9d47d1a427f72c876f7f44c/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1804-L1866 Preferring param-env candidates when the builtin candidate has nested obligations *still* ends up leading to detrimental inference guidance, like: ```rust fn hello<T>() where (T,): Sized { let x: (_,) = Default::default(); // ^^ The `Sized` obligation on the variable infers `_ = T`. let x: (i32,) = x; // We error here, both a type mismatch and also b/c `T: Default` doesn't hold. } ``` Therefore this PR adjusts the candidate precedence of `Sized` obligations by making them a distinct candidate kind and unconditionally preferring them over all other candidate kinds. Special-casing `Sized` this way is necessary as there are a lot of traits with a `Sized` super-trait bound, so a `&'a str: From<T>` where-bound results in an elaborated `&'a str: Sized` bound. People tend to not add explicit where-clauses which overlap with builtin impls, so this tends to not be an issue for other traits. We don't know of any tests/crates which need preference for other builtin traits. As this causes builtin impls to diverge from user-written impls we would like to minimize the affected traits. Otherwise e.g. moving impls for tuples to std by using variadic generics would be a breaking change. For other builtin impls it's also easier for the preference of builtin impls over where-bounds to result in issues. --- There are two ways preferring builtin impls over where-bounds can be incorrect and undesirable: - applying the builtin impl results in undesirable region constraints. E.g. if only `MyType<'static>` implements `Copy` then a goal like `(MyType<'a>,): Copy` would require `'a == 'static` so we must not prefer it over a `(MyType<'a>,): Copy` where-bound - this is mostly not an issue for `Sized` as all `Sized` impls are builtin and don't add any region constraints not already required for the type to be well-formed - however, even with `Sized` this is still an issue if a nested goal also gets proven via a where-bound: [playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=30377da5b8a88f654884ab4ebc72f52b) - if the builtin impl has associated types, we should not prefer it over where-bounds when normalizing that associated type. This can result in normalization adding more region constraints than just proving trait bounds. rust-lang#133044 - not an issue for `Sized` as it doesn't have associated types. r? lcnr

bors · 2025-03-13T18:58:26Z

⌛ Trying commit 37235a2 with merge 7d2c602...

jackh726 · 2025-03-13T20:52:43Z

Overall seems reasonable, but I would expect the tests in rust-lang/trait-system-refactor-initiative#162 to be included in this PR...

bors · 2025-03-13T20:59:15Z

☀️ Try build successful - checks-actions
Build commit: 7d2c602 (7d2c602797febca4587c17dc6a794e2daf5de473)

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

compiler-errors · 2025-03-13T21:18:02Z

Tests from rust-lang/trait-system-refactor-initiative#162 added.

compiler-errors · 2025-03-13T21:18:37Z

tests/ui/traits/incomplete-infer-via-sized-wc.rs

+    is_sized::<MaybeSized<_>>();
+    //~^ ERROR type annotations needed


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.

rust-timer · 2025-03-14T01:09:42Z

Finished benchmarking commit (7d2c602): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -3.8%, secondary -3.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.8%	[-4.4%, -3.1%]	2
Improvements ✅ (secondary)	-3.3%	[-3.3%, -3.3%]	1
All ❌✅ (primary)	-3.8%	[-4.4%, -3.1%]	2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 775.02s -> 775.85s (0.11%)
Artifact size: 365.09 MiB -> 365.19 MiB (0.03%)

rfcbot · 2025-03-18T17:33:55Z

🔔 This is now entering its final comment period, as per the review above. 🔔

rustbot assigned lcnr Mar 7, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 7, 2025

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 7, 2025

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025

Auto merge of rust-lang#138176 - compiler-errors:rigid-sized-obl, r=<…

51b1964

…try> Prefer built-in sized obligations for rigid types always r? lcnr

This comment has been minimized.

Sign in to view

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Mar 7, 2025

rustbot added the perf-regression Performance regression. label Mar 7, 2025

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Mar 8, 2025

lcnr reviewed Mar 10, 2025

View reviewed changes

compiler-errors force-pushed the rigid-sized-obl branch 2 times, most recently from a1c78bb to ee3359a Compare March 10, 2025 15:17

compiler-errors changed the title ~~Prefer built-in sized obligations for rigid types always~~ Prefer built-in sized impls (and only sized impls) for rigid types always Mar 10, 2025

compiler-errors force-pushed the rigid-sized-obl branch from ee3359a to 08d8f3a Compare March 10, 2025 16:50

lcnr mentioned this pull request Mar 13, 2025

hopefully irrelevant candidate behavior difference of Sized/trivial builtin impls rust-lang/trait-system-refactor-initiative#162

Open

compiler-errors added T-types Relevant to the types team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 13, 2025

compiler-errors force-pushed the rigid-sized-obl branch from 150ca15 to 37235a2 Compare March 13, 2025 18:56

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 13, 2025

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 13, 2025

This comment has been minimized.

Sign in to view

oli-obk reviewed Mar 13, 2025

View reviewed changes

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs Show resolved Hide resolved

compiler-errors added 3 commits March 13, 2025 21:12

Prefer built-in sized impls for rigid types always

f9696dd

Only prefer Sized candidates, and only if they certainly hold

aebbd42

Flesh out tests

c0230f4

compiler-errors force-pushed the rigid-sized-obl branch from 37235a2 to c0230f4 Compare March 13, 2025 21:17

compiler-errors commented Mar 13, 2025

View reviewed changes

rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Mar 14, 2025

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 18, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

compiler-errors commented Mar 7, 2025 •

edited by lcnr

Loading

compiler-errors commented Mar 7, 2025

This comment has been minimized.

bors commented Mar 7, 2025

bors commented Mar 7, 2025

This comment has been minimized.

compiler-errors commented Mar 7, 2025

craterbot commented Mar 7, 2025

craterbot commented Mar 7, 2025

rust-timer commented Mar 7, 2025

craterbot commented Mar 8, 2025

lcnr Mar 10, 2025

compiler-errors Mar 10, 2025

compiler-errors Mar 10, 2025

lcnr Mar 10, 2025 •

edited

Loading

compiler-errors commented Mar 10, 2025

bors commented Mar 10, 2025

compiler-errors commented Mar 11, 2025

lcnr commented Mar 13, 2025

compiler-errors commented Mar 13, 2025

rfcbot commented Mar 13, 2025 •

edited by spastorino

Loading

compiler-errors commented Mar 13, 2025

This comment has been minimized.

bors commented Mar 13, 2025

jackh726 commented Mar 13, 2025

bors commented Mar 13, 2025

This comment has been minimized.

compiler-errors commented Mar 13, 2025

compiler-errors Mar 13, 2025

rust-timer commented Mar 14, 2025

rfcbot commented Mar 18, 2025

	// 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 });
	}

	// Before we consider where-bounds, we have to deduplicate them here and also
	// drop where-bounds in case the same where-bound exists without bound vars.
	// This is necessary as elaborating super-trait bounds may result in duplicates.
	'search_victim: loop {
	for (i, this) in candidates.iter().enumerate() {
	let ParamCandidate(this) = this.candidate else { continue };
	for (j, other) in candidates.iter().enumerate() {
	if i == j {
	continue;
	}

	let ParamCandidate(other) = other.candidate else { continue };
	if this == other {
	candidates.remove(j);
	continue 'search_victim;
	}

	if this.skip_binder().trait_ref == other.skip_binder().trait_ref
	&& this.skip_binder().polarity == other.skip_binder().polarity
	&& !this.skip_binder().trait_ref.has_escaping_bound_vars()
	{
	candidates.remove(j);
	continue 'search_victim;
	}
	}
	}

	break;
	}

	// The next highest priority is for non-global where-bounds. However, while we don't
	// prefer global where-clauses here, we do bail with ambiguity when encountering both
	// a global and a non-global where-clause.
	//
	// Our handling of where-bounds is generally fairly messy but necessary for backwards
	// compatibility, see #50825 for why we need to handle global where-bounds like this.
	let is_global = \|c: ty::PolyTraitPredicate<'tcx>\| c.is_global() && !c.has_bound_vars();
	let param_candidates = candidates
	.iter()
	.filter_map(\|c\| if let ParamCandidate(p) = c.candidate { Some(p) } else { None });
	let mut has_global_bounds = false;
	let mut param_candidate = None;
	for c in param_candidates {
	if is_global(c) {
	has_global_bounds = true;
	} else if param_candidate.replace(c).is_some() {
	// Ambiguity, two potentially different where-clauses
	return None;
	}
	}

		is_sized::<MaybeSized<_>>();
		//~^ ERROR type annotations needed

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

Are you sure you want to change the base?

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

Conversation

compiler-errors commented Mar 7, 2025 • edited by lcnr Loading

compiler-errors commented Mar 7, 2025

This comment has been minimized.

bors commented Mar 7, 2025

bors commented Mar 7, 2025

This comment has been minimized.

compiler-errors commented Mar 7, 2025

craterbot commented Mar 7, 2025

craterbot commented Mar 7, 2025

rust-timer commented Mar 7, 2025

Overall result: ❌✅ regressions and improvements - please read the text below

craterbot commented Mar 8, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcnr Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

compiler-errors commented Mar 10, 2025

bors commented Mar 10, 2025

compiler-errors commented Mar 11, 2025

lcnr commented Mar 13, 2025

compiler-errors commented Mar 13, 2025

rfcbot commented Mar 13, 2025 • edited by spastorino Loading

compiler-errors commented Mar 13, 2025

This comment has been minimized.

bors commented Mar 13, 2025

jackh726 commented Mar 13, 2025

bors commented Mar 13, 2025

This comment has been minimized.

compiler-errors commented Mar 13, 2025

Choose a reason for hiding this comment

rust-timer commented Mar 14, 2025

Overall result: no relevant changes - no action needed

rfcbot commented Mar 18, 2025

compiler-errors commented Mar 7, 2025 •

edited by lcnr

Loading

lcnr Mar 10, 2025 •

edited

Loading

rfcbot commented Mar 13, 2025 •

edited by spastorino

Loading