-
Notifications
You must be signed in to change notification settings - Fork 10.5k
#71870 Respect @discardableResult
when calling with try?
#72380
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
base: main
Are you sure you want to change the base?
Conversation
|
||
ApplyExpr *call = nullptr; | ||
|
||
// Unwrap optional try to get inner function calls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function call expr of try? foo()
is wrapped in OptionalTryExpr
and InjectIntoOptionalExpr
. It need to unwrap them.
(optional_try_expr type="Int?" location=issue.swift:5:8 range=[issue.swift:5:3 - line:5:12] thrown_error="<null>"
(inject_into_optional implicit type="Int?" location=issue.swift:5:8 range=[issue.swift:5:8 - line:5:12]
(call_expr type="Int" location=issue.swift:5:8 range=[issue.swift:5:8 - line:5:12] isolation_crossing="none"
(declref_expr type="() throws -> Int" location=issue.swift:5:8 range=[issue.swift:5:8 - line:5:8] decl="issue.(file).foo()@issue.swift:2:6" function_ref=single)
(argument_list))))
lib/Sema/TypeCheckStmt.cpp
Outdated
{ | ||
auto nextExpr = valueE; | ||
while(true) { | ||
if (nextExpr == nullptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder if it would work the same if you move this check into the while
condition? i.e:
while(nextExpr != nullptr)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jamezzzb Fixed. Thanks
@swift-ci please smoke test |
ca884a2
to
ff08030
Compare
@giginet Please refrain from prefixing GitHub issue numbers with "SR". An SR identifier is a distinct identifier that was given to a report in the old Jira bug tracker, so only old issues that date back to Jira and were migrated have such an identifier — in addition to the GitHub issue number. |
@discardableResult
when calling with try?
@discardableResult
when calling with try?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution! I think this change requires Swift Evolution (you can start by pitching it in Swift Evolution -> Pitches category). The problem here is semantic - try?
actually produces an optional value that wraps the result of the call where nil
case indicates that an error has been thrown, which is semantically distinct from the result of the function itself, that's why i.e. getSemanticsProvidingExpr()
is not looking through OptionalTryExpr
.
When functions are called with optional try (
try?
) raise warnings even if the function annotates@discardableResult
.@discardableResult
should be respected in such a case.This PR changes the logic to raise the warning after checking
discardableResult
was applied.warning
is suppressed with this change.Resolves #71870
@discardableResult
works withtry
andtry!
, but nottry?
· Issue #71870 · apple/swift