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

Bad error message with async main #68523

Closed
Nokel81 opened this issue Jan 24, 2020 · 19 comments · Fixed by #71174
Closed

Bad error message with async main #68523

Nokel81 opened this issue Jan 24, 2020 · 19 comments · Fixed by #71174
Assignees
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-papercut Diagnostics: An error or lint that needs small tweaks. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Nokel81
Copy link
Contributor

Nokel81 commented Jan 24, 2020

If someone tries to do the following async fn main() -> Result<(), Box<dyn std::error::Error> the error message is not helpful (though it is technically correct).

error[E0277]: `main` has invalid return type `impl futures::Future`
  --> rs/agent-updater/src/main.rs:46:20
   |
46 | async fn main() -> Result<(), Box<dyn std::error::Error>> {
   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `main` can only return types that implement `std::process::Termination`
   |
   = help: consider using `()`, or a `Result`

I think that it should point to the async keyword and say that standard rust does not support async main.

This issue has been assigned to @Nokel81 via this comment.

@Centril Centril added A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-async-await Area: Async & Await labels Jan 24, 2020
@JohnTitor JohnTitor added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jan 24, 2020
@tmandry tmandry added AsyncAwait-OnDeck AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. labels Jan 28, 2020
@Centril
Copy link
Contributor

Centril commented Jan 28, 2020

Implementation notes:

cc @estebank who has dealt with fn main diagnostics recently.

@estebank
Copy link
Contributor

This is a good first-contribution ticket I can help with.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Jan 28, 2020

I will take you up on that offer Wednesday evening if that is good with you.

@Centril
Copy link
Contributor

Centril commented Jan 28, 2020

@rustbot assign @Nokel81

@rustbot rustbot self-assigned this Jan 28, 2020
@Nokel81
Copy link
Contributor Author

Nokel81 commented Jan 30, 2020

@rustbot claim

@rustbot rustbot assigned rustbot and unassigned rustbot Jan 30, 2020
@Nokel81
Copy link
Contributor Author

Nokel81 commented Jan 30, 2020

@estebank Though I realize it might be too late for today. But how should be chat?

@estebank
Copy link
Contributor

I'm on the street now, but if you have specific questions I can answer them when I get home.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Feb 16, 2020

Work is going well. I should have a preliminary PR up early next week.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Feb 18, 2020

I have two things that I have run into:

  1. should I be using rustc_span::source_map::dummy_spanned?
  2. I am having trouble figuring out why I get a bug! panic at src/librustc_metadata/rmeta/decoder.rs:398: Cannot decode Span without Session., would anyone have an idea as to how to set up a Session?

@estebank
Copy link
Contributor

@Nokel81 could you push your code to a branch we can look at and comment inline? You should be extracting an existing span.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Feb 19, 2020

  1. There is a bunch of places in the code where map_or_else or a default hir::IsAsync::NotAsync was used and I needed to a way to make it Spanned<hir::IsAsync>
  2. https://github.com/Nokel81/rust/tree/async_main_error
  3. I am also having trouble trying to figure out where fn main is type checked as different then any other function.

@tmandry tmandry added the P-medium Medium priority label Mar 3, 2020
@tmandry
Copy link
Member

tmandry commented Mar 3, 2020

Ping from triage. @estebank, could you follow up?

@tmandry
Copy link
Member

tmandry commented Mar 17, 2020

Ping. @estebank, is this something you can continue to mentor? @Nokel81, are you still interested in working on this?

@Nokel81
Copy link
Contributor Author

Nokel81 commented Mar 17, 2020

I am still interested in working on this.

@nikomatsakis
Copy link
Contributor

I'm looking quickly over the branch, @Nokel81.

@estebank or others may feel differently but my take:

I don't think it's a good idea to return Spanned<hir::IsAsync> if we are going to sometimes use a dummy span. I see that we do use a similar pattern for VisibilityKind, but I'm of the opinion that dummy spans should be avoided and should not be common.

If we were going to use Spanned, I'd want to avoid the dummy span, and instead use the span of the fn keyword or something when the function is not async.

However, we could also just just include the span for the "yes, is async" variant. This is what the ast::Async type does (see the Yes variant). We could certainly modify hir::Async to include a span just on the Yes variant as well. I don't see a ton of precedent for that in the HIR, but it seems reasonable.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Mar 24, 2020

Thanks for the quick look over @nikomatsakis. I am wondering what should happen if an error occurs where it is required to be async but isn't (maybe I am thinking too hard in the future).

I sort of like the span of the fn keyword solution, and it is definitely better than using a dummy value.

@nikomatsakis
Copy link
Contributor

@Nokel81 we can solve that problem later, but in that scenario, having a "dummy span" on hand would not be especially useful anyway -- that is, though, I think the right question to be asking, which is why I suggested the span for the "fn" keyword (or perhaps the fn name). i.e., what would you want to highlight in such a case?

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Mar 24, 2020

but perhaps better to wait until indeed we have such a message to emit, which might help clarify things

@Nokel81
Copy link
Contributor Author

Nokel81 commented Mar 24, 2020

Makes sense, I'll see about transitioning my change set to that using hir::Async::Yes with span information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-papercut Diagnostics: An error or lint that needs small tweaks. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants