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

Confusing error message with panic!() and impl trait #66523

Closed
Aaron1011 opened this issue Nov 18, 2019 · 9 comments
Closed

Confusing error message with panic!() and impl trait #66523

Aaron1011 opened this issue Nov 18, 2019 · 9 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Aaron1011
Copy link
Member

Aaron1011 commented Nov 18, 2019

The following code:

fn bar() -> Vec<impl Copy> {
    panic!()
}

Currently gives the following error:

error[E0283]: type annotations required: cannot resolve `_: std::marker::Copy`
 --> src/lib.rs:1:17
  |
1 | fn bar() -> Vec<impl Copy> {
  |                 ^^^^^^^^^
  |
  = note: the return type of a function must have a statically known size

This message has a weird span, and doesn't give any clue that the problem lies with the call to panic!

@JohnTitor JohnTitor added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 18, 2019
@jonas-schievink jonas-schievink added the A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. label Nov 18, 2019
@Centril
Copy link
Contributor

Centril commented Nov 19, 2019

(Sidenote: "cannot resolve" trips me up every time since it feels like it talking about name resolution, not type checking)

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jan 17, 2020
Account for common `impl Trait`/`dyn Trait` return type errors

- When all return paths have the same type, suggest `impl Trait`.
- When all return paths implement the expected `trait`, suggest `Box<dyn Trait>` and mention using an `enum`.
- When multiple different types are returned and `impl Trait` is expected, extend the explanation.
- When return type is `impl Trait` and the return paths do not implement `Trait`, point at the returned values.
- Split `src/librustc/traits/error_reporting.rs` into multiple files to keep size under control.

Fix rust-lang#68110, cc rust-lang#66523.
@estebank
Copy link
Contributor

estebank commented Feb 4, 2020

Current output is a slight regression introduced by #68195:

error[E0720]: opaque type expands to a recursive type
 --> src/lib.rs:1:17
  |
1 | fn bar() -> Vec<impl Copy> {
  |                 ^^^^^^^^^ expands to a recursive type
  |
  = note: type resolves to itself

@estebank
Copy link
Contributor

estebank commented Feb 4, 2020

(Sidenote: "cannot resolve" trips me up every time since it feels like it talking about name resolution, not type checking)

Would "cannot fulfill _: std::marker::Copy" be a better wording?

@Centril
Copy link
Contributor

Centril commented Feb 4, 2020

@estebank Maybe "cannot satisfy _: Copy" (we use "satisfy" more commonly I think)?

@mcgoo
Copy link

mcgoo commented Mar 10, 2020

Same issue I think but with a different diagnostic: the trait bound '(): std::future::Future' is not satisfied

fn foo() -> impl std::future::Future<Output = i32> {
    unimplemented!();
}

gives

error[E0277]: the trait bound `(): std::future::Future` is not satisfied
 --> src/lib.rs:1:13
  |
1 | fn foo() -> impl std::future::Future<Output = i32> {
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::future::Future` is not implemented for `()`
2 |     unimplemented!();
  |     ----------------- consider removing this semicolon
  |
  = note: the return type of a function must have a statically known size
  = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

I expected it would do the same with async fn foo() -> i32 { panic!(); } but it works fine.

Centril added a commit to Centril/rust that referenced this issue Apr 5, 2020
@nrxus
Copy link

nrxus commented Apr 7, 2020

I just recently hit this, and I don't believe that it is related to the panic:

struct MarkerHolder<T> {
    _marker: std::marker::PhantomData<*const T>
}

fn make_marker() -> MarkerHolder<impl std::fmt::Display> {
    MarkerHolder {
        _marker: std::marker::PhantomData,
    }
}

fails with:

  |
5 | fn make_marker() -> MarkerHolder<impl std::fmt::Display> {
  |                                  ^^^^^^^^^^^^^^^^^^^^^^ expands to a recursive type
  |
  = note: type resolves to itself

Could someone help me understand if this error is intentional just worded weirdly (I don't see how my return type is recursive), or if this kind of code should compile?

EDIT: On second thought, I can totally see why it errors, the compiler has no way to know what type to assign to the opaque type. I still think that saying it is a recursive type is misleading though.

@estebank estebank self-assigned this Apr 19, 2020
estebank added a commit to estebank/rust that referenced this issue Jun 15, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Jun 16, 2020
…komatsakis

Expand "recursive opaque type" diagnostic

Fix rust-lang#70968, partially address rust-lang#66523.
tmandry added a commit to tmandry/rust that referenced this issue Jun 17, 2020
…komatsakis

Expand "recursive opaque type" diagnostic

Fix rust-lang#70968, partially address rust-lang#66523.
RalfJung added a commit to RalfJung/rust that referenced this issue Jun 18, 2020
…komatsakis

Expand "recursive opaque type" diagnostic

Fix rust-lang#70968, partially address rust-lang#66523.
Manishearth added a commit to Manishearth/rust that referenced this issue Jun 18, 2020
…komatsakis

Expand "recursive opaque type" diagnostic

Fix rust-lang#70968, partially address rust-lang#66523.
Manishearth added a commit to Manishearth/rust that referenced this issue Jun 18, 2020
…komatsakis

Expand "recursive opaque type" diagnostic

Fix rust-lang#70968, partially address rust-lang#66523.
@estebank
Copy link
Contributor

Current output:

error[E0720]: cannot resolve opaque type
 --> src/lib.rs:1:17
  |
1 | fn bar() -> Vec<impl Copy> {
  |                 ^^^^^^^^^ cannot resolve opaque type
2 |     panic!()
  |     -------- this returned value is of `!` type
  |
  = help: this error will resolve once the item's body returns a concrete type

error[E0277]: `()` is not a future
 --> src/lib.rs:1:13
  |
1 | fn foo() -> impl std::future::Future<Output = i32> {
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `()` is not a future
2 |     unimplemented!();
  |     ----------------- consider removing this semicolon
  |
  = help: the trait `Future` is not implemented for `()`

error[E0720]: cannot resolve opaque type
 --> src/lib.rs:5:34
  |
5 |   fn make_marker() -> MarkerHolder<impl std::fmt::Display> {
  |                                    ^^^^^^^^^^^^^^^^^^^^^^ recursive opaque type
6 | /     MarkerHolder {
7 | |         _marker: std::marker::PhantomData,
8 | |     }
  | |_____- returning here with type `MarkerHolder<impl std::fmt::Display>`

@jyn514
Copy link
Member

jyn514 commented Jul 7, 2021

Triage: the current output is

error[E0720]: cannot resolve opaque type
 --> src/lib.rs:1:17
  |
1 | fn bar() -> Vec<impl Copy> {
  |                 ^^^^^^^^^ cannot resolve opaque type
2 |     panic!()
  |     -------- this returned value is of `!` type
  |
  = help: this error will resolve once the item's body returns a concrete type

Which seems pretty good to me - should this be closed as a duplicate of #69882 ?

@estebank
Copy link
Contributor

The only thing I'd change it so change "item's body" with "bar's body" or "the function's body", but otherwise this is good enough to close the ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants