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

Do not complain about missing main when a block has been recovered #67413

Closed
wants to merge 1 commit into from

Conversation

estebank
Copy link
Contributor

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 19, 2019

// Avoid complaining about main in
// `src/test/ui/parser/mismatched-braces/missing-close-brace-in-trait.rs`.
*self.sess.reached_eof.borrow_mut() = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that we actually reach Eof in all cases, e.g. the return below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to change it back. Setting reached_eof before all of the returns in the loop would cause the missing main error to come up (!?).

@rust-highfive

This comment has been minimized.

@Centril Centril 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 20, 2019
@bors

This comment has been minimized.

@bors

This comment has been minimized.

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

Centril commented Jan 9, 2020

I don't feel entirely comfortable reviewing the correctness of this right now (in particular that we don't suddenly allow code) and I don't have much time to do an in-depth review, so I'll r? @petrochenkov for now.

That said, here's a sledge-hammer suggestion which would be easy to be confident in: Record whether .has_err() after parsing and don't emit errors about missing fn main if there were.

@rust-highfive rust-highfive assigned petrochenkov and unassigned Centril Jan 9, 2020
@petrochenkov
Copy link
Contributor

I'd rather get rid of reached_eof and always emit the "no main" diagnostics instead.

This is such a random ad hoc bit of logic.
The unclosed delimiter recovery can swallow any kinds of items causing various semantic / type errors due to that, why should the "no main" error be treated specially?

@petrochenkov
Copy link
Contributor

The existing logic seems to set reached_eof pretty inconsistently too, and not necessarily when crate parsing encounters EOF while having an unclosed delimiter.
Every parser created to parse something local can set the global value in the session.
I also don't understand why recover_closing_delimiter and emit_unclosed_delims are two seemingly independent pieces of code.

I don't want to have this debt accumulating on top of other debt, reporting a false positive "no main" error in some cases has much less importance than that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants