Avoid leaking opaque hidden types via auto trait candidates

[bit-aloo]

View all comments

closes: #134578

[rustbot]

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 74 candidates
• Random selection from 16 candidates

[bit-aloo]

r? @lcnr

[rustbot]

Some changes occurred to the core trait solver

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

[lcnr]

please move this function into trait_goals.rs 🤔

it is only used from there and it doesn't feel "purely structural" enough to belong in this file

View changes since the review

[lcnr]

Suggested change

	let ty::Alias(_, ty::AliasTy { kind: ty::Opaque { def_id, .. }, .. }) =
	let ty::Alias(ty::Rigid::Yes, ty::AliasTy { kind: ty::Opaque { def_id, .. }, .. }) =

View changes since the review

[lcnr]

why?

View changes since the review

[bit-aloo]

If we remove this we have two failing test:

[ui] tests/ui/traits/error-reporting/leaking-vars-in-cause-code-1.rs
[ui] tests/ui/traits/next-solver/auto-with-drop_tracking_mir.rs#fail

For auto-with-drop_tracking_mir.rs, we lose the cause and .await location:

@@ -1,29 +1,12 @@
-error[E0277]: `dyn AsyncFn<Fut = Pin<Box<dyn Future<Output = ()> + Send>>>` cannot be shared between threads safely
+error[E0277]: `impl Future<Output = ()>` cannot be sent between threads safely
   --> $DIR/leaking-vars-in-cause-code-1.rs:32:17
    |
 LL |     assert_send(cursed_fut());
-   |     ----------- ^^^^^^^^^^^^ `dyn AsyncFn<Fut = Pin<Box<dyn Future<Output = ()> + Send>>>` cannot be shared between threads safely
+   |     ----------- ^^^^^^^^^^^^ `impl Future<Output = ()>` cannot be sent between threads safely
    |     |
    |     required by a bound introduced by this call
    |
-   = help: the trait `Sync` is not implemented for `dyn AsyncFn<Fut = Pin<Box<dyn Future<Output = ()> + Send>>>`
-   = note: required for `&dyn AsyncFn<Fut = Pin<Box<dyn Future<Output = ()> + Send>>>` to implement `Send`
-note: required because it's used within this `async` fn body
-  --> $DIR/leaking-vars-in-cause-code-1.rs:19:53
-   |
-LL |   async fn wrap_call<P: AsyncFn + ?Sized>(filter: &P) {
-   |  _____________________________________________________^
-LL | |     filter.call().await;
-LL | | }
-   | |_^
-note: required because it's used within this `async` fn body
-  --> $DIR/leaking-vars-in-cause-code-1.rs:27:23
-   |
-LL |   async fn cursed_fut() {
-   |  _______________________^
-LL | |     wrap_call(get_boxed_fn().as_ref()).await;
-LL | | }
-   | |_^
+   = help: the trait `Send` is not implemented for `impl Future<Output = ()>`
 note: required by a bound in `assert_send`
   --> $DIR/leaking-vars-in-cause-code-1.rs:36:19
    |

For leaking-vars-in-cause-code-1.rs, we also lose the nested async-function context:

@@ -1,21 +1,12 @@
-error: future cannot be sent between threads safely
+error[E0277]: `impl Future<Output = ()>` cannot be sent between threads safely
   --> $DIR/auto-with-drop_tracking_mir.rs:25:13
    |
 LL |     is_send(foo());
-   |             ^^^^^ future returned by `foo` is not `Send`
+   |     ------- ^^^^^ `impl Future<Output = ()>` cannot be sent between threads safely
+   |     |
+   |     required by a bound introduced by this call
    |
-help: the trait `Sync` is not implemented for `NotSync`
-  --> $DIR/auto-with-drop_tracking_mir.rs:8:1
-   |
-LL | struct NotSync;
-   | ^^^^^^^^^^^^^^
-note: future is not `Send` as this value is used across an await
-  --> $DIR/auto-with-drop_tracking_mir.rs:16:11
-   |
-LL |     let x = &NotSync;
-   |         - has type `&NotSync` which is not `Send`
-LL |     bar().await;
-   |           ^^^^^ await occurs here, with `x` maybe used later
+   = help: the trait `Send` is not implemented for `impl Future<Output = ()>`
 note: required by a bound in `is_send`
   --> $DIR/auto-with-drop_tracking_mir.rs:24:24
    |
@@ -24,3 +15,4 @@
 
 error: aborting due to 1 previous error
 
+For more information about this error, try `rustc --explain E0277`.

It seems like we should continue diagnostic traversal for AsyncFn; otherwise, the resulting diagnostic loses most of its useful context. So I’m not sure we should remove this exception. 🤔 Wanna vibe check on what you think?

[lcnr]

I guess we should actually just leak foreign types here for diagnostics? Given that the reason we stopped the proof tree visitor here was to avoid leaking non-local closures etc, leaking them for async functions means we should leak em everywhere and just support them in the diagnostics code?

[bit-aloo]

@lcnr this one can get a look, almost to finish..

[lcnr]

@rustbot authot