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-1052] Implement SE-0047 - Defaulting non-Void functions so they warn on unused results #43664
Comments
Comment by Trent Nadeau (JIRA) By "independent patches", do you mean separate commits or completely separate PRs? |
I'd prefer separate PR's. Thanks! |
Comment by Trent Nadeau (JIRA) Created PR for part 1 (parsing): #1887 |
Another todo I forgot: we need to parse and recognize the doc comment extensions (MutableVariant🙂 etc. |
Comment by Trent Nadeau (JIRA) Chris, can my part 1 PR get reviewed and merged? I'd like to do more work on this issue this weekend, but this first step is blocking the rest. |
Weird, for some reason, I didn't get the email about this. I'll take a look right now! |
Comment by Trent Nadeau (JIRA) Thanks, Chris! |
Comment by Trent Nadeau (JIRA) Per Dmitri Gribenko, it may be useful to support adding the new attribute on subscripts and properties. It may not be necessary on properties though as it appears to be checked via other means and is an error (see GitHub comment). Per Chris Lattner, allowing the new attribute on new declaration types should only be added if there's demand. |
Comment by Trent Nadeau (JIRA) Created PR for Part 2 (stdlib): #2044 |
Comment by Trent Nadeau (JIRA) Created PR for Part 3 (clang importer): #2103 |
Comment by Trent Nadeau (JIRA) SE-0047 mentions new Should I add it as in the proposal or to be consistent with the existing doc keyword naming? |
My understanding is that these doc comments are case insensitive. The stdlib folks prefer camel case for them. |
Comment by Trent Nadeau (JIRA) Created PR for Part 4 (doc comment parsing): #2204 |
Comment by Trent Nadeau (JIRA) I'm starting to look at Part 5 (having the compiler use the new attr). What I have so far appears to be working with the exception of signatures like this:
where the return type is generic. For regular functions, I can skip the warning if the result type in the decl is Any ideas on how to tell if a call to a generic function like |
Comment by Trent Nadeau (JIRA) Created PR for Part 5 (use |
Comment by Trent Nadeau (JIRA) Created PR for Part 6 (remove |
Comment by Trent Nadeau (JIRA) All PRs merged. |
Additional Detail from JIRA
md5: 77cf91f85074b4a3b8d2ded18cca58e5
relates to:
Issue Description:
SE-0047 has been accepted:
https://github.com/apple/swift-evolution/blob/master/proposals/0047-nonvoid-warn.md
I suggest implementing this as a series of independent patches:
1) Introduce the @discardableResult declaration attribute.
2) Change the standard library to use it on various mutating methods that return values.
3) Change the clang importer to auto-attach the attribute to imported declarations (ones that are not marked with the clang "attribute((warn_unused_result))").
4) Parse and recognize the new doc comments (mutableVariant, etc.). See comment from Chris Lattner below.
5) Change the compiler to start listening to the @discardableResult attribute.
6) Change the @warn_unused_result attribute to be recognized, and produce an error + fixit to remove it.
The text was updated successfully, but these errors were encountered: