-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[Sema] Uninhabited stored property downgrade to warning #20211
Conversation
Hm. Upon reading these, I don't think the "Void" warning is necessarily a good idea, because it doesn't say why it's a bad idea to have, e.g., Empty as a type. Maybe we could replace the "which may be unexpected" with the "which is an enum with no cases" / "which contains an enum that has no cases"? @jckarter, what do you think? |
(Thanks, Alejandro.) |
That seems like a clearer explanation, sure. Hotever, note that this would typically be paired with the "is never executed" warning about any following statements using the variable, which ought to help clarify why there's an issue. |
@jrose-apple does this look better? |
@swift-ci Please test |
Build failed |
Build failed |
@jckarter are the bots broken? |
The failures certainly don't look related to this change. I'll follow up. Meanwhile, we can try a smoke test. |
@swift-ci Please smoke test |
@jckarter I think the bots are functional once again. Can you kick off again? |
@swift-ci Please test |
Build failed |
@swift-ci Please smoke test |
fix conflict with reflection test
@jckarter can you kick off once more? |
@swift-ci Please smoke test |
1 similar comment
@swift-ci Please smoke test |
@swift-ci Please smoke test OS X platform |
1 similar comment
@swift-ci Please smoke test OS X platform |
Huh... |
@jckarter @xwu @jrose-apple is this ready for merge? |
LGTM. @jckarter, any further comments? |
Looks good. Thanks @Azoy! |
Refer to here: #19992 (comment)
cc: @jrose-apple @jckarter