Consider principal trait ref's auto-trait super-traits in dyn upcasting

[compiler-errors]

Given traits like:

trait Subtrait: Supertrait + Send {}
trait Supertrait {}

We should be able to upcast dyn Subtrait to dyn Supertrait + Send. This is not currently possible, because when upcasting, we look at the list of auto traits in the object type (dyn Subtrait, which has no auto traits in its bounds) and compare them to the target's auto traits (dyn Supertrait + Send, which has Send in its bound).

Since the target has auto traits that are not present in the source, the upcasting fails. This is overly restrictive, since dyn Subtrait will always implement Send via its built-in object impl. I propose to loosen this restriction here.

r? types

---

Aside: Fix this in astconv instead?

edit: This causes too many failures. See #119825 (comment)

We may also fix this by by automatically elaborating all auto-trait supertraits during AstConv::conv_object_ty_poly_trait_ref. That is, we can make it so that dyn Subtrait is elaborated into the same type of dyn Subtrait + Send.

I'm open to considering this solution instead, but it would break coherence in the following example:

trait Foo: Send {}

trait Bar {}
impl Bar for dyn Foo {}
impl Bar for dyn Foo + Send {}
//~^ This would begin to be an overlapping impl.

[rustbot]

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

[compiler-errors]

This check is redundant.

[compiler-errors]

Please review this code with whitespace disabled.

[compiler-errors]

This auto_traits_compatible check is only necessary for the principal_def_id_a == principal_def_id_b case. The match_upcast_principal function checks auto traits itself.

[compiler-errors]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 6cfde57 with merge 82115ba...

[bors]

☀️ Try build successful - checks-actions
Build commit: 82115ba (82115bac2b802bf307220d2d14b5d970ac833e4f)

[rust-timer]

Finished benchmarking commit (82115ba): 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

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

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

Binary size

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

Bootstrap: 671.325s -> 671.846s (0.08%)
Artifact size: 312.40 MiB -> 312.39 MiB (-0.00%)

[RalfJung]

Could you add a similar test to Miri? (src/tools/miri/tests/pass)

[Jules-Bertholet]

We may also fix this by by automatically elaborating all auto-trait supertraits during AstConv::conv_object_ty_poly_trait_ref. That is, we can make it so that dyn Subtrait is elaborated into the same type of dyn Subtrait + Send.

This seems like a no-brainer, assuming that crater is happy? It would be consistent with the other normalizations that dyn does (dyn Foo + Send = dyn Send + Foo = dyn Foo + Send + Send = trait Bar = Foo + Send; dyn Bar).

[lcnr]

r? @lcnr

[lcnr]

lgtm after adding a run-pass test (and miri), which is what @RalfJung is asking for.

I am personally slightly in favor of elaborating super traits eagerly and (maybe) linting if they are explicitly mentioned even though they are implied. Would you mind implementing that and running crater?

I guess that's a t-lang question?

[compiler-errors]

OK, I guess I'd also rather just elaborate the super-auto-traits eagerly. That's not hard to impl.

[rfcbot]

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

[bors]

☔ The latest upstream changes (presumably #120862) made this pull request unmergeable. Please resolve the merge conflicts.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[compiler-errors]

rebased

@bors r=lcnr

[bors]

📌 Commit ec8e898 has been approved by lcnr

It is now in the queue for this repository.

[bors]

⌛ Testing commit ec8e898 with merge b656f51...

[bors]

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing b656f51 to master...

[rust-timer]

Finished benchmarking commit (b656f51): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -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)	7.7%	[7.7%, 7.7%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

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

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)	5.1%	[5.1%, 5.1%]	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: 636.031s -> 639.504s (0.55%)
Artifact size: 306.32 MiB -> 306.30 MiB (-0.01%)