Skip to content

[SR-1752] Fix warning about unused result if return type is Void? #3057

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

Merged
merged 1 commit into from
Jul 6, 2016

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Jun 18, 2016

What's in this pull request?

Right now, if a function has a return value of Optional<Void> a warning about an unused result is generated. This also applies for the use of optional chaining inside a ternary operator, e.g. if c.f() returns Void and you do true ? c?.f() : c?.f() a warning saying Expression of type '()?' is unused is generated.

Resolved bug number: (SR-1752)


Before merging this pull request to apple/swift repository:

  • Test pull request on Swift continuous integration.

Triggering Swift CI

The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:

Smoke Testing

Platform Comment
All supported platforms @swift-ci Please smoke test
All supported platforms @swift-ci Please smoke test and merge
OS X platform @swift-ci Please smoke test OS X platform
Linux platform @swift-ci Please smoke test Linux platform

Validation Testing

Platform Comment
All supported platforms @swift-ci Please test
All supported platforms @swift-ci Please test and merge
OS X platform @swift-ci Please test OS X platform
OS X platform @swift-ci Please benchmark
Linux platform @swift-ci Please test Linux platform

Lint Testing

Language Comment
Python @swift-ci Please Python lint

Note: Only members of the Apple organization can trigger swift-ci.

@harlanhaskins
Copy link
Contributor

@swift-ci please test

@lattner
Copy link
Contributor

lattner commented Jun 19, 2016

Thanks for working on this, but as the test runs above show, this causes a regression on the test suite.

@ahoppen
Copy link
Member Author

ahoppen commented Jun 19, 2016

I know, we just launched the CI in the lab to have a look at the test results and I didn't have time to update the tests yet. An update will follow in the next few days.

@ahoppen
Copy link
Member Author

ahoppen commented Jun 20, 2016

OK, I updated the tests and they should pass now.

try? in combination with a function returning Void will also no longer issue a warning as it was discussed in these swift-evolution threads: Original thread, my answer

This should simplify the rules on when a warning about an unused result is issued. Now such a warning should only be issued if the result type is not Void wrapped in any number of optionals and the called function is not marked as @discardableResult.

@ahoppen
Copy link
Member Author

ahoppen commented Jun 25, 2016

I resolved the merge conflicts that occurred because another test case was added to diagnostics.swift in 612991e

@CodaFi
Copy link
Contributor

CodaFi commented Jul 6, 2016

@ahoppen Looks like another conflict got introduced. Could you rebase again?

@ahoppen
Copy link
Member Author

ahoppen commented Jul 6, 2016

Once again the merge conflicts just resulted from another test being added to diagnostics.swift. I've resolved them now.

@CodaFi
Copy link
Contributor

CodaFi commented Jul 6, 2016

Thank you!

@swift-ci please test

@CodaFi
Copy link
Contributor

CodaFi commented Jul 6, 2016

macOS failures are unrelated to this patch.

Nice work @ahoppen 💥

@CodaFi
Copy link
Contributor

CodaFi commented Jul 6, 2016

Once more, with feeling!

@swift-ci please test and merge

@swift-ci swift-ci merged commit 51f364d into swiftlang:master Jul 6, 2016
@ahoppen ahoppen deleted the SR-1752 branch July 9, 2016 11:11
@timbodeit
Copy link
Contributor

Mailing list links are down.
They refer to the [swift-evolution] [Draft] Tuple-Based Compound Optional Binding thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants