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

Enhance error message when misspelled label to value in break expression #80023

Merged
merged 2 commits into from
Dec 18, 2020
Merged

Enhance error message when misspelled label to value in break expression #80023

merged 2 commits into from
Dec 18, 2020

Conversation

sasurau4
Copy link
Contributor

Fix #79424

@rust-highfive
Copy link
Collaborator

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 14, 2020
lcnr
lcnr reviewed Dec 14, 2020
View reviewed changes
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

❤️

How does this interact with something like

fn main() {
    'qq: loop {
        qq;
    }
}

if we also emit this help there it probably makes sense to check the source to check if we are inside of a break expression.

compiler/rustc_resolve/src/late/diagnostics.rs Outdated Show resolved Hide resolved
@sasurau4 sasurau4 force-pushed the feature/enhance-error-message-when-wrongly-written-broken-label branch from c907313 to 9570008 Compare December 15, 2020 11:47
@sasurau4
Copy link
Contributor Author

sasurau4 commented Dec 15, 2020 •
edited
Loading

Thanks for your review!

if we also emit this help there it probably makes sense to check the source to check if we are inside of a break expression.

How to check that? When I look into source, it's PathSource::Expr(None). Can I check if we are inside of a break expr with another way?

@sasurau4 sasurau4 requested a review from lcnr December 15, 2020 11:53
@sasurau4 sasurau4 force-pushed the feature/enhance-error-message-when-wrongly-written-broken-label branch from 9570008 to 7d18768 Compare December 15, 2020 12:01
@lcnr
Copy link
Contributor

lcnr commented Dec 15, 2020

not actually sure, we probably should try harder to get a relevant PathSource here but that is not really trivial.

Considering that the suggestion can easily be incorrect I feel that a simple help message instead span_suggestion might actually be slightly preferable here 😆 @sasurau4 how do you feel about this?

Am happy with whatever you end up prefering.

@sasurau4
Copy link
Contributor Author

@lcnr OK, I'll try to get relevent PathSource.

For span_suggestion, it would be preferable for me rather than using span_help. When using span_help, I had to insert ident_label.name to message by format macro but it doesn't need when using span_suggetion. It seems better.

@sasurau4
Copy link
Contributor Author

I look into PathSource and parent_scope, PathSource have Expr(None) and seems not able to access parent PathSource. On the other hand, parent_scope has only module related method and field.
I have no idea to achieve it. I think it's enough even though without checking. How about it?

@lcnr
Copy link
Contributor

lcnr commented Dec 16, 2020

r=me afterwards

sasurau4 and others added 2 commits December 17, 2020 20:22
@sasurau4 @lcnr
Enhance error message when misspelled label to value in break expression
7b9ee11 
Apply suggestions from code review

Co-authored-by: lcnr <bastian_kauschke@hotmail.de>
@sasurau4 @pickfire
Add test case for break expr with misspelled value
e9ca290 
Update src/test/ui/loops/loop-break-value.rs

Co-authored-by: Ivan Tham <pickfire@riseup.net>
@sasurau4 sasurau4 force-pushed the feature/enhance-error-message-when-wrongly-written-broken-label branch from fc29077 to e9ca290 Compare December 17, 2020 11:22
pickfire
pickfire reviewed Dec 17, 2020
View reviewed changes
src/test/ui/label/label_misspelled.rs
Comment on lines +2 to +6
'LOOP: loop {
LOOP;
//~^ ERROR cannot find value `LOOP` in this scope
};
'while_loop: while true { //~ WARN denote infinite loops with
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering, is the recommended label capitalization SCREAMING_SNAKE_CASE or snake_case?

Copy link
Contributor

Choose a reason for hiding this comment

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

the recommended label is snake_case from what I know

@lcnr
Copy link
Contributor

lcnr commented Dec 17, 2020

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 17, 2020

📌 Commit e9ca290 has been approved by lcnr

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 17, 2020
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Dec 17, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2020
@bors
Auto merge of rust-lang#80138 - Dylan-DPC:rollup-qamsfyh, r=Dylan-DPC
f3800db 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#78164 (Prefer regions with an `external_name` in `approx_universal_upper_bound`)
 - rust-lang#80003 (Fix overflow when converting ZST Vec to VecDeque)
 - rust-lang#80023 (Enhance error message when misspelled label to value in break expression)
 - rust-lang#80046 (Add more documentation to `Diagnostic` and `DiagnosticBuilder`)
 - rust-lang#80109 (Remove redundant and unreliable coverage test results)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a4b95ee into rust-lang:master Dec 18, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 18, 2020
@sasurau4 sasurau4 deleted the feature/enhance-error-message-when-wrongly-written-broken-label branch December 19, 2020 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lcnr lcnr lcnr approved these changes

@pickfire pickfire pickfire left review comments

Assignees

@lcnr lcnr

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.50.0
Development

Successfully merging this pull request may close these issues.

[ER] Better error message for wrongly written break label
6 participants
@sasurau4 @rust-highfive @lcnr @bors @pickfire @rustbot