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

Decide whether to suggest anything when encountering break and expecting ! #63390

Closed
estebank opened this issue Aug 8, 2019 · 9 comments
Closed
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. F-never_type `#![feature(never_type)]` P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Aug 8, 2019

#63337 (comment) introduces suggestions when encountering break without a value in a context that expects a value other than (). In the case that the expected value is ! we don't suggest anything. Once ! is stabilized, we should look at the actual usage and see if there's any sane situation where we would like to suggest something like break loop {}.

CC @Centril

@estebank estebank added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-diagnostics Area: Messages for errors, warnings, and lints P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. F-never_type `#![feature(never_type)]` labels Aug 8, 2019
@estebank estebank added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Aug 8, 2019
@johnfercher
Copy link

Hi, I would like to work in this issue. Could you explain the context of the modification?

@estebank
Copy link
Contributor Author

Hi @johnfercher! Thank you for wanting to contribute. At this point we first need to evaluate whether suggesting anything in this case is useful or detrimental to newcomers. The actual implementation part is straightforward and I can help you with it, but first we need to confirm we want to add such a suggestion to the compiler. Does that make sense?

@johnfercher
Copy link

Hi, @estebank totally make sense. How we proceed to confirm this?

@estebank
Copy link
Contributor Author

estebank commented Oct 1, 2019

@Centril who should be involved in this conversation?

@Centril
Copy link
Contributor

Centril commented Oct 1, 2019

I'm not sure, but let's wait until ! has been stable for a bit tho, yea?

@justjosias
Copy link
Contributor

It's been a year. Should this be looked at again?

@hameerabbasi
Copy link
Contributor

Another year. :) Maybe E-easy should be removed, so new contributors like me don't stumble on it?

@compiler-errors compiler-errors removed the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Apr 1, 2022
@pnkfelix
Copy link
Member

Discussed in T-compiler backlog bonanza.

Its not clear what concrete piece of code/scenario this issue is talking about.

As best I can tell, it is describing a scenario like this:

fn diverge() -> ! {
    loop {
        break 3;
    }
}

and the diagnostic needs to suggest something to put in for the input expression to break, and someone at some point suggested this:

fn diverge() -> ! {
    loop {
        break loop { };
    }
}

If that's what's being discussed, I definitely do not think the above suggestion is good. I don't know what the right suggestion is (it could be "do not break", or it could be "replace the break with an inner loop { }, or panic, etc.)

Anyway, we do not understand the exact scenario that was under discussion in the first place; the above is largely guesswork.


Furthermore, this isn't really a tracking issue; it isn't tracking progress on a language/compiler feature. Its just a note about a potential gotcha in the code.

@rustbot label: -C-tracking-issue

@rustbot rustbot removed the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Aug 12, 2022
@estebank
Copy link
Contributor Author

Let's just close. I've lost the context of what I was thinking back then, and I feel like it might be ok to ignore the issue for now until we have an actionable report with a realistic case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. F-never_type `#![feature(never_type)]` P-low Low 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

8 participants