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 A-diagnostics A-suggestion-diagnostics AsyncAwait-Triaged C-enhancement D-confusing D-papercut E-needs-mentor P-medium T-compiler

Comments

@Nokel81
Copy link
Contributor

@Nokel81 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 A-diagnostics D-confusing D-papercut T-compiler A-async-await labels Jan 24, 2020
@JohnTitor JohnTitor added the C-enhancement label Jan 24, 2020
@tmandry tmandry added AsyncAwait-OnDeck AsyncAwait-Triaged labels Jan 28, 2020
@Centril
Copy link
Contributor

@Centril Centril commented Jan 28, 2020

Implementation notes:

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

@estebank
Copy link
Contributor

@estebank estebank commented Jan 28, 2020

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

@Nokel81
Copy link
Contributor Author

@Nokel81 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 Centril commented Jan 28, 2020

@rustbot assign @Nokel81

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

@Nokel81 Nokel81 commented Jan 30, 2020

@rustbot claim

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

@Nokel81 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

@estebank estebank commented Jan 30, 2020

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

@tmandry tmandry added this to To do in wg-async work Feb 11, 2020
@Nokel81
Copy link
Contributor Author

@Nokel81 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 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

@estebank estebank commented Feb 18, 2020

@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 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 label Mar 3, 2020
@tmandry
Copy link
Contributor

@tmandry tmandry commented Mar 3, 2020

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

@tmandry tmandry moved this from To do to In progress in wg-async work Mar 3, 2020
@tmandry
Copy link
Contributor

@tmandry 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 Nokel81 commented Mar 17, 2020

I am still interested in working on this.

@tmandry tmandry added the E-needs-mentor label Mar 24, 2020
@nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Mar 24, 2020

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 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

@nikomatsakis nikomatsakis commented Mar 24, 2020

@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 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 Nokel81 commented Mar 24, 2020

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

@bors bors closed this as completed in e3a514c Apr 21, 2020
wg-async work automation moved this from In progress to Done Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await A-diagnostics A-suggestion-diagnostics AsyncAwait-Triaged C-enhancement D-confusing D-papercut E-needs-mentor P-medium T-compiler
Projects
Development

Successfully merging a pull request may close this issue.

7 participants