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

[SR-14883] Consider must in diagnostic error message for empty switch cases #57230

Closed
swift-ci opened this issue Jul 7, 2021 · 4 comments
Closed

Comments

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Jul 7, 2021

Previous ID SR-14883
Radar rdar://problem/80277466
Original Reporter msbit (JIRA User)
Type Improvement
Status Closed
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Improvement, DiagnosticsQoI, StarterBug
Assignee wongzigii (JIRA)
Priority Medium

md5: c2b1762aef01cfc470f0706ce3ac5a80

Issue Description:

When compiling something like:

```
import Darwin

switch getpid() {
case 1:
default:
}
```

compilation fails with the following errors:

```
test.swift:4:1: error: 'case' label in a 'switch' should have at least one executable statement
case 1:
^~~~~~~
break
test.swift:5:1: error: 'default' label in a 'switch' should have at least one executable statement
default:
^~~~~~~~
break
```

Given that these are errors, it seems sensible (to me) to change the wording from `should` to `must`.

@typesanitizer
Copy link

@typesanitizer typesanitizer commented Jul 7, 2021

Sounds reasonable to me.

@swift-ci create

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Jul 14, 2021

Comment by Zigii Wong (JIRA)

Hello theindigamer (JIRA User) I am just trying to resolve this SR as my first contribution to Swift compiler. I've changed `DiagnosticsParse.def` file and the test file `switch.swift` in `test/Parse` folder, what is the meaning of the `8-8= break` by the way?

What else should I make changes to finish this one? How can I test the change locally?

@typesanitizer
Copy link

@typesanitizer typesanitizer commented Jul 14, 2021

The M-N=blah syntax means that there is a fix-it to replace the text for column numbers in the range M ..< N with the text blah. So with 8-8= break, it would add a break at the position (preceded by a space).

You can run tests locally by recompiling the compiler with ninja -C builddir swift-frontend and using lit.py. For more details, check out https://github.com/apple/swift/blob/main/docs/HowToGuides/GettingStarted.md#running-tests

I don't think there should be any changes needed apart from the test cases and the diagnostic file. In case there are other tests which also have this diagnostic, you may see failures there too, so you'll need to update the text.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Jul 15, 2021

Comment by Zigii Wong (JIRA)

Submit PR here: https://github.com/apple/swift/pull/38391

Unfortunately some tests on Linux are failed.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants