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

Poor Trailing Semicolon Error in -> impl Trait Function #54771

Open
cramertj opened this issue Oct 2, 2018 · 9 comments
Open

Poor Trailing Semicolon Error in -> impl Trait Function #54771

cramertj opened this issue Oct 2, 2018 · 9 comments
Labels
A-closures Area: closures (`|args| { .. }`) A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cramertj
Copy link
Member

cramertj commented Oct 2, 2018

The diagnostics that check whether the concrete return type of an -> impl Trait function meets the : Trait bound should special-case the error when the concrete type is () to suggest removing the semicolon. Without this, the errors for a misplaced semicolon are misleading:

Without impl Trait:

trait Bar {}
impl Bar for u8 {}
fn foo() -> u8 {
    5;
}
error[E0308]: mismatched types
 --> src/main.rs:3:16
  |
3 |   fn foo() -> u8 {
  |  ________________^
4 | |     5;
  | |      - help: consider removing this semicolon
5 | | }
  | |_^ expected u8, found ()
  |
  = note: expected type `u8`
             found type `()`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: Could not compile `playground`.

With impl Trait:

trait Bar {}
impl Bar for u8 {}
fn foo() -> impl Bar {
    5;
}
error[E0277]: the trait bound `(): Bar` is not satisfied
 --> src/main.rs:3:13
  |
3 | fn foo() -> impl Bar {
  |             ^^^^^^^^ the trait `Bar` is not implemented for `()`
  |
  = note: the return type of a function must have a statically known size

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
error: Could not compile `playground`.
@cramertj
Copy link
Member Author

cramertj commented Oct 2, 2018

Note: there's a similar issue in closure return types:

trait Bar {}
impl Bar for u8 {}

fn bar<R: Bar>(_: impl Fn() -> R) {}
fn main() {
    bar(|| { 5u8; })
}

gives

error[E0277]: the trait bound `(): Bar` is not satisfied
 --> src/main.rs:6:5
  |
6 |     bar(|| { 5u8; })
  |     ^^^ the trait `Bar` is not implemented for `()`
  |
note: required by `bar`
 --> src/main.rs:4:1
  |
4 | fn bar<R: Bar>(_: impl Fn() -> R) {}
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. A-closures Area: closures (`|args| { .. }`) labels Oct 3, 2018
@csmoe
Copy link
Member

csmoe commented Oct 3, 2018

the impl-Trait <: T subtyping seems fail here:

if self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err() {
return;
}


DEBUG LOG:

DEBUG 2018-10-03T09:41:07Z: rustc_typeck::check: >> typechecking: expr=expr(21: { 1u8; }) expected=ExpectHasType(_)
DEBUG 2018-10-03T09:41:07Z: rustc_typeck::check: >> typechecking: expr=expr(16: 1u8) expected=NoExpectation
DEBUG 2018-10-03T09:41:07Z: rustc_typeck::check: write_ty(HirId { owner: DefIndex(0:5), local_id: ItemLocalId(1) }, u8) in fcx 0x7ffef324d930
DEBUG 2018-10-03T09:41:07Z: rustc_typeck::check: type of expr 1u8 (id=16) is...
DEBUG 2018-10-03T09:41:07Z: rustc_typeck::check: ... u8, expected is NoExpectation
/// STOP!!!
DEBUG 2018-10-03T09:41:07Z: rustc_typeck::check: write_ty(HirId { owner: DefIndex(0:5), local_id: ItemLocalId(3) }, ()) in fcx 0x7ffef324d930
DEBUG 2018-10-03T09:41:07Z: rustc_typeck::check: write_ty(HirId { owner: DefIndex(0:5), local_id: ItemLocalId(4) }, ()) in fcx 0x7ffef324d930
DEBUG 2018-10-03T09:41:07Z: rustc_typeck::check: type of expr { 1u8; } (id=21) is...
DEBUG 2018-10-03T09:41:07Z: rustc_typeck::check: ... (), expected is ExpectHasType(_)
D

as it shows the checking should move to lint_remove_semi before the 2nd time of calling write_ty , or then expr { 1u8; } will be "parsed" as (), that's why we get (): Bar is not satisfied.

@estebank
Copy link
Contributor

estebank commented Feb 5, 2019

there's a similar issue in closure return types

@cramertj closure output is already problematic for the simple case there too:

error[E0308]: mismatched types
 --> src/main.rs:6:12
  |
6 |     bar(|| { 5u8; })
  |            ^^^^^^^^ expected u32, found ()
  |
  = note: expected type `u32`
             found type `()`

Disregard this, we actually do the right thing for this case:

error[E0308]: mismatched types
 --> file.rs:3:12
  |
3 |     bar(|| { 5; })
  |            ^^^-^^
  |            |  |
  |            |  help: consider removing this semicolon
  |            expected u8, found ()
  |
  = note: expected type `u8`
             found type `()`

Centril added a commit to Centril/rust that referenced this issue Feb 13, 2019
On return type `impl Trait` for block with no expr point at last semi

Partial solution, doesn't actually validate that the last statement in the function body can satisfy the trait bound, but it's a good incremental improvement over the status quo.

```
error[E0277]: the trait bound `(): Bar` is not satisfied
  --> $DIR/impl-trait-return-trailing-semicolon.rs:3:13
   |
LL | fn foo() -> impl Bar {
   |             ^^^^^^^^ the trait `Bar` is not implemented for `()`
LL |     5;
   |      - consider removing this semicolon
   |
   = note: the return type of a function must have a statically known size
```

Partially addresses rust-lang#54771.
pietroalbini added a commit to pietroalbini/rust that referenced this issue Mar 8, 2019
On return type `impl Trait` for block with no expr point at last semi

Partial solution, doesn't actually validate that the last statement in the function body can satisfy the trait bound, but it's a good incremental improvement over the status quo.

```
error[E0277]: the trait bound `(): Bar` is not satisfied
  --> $DIR/impl-trait-return-trailing-semicolon.rs:3:13
   |
LL | fn foo() -> impl Bar {
   |             ^^^^^^^^ the trait `Bar` is not implemented for `()`
LL |     5;
   |      - consider removing this semicolon
   |
   = note: the return type of a function must have a statically known size
```

Partially addresses rust-lang#54771.
@estebank
Copy link
Contributor

Current output:

error[E0277]: the trait bound `(): Bar` is not satisfied
 --> src/lib.rs:3:13
  |
3 | fn foo() -> impl Bar {
  |             ^^^^^^^^ the trait `Bar` is not implemented for `()`
4 |     5;
  |      - consider removing this semicolon
  |
  = note: the return type of a function must have a statically known size

It still needs checking that the last statement's expr can actually conform to the trait, but the naïve behavior is there.

@estebank estebank added E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. P-medium Medium priority A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. labels Jun 11, 2019
@edwardw
Copy link
Contributor

edwardw commented Aug 20, 2019

This opaque the trait bound ... is not satisfied error message creeps up in other places too. E.g., this stackoverflow question regarding futures::stream::Stream::for_each. The more prevalent std::iter::Iterator::for_each expects a closure of unit type, but this one expects a closure that returns something. rustc ends up emitting mysterious error messages and the error also propagates to the next function call in the chain.

Ideally, the error message in this case should be something like:

...
socket
    .for_each(|message| {
        println!("recieved: {:?}", message) // error: mismatched types: expected `IntoFuture` but found `()`
    })
    .map_err(|e| println!("read error: {:?}", e))
...

But is this possible at all?

@ultrasaurus
Copy link

I also found this error message confusing and referred here via stackoverflow question

Setting aside whether it is possible, it would have been really helpful to see an error like:

expected return value to be a type that implements `IntoFuture` trait;  
however, return value has type `()` which does not implement the required trait
-- perhaps this code is missing an explicit return value?

That's kind of wordy, but thought another suggestion might help someone to come up with an improvement here.

The key thing is issue for me is that the annotation directs my attention to the top of the function (which is good since I would need to look that up in the docs), but I didn't have the experience to recognize that it was referring to an incorrect type of the return value and had forgotten that a function that ends with println! is actually returning ().

@estebank
Copy link
Contributor

@ultrasaurus thanks for the suggestion! This is something we certainly want to improve but because reasons (gesticulates wildly) we are limited in what we can do today. We have some planned work that will help tremendously, I think and the minimized case in your question is very useful.

@oli-obk another case of the "pointing at call instead of argument making things confusing" problem that might get a great improvement by keeping the right obligation.span.

@jeremy-kothe
Copy link

Upvoting for frustration.

@crlf0710 crlf0710 added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 11, 2020
osa1 added a commit to osa1/rust that referenced this issue Jan 26, 2021
Don't suggest it if the last statement doesn't have a semicolon

Fixes rust-lang#81098

See also rust-lang#54771 for why this suggestion was added
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 26, 2021
Refine "remove semicolon" suggestion in trait selection

Don't suggest it if the last statement doesn't have a semicolon

Fixes rust-lang#81098

See also rust-lang#54771 for why this suggestion was added
zaharidichev pushed a commit to zaharidichev/rust that referenced this issue Mar 15, 2021
Don't suggest it if the last statement doesn't have a semicolon

Fixes rust-lang#81098

See also rust-lang#54771 for why this suggestion was added
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 10, 2022
…bank

Only suggest removing semicolon when expression is compatible with `impl Trait`

rust-lang#54771 (comment)
> It still needs checking that the last statement's expr can actually conform to the trait, but the naïve behavior is there.

Only suggest removing a semicolon when the type behind the semicolon actually implements the trait in an RPIT `-> impl Trait`. Also upgrade the label that suggests removing the semicolon to a suggestion (should it be verbose?).

cc rust-lang#54771
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 11, 2022
…bank

Only suggest removing semicolon when expression is compatible with `impl Trait`

rust-lang#54771 (comment)
> It still needs checking that the last statement's expr can actually conform to the trait, but the naïve behavior is there.

Only suggest removing a semicolon when the type behind the semicolon actually implements the trait in an RPIT `-> impl Trait`. Also upgrade the label that suggests removing the semicolon to a suggestion (should it be verbose?).

cc rust-lang#54771
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 11, 2022
…bank

Only suggest removing semicolon when expression is compatible with `impl Trait`

rust-lang#54771 (comment)
> It still needs checking that the last statement's expr can actually conform to the trait, but the naïve behavior is there.

Only suggest removing a semicolon when the type behind the semicolon actually implements the trait in an RPIT `-> impl Trait`. Also upgrade the label that suggests removing the semicolon to a suggestion (should it be verbose?).

cc rust-lang#54771
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 11, 2022
…bank

Only suggest removing semicolon when expression is compatible with `impl Trait`

rust-lang#54771 (comment)
> It still needs checking that the last statement's expr can actually conform to the trait, but the naïve behavior is there.

Only suggest removing a semicolon when the type behind the semicolon actually implements the trait in an RPIT `-> impl Trait`. Also upgrade the label that suggests removing the semicolon to a suggestion (should it be verbose?).

cc rust-lang#54771
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 11, 2022
Only suggest removing semicolon when expression is compatible with `impl Trait`

rust-lang#54771 (comment)
> It still needs checking that the last statement's expr can actually conform to the trait, but the naïve behavior is there.

Only suggest removing a semicolon when the type behind the semicolon actually implements the trait in an RPIT `-> impl Trait`. Also upgrade the label that suggests removing the semicolon to a suggestion (should it be verbose?).

cc rust-lang#54771
@estebank
Copy link
Contributor

estebank commented Nov 7, 2023

Current output:

error[E0308]: mismatched types
 --> src/lib.rs:3:13
  |
3 | fn foo() -> u8 {
  |    ---      ^^ expected `u8`, found `()`
  |    |
  |    implicitly returns `()` as its body has no tail or `return` expression
4 |     5;
  |      - help: remove this semicolon to return this value
error[E0277]: the trait bound `(): Bar` is not satisfied
 --> src/lib.rs:3:13
  |
3 | fn foo() -> impl Bar {
  |             ^^^^^^^^ the trait `Bar` is not implemented for `()`
4 |     5;
  |     -- help: remove this semicolon
  |     |
  |     this expression has type `{integer}`, which implements `Bar`
error[E0277]: the trait bound `(): Bar` is not satisfied
 --> src/main.rs:6:5
  |
6 |     bar(|| { 5u8; })
  |     ^^^ the trait `Bar` is not implemented for `()`
  |
  = help: the trait `Bar` is implemented for `u8`
note: required by a bound in `bar`
 --> src/main.rs:4:11
  |
4 | fn bar<R: Bar>(_: impl Fn() -> R) {}
  |           ^^^ required by this bound in `bar`

@estebank estebank removed the A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. label Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: closures (`|args| { .. }`) A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. P-medium Medium priority 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

7 participants