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.

Sign up for GitHub

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

Jump to bottom

overhaul "missing main" diagnostic #79086

Closed
wants to merge 1 commit into from
Closed

overhaul "missing main" diagnostic #79086

wants to merge 1 commit into from

Conversation

euclio
Copy link
Contributor

@euclio euclio commented Nov 16, 2020

This PR updates the diagnostic emitted when there is no main function at the crate root.

  • Chiefly, shrinks the span to avoid highlighting the entire root module
  • Uses the modern diagnostic machinery instead of referring to file names in the message
  • Removes a reference to the unstable #[main] attribute

Fixes #77968.

r? @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 16, 2020
@jyn514 jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 16, 2020
jyn514
jyn514 reviewed Nov 16, 2020
View reviewed changes
compiler/rustc_passes/src/entry.rs
Comment on lines +190 to +195
err.span_suggestion(
sp.shrink_to_lo(),
"define a function named `main` at the crate root",
String::from("fn main() {}\n"),
Applicability::MaybeIncorrect,
);
Copy link
Member

@jyn514 jyn514 Nov 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another hint that would be helpful is to pass --crate-type lib if you didn't mean for this to be a binary. I run into that a lot testing out things for rustdoc (really I want rust-lang/rust-playground#489, but in the meantime ...)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, a help with this info might be warranted.

estebank
estebank approved these changes Nov 25, 2020
View reviewed changes
compiler/rustc_passes/src/entry.rs
Comment on lines +186 to +187
let span = MultiSpan::from_spans(visitor.non_main_fns.iter().map(|(_, sp)| *sp).collect());
err.span_help(span, "consider moving one of these function definitions to the crate root");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't being tested, is it? Found the test.

Also, I wonder if a span_label per "non main fns" might look better.

The message doesn't account for a single fn being found (use pluralize!()).

compiler/rustc_passes/src/entry.rs
Comment on lines +190 to +195
err.span_suggestion(
sp.shrink_to_lo(),
"define a function named `main` at the crate root",
String::from("fn main() {}\n"),
Applicability::MaybeIncorrect,
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, a help with this info might be warranted.

src/test/ui/continue-after-missing-main.stderr
Comment on lines 1 to +3
error[E0601]: `main` function not found in crate `continue_after_missing_main`
--> $DIR/continue-after-missing-main.rs:1:1
|
LL | / #![allow(dead_code)]
LL | |
LL | | struct Tableau<'a, MP> {
LL | | provider: &'a MP,
... |
LL | |
LL | | }
| |_^ consider adding a `main` function to `$DIR/continue-after-missing-main.rs`
help: define a function named `main` at the crate root
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple of cases now where the main diagnostic doesn't have a primary span. We should have a span pointing to the first char at least. I wonder if it is because of the code deleted between 205 and 212.

@crlf0710 crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 11, 2020
@crlf0710
Copy link
Member

@euclio Triage: any updates on this?

@euclio
Copy link
Contributor Author

euclio commented Dec 13, 2020

Still on my radar. Hoping to get some time to work on this over the holidays.

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 11, 2021
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 1, 2021
@JohnCSimon
Copy link
Member

ping from triage:
@euclio can you please post your status?

@euclio
Copy link
Contributor Author

euclio commented Feb 4, 2021

I don't know when I'll be able to return to this. Closing for now.

@euclio euclio closed this Feb 4, 2021
@dtolnay dtolnay mentioned this pull request Jan 27, 2022
Closed
@euclio euclio deleted the missing-main branch January 22, 2023 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 left review comments

@estebank estebank estebank approved these changes

Assignees

@estebank estebank

Labels
A-diagnostics Area: Messages for errors, warnings, and lints S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error span for missing main function is too large
6 participants
@euclio @crlf0710 @JohnCSimon @estebank @jyn514 @rust-highfive