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-3082] Throwing constructors with a method immediately called on them result in warning #45672

Open
khanlou opened this issue Oct 30, 2016 · 10 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior.

Comments

@khanlou
Copy link
Contributor

khanlou commented Oct 30, 2016

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
Copy link
Contributor

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
Copy link
Contributor Author

khanlou commented Oct 31, 2016

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
Copy link
Contributor Author

khanlou commented Oct 31, 2016

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

@belkadan
Copy link
Contributor

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
Copy link
Contributor

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

@rjmccall
Copy link
Member

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
Copy link
Collaborator

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
Copy link
Contributor

ahoppen commented Dec 20, 2016

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
Copy link
Collaborator

YOCKOW commented Feb 10, 2021

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.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@StevenSorial
Copy link

Hopefully, this can be fixed soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior.
Projects
None yet
Development

No branches or pull requests

7 participants