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

diagnostic for naive recursive async functions is really unfriendly #62539

Open
nikomatsakis opened this issue Jul 9, 2019 · 5 comments

Comments

Projects
None yet
3 participants
@nikomatsakis
Copy link
Contributor

commented Jul 9, 2019

The following Rust source (playground):

#![feature(async_await)]

async fn foo(n: usize) {
    if n > 0 {
        foo(n - 1).await;
    }
}

fn main() {
}

gives the error:

error[E0720]: opaque type expands to a recursive type
 --> src/main.rs:3:24
  |
3 | async fn foo(n: usize) {
  |                        ^ expands to self-referential type
  |
  = note: expanded type is `std::future::GenFuture<[static generator@src/main.rs:3:24: 7:2 n:usize {usize, bool, impl std::future::Future, ()}]>`

We could be a lot clearer.

@gilescope

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

@rustbot claim

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

I think an error like

error[E0720]: recursion is not supported in async fn
 --> src/main.rs:3:24
  |
3 | async fn foo(n: usize) {
  |                        ^ async fn cannot invoke themselves directly
  |
  = note: to create a recursive async fn, you must rewrite to return a boxed future
  = for more information, see https://rust-lang.github.io/async-book/index.html
@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

I suspect we can intercept that "opaque type" error and check on the origin field of the opaque type, looking for AsyncFn

@withoutboats

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

In addition to adding specific output for async fn, it would also be good to stop outputing "self-referential type" in any context, and instead use "recursive type" as this error message does at one point. Nowadays, "self-referential" is used very often to mean they have references pointing into themselves, rather than how it is being used here, and someone who googles it will get very unhelpful results. I imagine this fix would just be a search and replace.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

It looks like the error is issued here:

fn check_opaque<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, substs: SubstsRef<'tcx>, span: Span) {
if let Err(partially_expanded_type) = tcx.try_expand_impl_trait_type(def_id, substs) {
let mut err = struct_span_err!(
tcx.sess, span, E0720,
"opaque type expands to a recursive type",
);
err.span_label(span, "expands to self-referential type");
if let ty::Opaque(..) = partially_expanded_type.sty {
err.note("type resolves to itself");
} else {
err.note(&format!("expanded type is `{}`", partially_expanded_type));
}
err.emit();
}
}

That function is invoked from here:

hir::ItemKind::Existential(..) => {
let def_id = tcx.hir().local_def_id(it.hir_id);
let substs = InternalSubsts::identity_for_item(tcx, def_id);
check_opaque(tcx, def_id, substs, it.span);
}

In that caller's location, we can access the argument to the Existential variant, which is a ExistTy struct. That struct has a field origin that tell us where the existential type came from. If we pass that field into the original function I cited, it can check if this is an async function and alter the error message wording.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jul 18, 2019

Rollup merge of rust-lang#62777 - gilescope:self-referencial-to-recur…
…sion, r=eddyb

Self-referencial type now called a recursive type

As per Boat's suggestion - rust-lang#62539, this makes the error message clearer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.