Don't require associated types with Self: Sized bounds in dyn Trait objects

[oli-obk]

Trait objects require all associated types to be specified, even if the associated type has an explicit where Self: Sized bound. The following snippet does not compile on master, but does with this PR.

fn _assert_is_object_safe(_: &dyn Foo) {}

pub trait Foo {
    type Bar where Self: Sized;
}

In contrast, if a Self: Sized bound is added to a method, the methodjust isn't callable on trait objects, but the trait can be made object safe just fine.

fn _assert_is_object_safe(_: &dyn Foo) {}

pub trait Foo {
    fn foo() where Self: Sized;
}

This PR closes this inconsistency (though it still exists for associated constants).

Additionally this PR adds a new lint that informs users they can remove associated type bounds from their trait objects if those associated type bounds have a where Self: Sized bound, and are thus useless.

r? @compiler-errors

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @compiler-errors (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[oli-obk]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 661b143ebaaee94d37cf44bd71f78007f2caf799 with merge 90c098e28583990068dbac5e390c33c77a2578e5...

[bors]

☀️ Try build successful - checks-actions
Build commit: 90c098e28583990068dbac5e390c33c77a2578e5 (90c098e28583990068dbac5e390c33c77a2578e5)

[rust-timer]

Finished benchmarking commit (90c098e28583990068dbac5e390c33c77a2578e5): comparison URL.

Overall result: ❌ regressions - 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 is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.5%	[0.4%, 1.0%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

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)	4.0%	[3.0%, 5.0%]	2
Improvements ✅ (primary)	-1.1%	[-1.4%, -0.8%]	4
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.1%	[-1.4%, -0.8%]	4

Cycles

Results

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)	2.2%	[2.2%, 2.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 646.24s -> 646.543s (0.05%)

[compiler-errors]

@rustbot author

[oli-obk]

These get deduplicated for users

[oli-obk]

@rfcbot fcp merge

I am starting this FCP for both T-types and T-lang. While I believe closing such gaps is entirely in the purview of T-types, considering that I want to close further such gaps, I would like T-lang to chime in to say whether they agree with my assessment and are fine with just getting pinged in the future, but not included in the FCPs.

The summary of what is being stabilized here is all documented in the main post.

[rfcbot]

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

• @BoxyUwU
• @aliemjay
• @compiler-errors
• @jackh726
• @joshtriplett
• @lcnr
• @nikomatsakis
• @oli-obk
• @pnkfelix
• @scottmcm
• @spastorino
• @tmandry

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!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[rust-timer]

Finished benchmarking commit (99f7d36): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

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)	3.3%	[3.3%, 3.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.0%	[-3.4%, -2.6%]	2
All ❌✅ (primary)	-	-	0

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: 655.25s -> 655.506s (0.04%)

[QuineDot]

Specifying non-dyn-usable associated types is not 100% useless (see issue #125660.) For example...

One area I could see this getting slightly weird is around "bridging" impls like impl Foo for &dyn Foo. In that case you still want to use ! or its equivalent and have methods panic in some cases. But that's a sign that you probably shouldn't be writing such an impl (for these traits) in the first place, and the situation already exists for where Self: Sized methods.

...you could impl Foo { Assoc = A; } for &dyn Foo<Assoc = A>, when there's a sensible way to do so.

I think that ideally long term you would be allowed to not implement items that have where clauses that are known to not hold

There's an accepted RFC going the other direction (allowing where clauses that never hold in more cases).