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

Update error handling for typed throws [SE-0413] #296

Merged
merged 20 commits into from
Jun 4, 2024

Conversation

amartini51
Copy link
Member

Fixes: rdar://118896654

@amartini51 amartini51 added TR Needed This pull request needs technical review Edit Needed This pull request needs editorial review labels Apr 21, 2024
@amartini51 amartini51 requested a review from DougGregor April 22, 2024 20:45
@amartini51
Copy link
Member Author

amartini51 commented Apr 22, 2024

Proposal authors, @DougGregor @minuscorp @torstenlehmann, can you please review for technical accuracy?

I've adjusted the framing of when to use typed throws slightly, compared to the SE proposal. The discussion in the SE proposal of implementation details and dependency-free code seems overlap in the sense that the errors aren't part of the code's API to its clients. When vending a throwing API, it's important to use throws(any Error).

I haven't yet added a comparison of throws(T) to rethrows. Do we want to recommend the typed throws spelling in general, or is there still a reason you might want to use rethrows? For example, rethrows still enforces the constraint that errors can come only from called code.

@minuscorp
Copy link

It looks good to me 👍

Copy link

@rebeccateibloom rebeccateibloom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy edits, as requested in 125975390.

Comment on lines 712 to 713
New versions of a library can throw new errors ---
including libraries used by your dependencies ---

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
New versions of a library can throw new errors ---
including libraries used by your dependencies ---
New versions of a library
--- including libraries that your dependencies use ---
can throw new errors,


- When running code on an embedded system
that doesn't support dynamic allocation of memory.
Throwing an instance `any Error` or another boxed protocol type

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Throwing an instance `any Error` or another boxed protocol type
Throwing an instance of `any Error` or another boxed protocol type

- When the errors are used only within some unit of code,
like a library,
and aren't part of the interface to that code.
Because the errors come only from the library,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Because the errors come only from the library,
Because the errors come from only the library

and not from other dependencies or the library's clients,
you can make an exhaustive list of all possible failures.
And because these errors are an implementation detail of the library,
and they're always handled within that library.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
and they're always handled within that library.
they're always handled within that library.

And because these errors are an implementation detail of the library,
and they're always handled within that library.

- In code that only throws errors that were thrown elsewhere,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- In code that only throws errors that were thrown elsewhere,
- In code that throws only errors that were thrown elsewhere,

Throwing an error of a specific type
lets Swift allocate that memory upfront instead.

- When the errors are used only within some unit of code,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of an elegant way to make this part of the sentence active ("When ____ uses the errors only within some unit of code"). Hopefully you know the accurate subject that belongs in the blank.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be a bit tricky to put into active voice, and not necessarily clearer — this is a good example of an actor that's poorly defined and unimportant. It might be better as "When the errors are contained within some unit of code"? Or to rewrite more aggressively:

When the errors are an implementation detail of some unit of code,
like a library,
and aren't part of the interface to that code.

I'll start with that last rewrite, and we can iterate.

you write `throws(StatisticsError)` when declaring the function,
instead of just writing `throws`.
This syntax is also called *typed throws*
because you write the error type after `throws` --- for example:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
because you write the error type after `throws` --- for example:
because you write the error type after `throws`, as in this example:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrote to frame more specifically what the developers should look at the example for.

Comment on lines 848 to 849
the `catch` clause can either handle every possible error,
or it can propagate unhandled errors for some surrounding scope to handle.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
the `catch` clause can either handle every possible error,
or it can propagate unhandled errors for some surrounding scope to handle.
the `catch` clause can either handle every possible error
or propagate unhandled errors for some surrounding scope to handle.

the error type is inferred as follows:

- If every `throws` statement and `try` expression in the `do` code block
is nested inside of an exhaustive error handling mechanism,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
is nested inside of an exhaustive error handling mechanism,
is nested inside of an exhaustive error-handling mechanism,

Comment on lines 369 to 371
1. Functions that throw any error, marked `throws(any Error)`.
1. Functions that throw a specific error, marked `throws(MyError)`.
1. Functions that don't throw, marked `throws(Never)`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't include periods because these aren't complete sentences, unless it is your style in TSPL to put periods at the end of all bulleted lists.

Suggested change
1. Functions that throw any error, marked `throws(any Error)`.
1. Functions that throw a specific error, marked `throws(MyError)`.
1. Functions that don't throw, marked `throws(Never)`.
1. Functions that throw any error, marked `throws(any Error)`
1. Functions that throw a specific error, marked `throws(MyError)`
1. Functions that don't throw, marked `throws(Never)`

@amartini51 amartini51 marked this pull request as ready for review May 10, 2024 21:50
Edits from <rdar://125975390>

Co-authored-by: Rebecca Teibloom <[email protected]>
@amartini51 amartini51 force-pushed the typed_throws_118896654 branch from dbfe725 to 4baad40 Compare May 16, 2024 05:20
Copy link
Member Author

@amartini51 amartini51 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rebeccateibloom Thanks! I've incorporated most of your edits and left queries on a few. There are some that I need to come back to tomorrow still too.

TSPL.docc/LanguageGuide/ErrorHandling.md Outdated Show resolved Hide resolved
TSPL.docc/ReferenceManual/Statements.md Show resolved Hide resolved
Comment on lines 728 to 730
requires allocating memory at runtime to store the error.
Throwing an error of a specific type
lets Swift allocate that memory upfront instead.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved "In contrast" to the beginning.

TSPL.docc/LanguageGuide/ErrorHandling.md Show resolved Hide resolved
Throwing an error of a specific type
lets Swift allocate that memory upfront instead.

- When the errors are used only within some unit of code,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be a bit tricky to put into active voice, and not necessarily clearer — this is a good example of an actor that's poorly defined and unimportant. It might be better as "When the errors are contained within some unit of code"? Or to rewrite more aggressively:

When the errors are an implementation detail of some unit of code,
like a library,
and aren't part of the interface to that code.

I'll start with that last rewrite, and we can iterate.

TSPL.docc/LanguageGuide/ErrorHandling.md Show resolved Hide resolved
like a function that takes a closure argument
and propagates any errors from that closure.
For a comparison between `rethrows`
and throwing a specific, generic, error type
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped "generic" to make the xref less specific, and easier to phrase. Started with the half of the comparison that's relevant to the current discussion.

Comment on lines 895 to 896
the `do` block can throw errors of only the specified *type*.
The *type* must be either
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(As above, line 734)

TSPL.docc/LanguageGuide/ErrorHandling.md Show resolved Hide resolved
As a result, you can use a nonthrowing function
A function's type includes whether it can throw an error,
and what type of error it throws.
This means, for example, you can use a nonthrowing function
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example given here is representative, not exhaustive — and the old wording is somewhat misleading, in suggesting this is the only consequence of being a subtype. There are other things that you can do with a subtype besides just using it where the supertype is expected. With that in mind, do you think "so" is an improvement here? I don't think it matches the meaning.

@rebeccateibloom
Copy link

@amartini51 I read through your comments and the latest commit. I marked a few of my past comments/queries as resolved. Thanks for the patient explanations!

Copy link

@rebeccateibloom rebeccateibloom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thank you.

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this! I left a few suggestions, but this covers all of the major aspects of typed throws that we need to cover.

you don't know ahead of time every error that could happen
while the code is running,
especially when propagating errors thrown somewhere else.
This approach also reflects the fact that errors can change over time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"This approach" starts both this sentence and the prior one. Perhaps use this instead?

Suggested change
This approach also reflects the fact that errors can change over time.
It also reflects the fact that errors can change over time.

requires allocating memory at runtime to store the error.
In contrast,
throwing an error of a specific type
lets Swift allocate that memory upfront.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using "allocate" again here implies the heap. Perhaps somehing like this:

Suggested change
lets Swift allocate that memory upfront.
lets Swift avoid heap allocation for errors.

Comment on lines 743 to 745
- In code that throws only errors that were thrown elsewhere,
like a function that takes a closure argument
and propagates any errors from that closure.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to mention that this is generic code here. We could say "in generic code", or do something like this:

Suggested change
- In code that throws only errors that were thrown elsewhere,
like a function that takes a closure argument
and propagates any errors from that closure.
- In code that only propagates errors described by generic parameters,
like a function that takes a closure argument
and propagates any errors from that closure.

Because the `do`-`catch` statement throws `StatisticsError` values,
`error` is a value of type `StatisticsError`.

<!-- XXX show multiple catch clauses with different patterns? -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you have to show multiple catch clauses specifically. It falls out from other sections.

`someThrowingFunction()` propagates any errors that `summarize(_:)` throws.
The errors from `summarize(_:)` are always `StatisticsError` values,
which is also a valid error for `someThrowingFunction()` to throw.
<!-- XXX Expand on subtyping here? -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to expand on subtyping. However, I would like to describe the Never case, because it's an important part of understanding how typed throws fits in the type system. For example, it could be something like this:

Just like you can write a function that never returns with a return type of Never, you can write a function that never throws with throws(Never):

```swift
func nonThrowingFunction() throws(Never) {
  // ...
}
```

This function cannot throw because it is impossible to create a value of type Never to throw.

@@ -1582,6 +1594,14 @@ and a throwing method can't satisfy a protocol requirement for a rethrowing meth
That said, a rethrowing method can override a throwing method,
and a rethrowing method can satisfy a protocol requirement for a throwing method.

<!-- XXX comparison between rethrows and generic typed throws
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of me wants to suggest that this whole section be revised to replace rethrows entirely with the use of generic typed throws, because that's how I expect the language to go over time. Then, rethrows would become more of a historical anecdote. However, that is a lot of work, and perhaps we'll find that rethrows will remain for longer than I expect. If so, I think it's fine to keep this comparison short: take the someFunction example from the beginning of the section and make it generic over the thrown error type, and note that it's providing a more precise guarantee about what errors it rethrows than rethrows does (because rethrows always produces an any Error).

}
```

<!-- XXX Is "do throws { }" allowed? -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is. It's equivalent to throws(any Error).

TSPL.docc/ReferenceManual/Statements.md Show resolved Hide resolved
and nonthrowing functions are subtypes of throwing functions.
As a result, you can use a nonthrowing function in the same places as a throwing one.
that can throw or rethrow an error must include the `throws` keyword.
<!-- XXX TR: Confirm rethrowing functions use 'throws' -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, rethrowing functions are always considered to throw any Error.

@amartini51 amartini51 removed TR Needed This pull request needs technical review Edit Needed This pull request needs editorial review labels May 28, 2024
@amartini51 amartini51 requested a review from bjlanier May 30, 2024 19:20
@amartini51 amartini51 merged commit d3bb2ea into swiftlang:main Jun 4, 2024
@amartini51 amartini51 deleted the typed_throws_118896654 branch June 4, 2024 18:03
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.

5 participants