[SR-3082] Throwing constructors with a method immediately called on them result in warning

[khanlou]

Previous ID	SR-3082
Radar	None
Original Reporter	@khanlou
Type	Bug

Environment

Swift 3, Xcode 8.1

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 0 | |Component/s | | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 2b4e024ed59312e8b6a1f66407e28d1e

relates to:

• SR-1681 spurious "unused result" warning in Swift 3

Issue Description:

A throwing constructor that has a method with a `discardableResult` immediately called on it results in a warning:

        struct ThrowingConstructor {
            init() throws {

            }

            @discardableResult
            func method() -> String {
                return "test"
            }
        }

        try? ThrowingConstructor().method()  // Warning: Result of 'try?' is unused

This issue can't be reproduced in a playground, I think because playgrounds don't have unused result errors turned on.

The warning can be silenced by using `_ =`. It can also be silenced with parentheses: `(try? ThrowingConstructor())?.method()`

[belkadan]

This is expected behavior. try? (and try) apply to the whole expression, and so if anything throws within that expression the entire thing is optional (which you are then supposed to test for, or at least explicitly ignore).

@rjmccall, anything to add?

[khanlou]

So it looks like this problem can be simplified to this, then:

struct Throwing {
    @discardableResult
    func method() throws -> String {
        return "test"
    }
}

let throwing = Throwing()
try? throwing.method()  // Warning: Result of 'try?' is unused

I still have an issue with it, which is that since `method()` is marked as having a discardableResult, `try?` should inherit that behavior. Note that regular `try` does appropriately inherit the behavior:

let throwing = Throwing()
do {
    try throwing.method()  // No warning
} catch _ {

}

[khanlou]

This bug looks related: https://bugs.swift.org/browse/SR-1681

[belkadan]

The result you're allowed to discard is the result of method, which explains the behavior of try. This is definitely closer to the optional chaining case you pointed out next, where something other than the result is being ignored, yet the compiler doesn't force you to deal with it. I still think that explicitly ignoring the result is the correct way to deal with this, though—you really are ignoring an error, and that should be called out explicitly.

[belkadan]

(@mattneub and I discussed try? in that bug as well.)

[rjmccall]

I agree with Soroush that the result of try? should be understood to be ignorable if the operand is. People do use try? idiomatically to ignore errors, and I think that has to be okay; specifically, if we weren't okay with it, we shouldn't have added try? at all.

[swift-ci]

Comment by Tim Bodeit (JIRA)

Im with @belkadan on regarding ignoring the error and discarding a potential result as two different things.
However, I do believe that the behavior should be consistent for both `@discardableResult` and void methods.

@@belkadan: Your reasoning in the comments of SR-3082 and SR-1681 matches your implementation of the unused `try?` warnings in bd031d. Since then this logic has been changed in regards to Void methods when SR-1752 was fixed in b956dc.

I have only glanced over the swift-evolution Draft Tuple-Based Compound Optional Binding thread referred to in the comments of PR#3057. However, when taking the above mentioned aspects into account, I do not think it gives a reason for the change to the warnings on unused results of `try?` expressions.

If no action is to be taken on this issue, I think we should reintroduce the warnings in regards to void expressions. Speaking in test behavior, this would mean reversing the changes to `test/Parse/try.swift` in b956dc.

Either way (addressing this issue for @discardableResult or reintroducing warnings for void): I'd be happy to write a PR for this.

cc @ahoppen: What is your opinion on this?

[ahoppen]

I'm in no position to give a fully qualified opinion on this but since you directly ask me, I'm currently in the camp to say that no warning should be issued in the above code.
Recalling the discussion for discarding Void? because of optional chaining, I think we said that, yes, there is actually information we are discarding, namely whether foo in bar?.foo() has actually been invoked, but the information content is too little and rarely used to warn about not using it.
I think this is similar. Since the result is discardable, we can consider it to have an information content of zero. That boils the information content of try? throwing.method() down to whether or not an error has been thrown. And we agree that this information content is also too small to cause a warning if method doesn't return anything.

[YOCKOW]

Reduced code:

@discardableResult
func discardMe() throws -> Int { 0 }
try discardMe()  // no warning
try! discardMe() // no warning
try? discardMe() // warning: result of 'try?' is unused

func returnsVoid() throws {}
try returnsVoid()  // no warning
try! returnsVoid() // no warning
try? returnsVoid() // no warning

This behavior has not changed still in Swift 5.3.
I think it's inconsistent, but I don't know what the majority thinks.

[StevenSorial]

Hopefully, this can be fixed soon