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.

Sign up for GitHub

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

Improve note on unsafe functions and unsafe blocks #4153

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve note on unsafe functions and unsafe blocks #4153

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/ch20-01-unsafe-rust.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,13 @@ With the `unsafe` block, we’re asserting to Rust that we’ve read the functio
documentation, we understand how to use it properly, and we’ve verified that
we’re fulfilling the contract of the function.

> Note: In earlier versions of Rust, the body of an unsafe function was treated
> as an `unsafe` block, so you could perform any unsafe operation within the
> body of an `unsafe` function. In later versions of Rust, the compiler will
> warn you that you need to use an `unsafe` block to perform unsafe operations
> in the body of an unsafe function. This is because Rust now distinguishes
> between `unsafe fn`, which defines what you need to do to call the function
> safely, and an `unsafe` block, where you actually uphold that “contract” the
> function establishes.
> Note: In earlier editions of Rust, the body of an unsafe function was treated
> as an `unsafe` block, so you could perform any unsafe operation within it.
> Starting with the 2024 edition, the compiler will warn you that you need to
> use an `unsafe` block to perform unsafe operations even in the body of an
> unsafe function. This helps to keep `unsafe` blocks as small as possible, as
> unsafe operations may not be needed across the whole function body. For more
> information about Rust editions, refer to [Appendix E][editions].
Comment on lines +189 to +195
Copy link
Contributor

Choose a reason for hiding this comment

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

Rereading this gave me the idea to invert the ordering a bit, and I think that helps a little with the flow.

Suggested change
> Note: In earlier editions of Rust, the body of an unsafe function was treated
> as an `unsafe` block, so you could perform any unsafe operation within it.
> Starting with the 2024 edition, the compiler will warn you that you need to
> use an `unsafe` block to perform unsafe operations even in the body of an
> unsafe function. This helps to keep `unsafe` blocks as small as possible, as
> unsafe operations may not be needed across the whole function body. For more
> information about Rust editions, refer to [Appendix E][editions].
> Note: Starting with the 2024 Edition of Rust, the compiler will warn you that
> you need to use an `unsafe` block to perform unsafe operations even in the
> body of an unsafe function. This helps to keep `unsafe` blocks as small as
> possible, as unsafe operations may not be needed across the whole function
> body. In earlier editions, the body of an unsafe function was treated as an
> `unsafe` block, so you could perform any unsafe operation within it. For more
> information about Rust editions, refer to [Appendix E][editions].

Copy link
Author

Choose a reason for hiding this comment

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

I think I somewhat prefer the current order. However, feel free to reuse and amend this as you like! If it helps, I can commit the reordering as well.

The way I see it is that the section is about (a) the fact that unsafe blocks allow you to call unsafe functions, which is otherwise disallowed and (b) introducing what unsafe functions are. (a) is mentioned above. As for (b), one aspect of unsafe functions is that they define an obligation to the caller, this is also explained above. The other aspect, that their bodies are effectively unsafe blocks, is in the process of being removed, and the note is about that. In its reordered version, it seems to assume that the reader already knows, or at least expects this second feature. In my opinion this breaks the link to the reader.

Copy link
Contributor

Choose a reason for hiding this comment

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

That’s reasonable, and got me thinking further on how to phrase it. Let me suggest another iteration – I cannot commit directly to the PR, but if you check the "let maintainers update this" button I'll be able to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> Note: In earlier editions of Rust, the body of an unsafe function was treated
> as an `unsafe` block, so you could perform any unsafe operation within it.
> Starting with the 2024 edition, the compiler will warn you that you need to
> use an `unsafe` block to perform unsafe operations even in the body of an
> unsafe function. This helps to keep `unsafe` blocks as small as possible, as
> unsafe operations may not be needed across the whole function body. For more
> information about Rust editions, refer to [Appendix E][editions].
> Note: Since the Rust 2024 Edition, Rust distinguishes between the body of an
> `unsafe fn` and an `unsafe` block. In earlier editions, the body of an unsafe
> function was treated as an `unsafe` block, so you could perform any unsafe
> operation within it. Starting with the 2024 edition, the compiler will warn
> you that you need to use an `unsafe` block to perform unsafe operations even
> in the body of an unsafe function. This helps to keep `unsafe` blocks as small
> as possible, as unsafe operations may not be needed across the whole function
> body. For more information about Rust editions, refer to [Appendix
> E][editions].

This is one sentence longer, but I think it does what I was wanting to do, i.e. emphasize the current state, which is what we generally aim for in the book, while keeping your original flow overall. I think it is fine to leave the reference to the editions appendix at the end, given the text now says "Edition" or "edition" three separate times before that!

Copy link
Author

@matthias-t matthias-t Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think something like that manages well to avoid the expectation of the feature. However, the back-and-forth gives the note an agitation that lacks the simple, calm style of the book.

Is it very important to you to begin with the newer editions' behavior? I sympathize with the goal to emphasize the current state, and I'd tend to agree that even if Rust never had this feature, it would still be helpful to clarify that unsafe functions' bodies are not unsafe blocks, which some (but likely not all) readers might expect. But I think this comes across with sufficient immediacy in the first sentence in this PR, which then quickly gives way to discussion of the current behavior.

If so, here's a suggestion that does so:

Suggested change
> Note: In earlier editions of Rust, the body of an unsafe function was treated
> as an `unsafe` block, so you could perform any unsafe operation within it.
> Starting with the 2024 edition, the compiler will warn you that you need to
> use an `unsafe` block to perform unsafe operations even in the body of an
> unsafe function. This helps to keep `unsafe` blocks as small as possible, as
> unsafe operations may not be needed across the whole function body. For more
> information about Rust editions, refer to [Appendix E][editions].
> Note: To perform unsafe operations in the body of an unsafe function, you need
> to use an `unsafe` block just as within a regular function.
>
> This helps to keep `unsafe` blocks as small as possible, as unsafe operations
> may not be needed across the whole function body. It was different in editions
> of Rust prior to 2024, where the body of an unsafe function was treated as an
> `unsafe` block. For more information about Rust editions, refer to [Appendix
> E][editions].

Copy link
Contributor

Choose a reason for hiding this comment

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

It is indeed quite important that we lead with the behavior new users will see, since that’s the default experience for someone reading through the book. I like your overall direction, but I think we can go a step further: we can just say that part inline:

To perform unsafe operations in the body of an unsafe function, you still need
to use an `unsafe` block just as within a regular function, and the compiler
will warn you if you forget. This helps to keep `unsafe` blocks as small as
possible, as unsafe operations may not be needed across the whole function body.

We don’t actually need to say anything about the edition-level change, just document the new default behavior. If someone is curious why they are not yet seeing that behavior, they can reference the edition guide etc. (This is true of all such changes to the book across editions!)


#### Creating a Safe Abstraction over Unsafe Code

Expand Down Expand Up @@ -540,4 +539,5 @@ that the code you have written upholds Rust’s rules.
[the-slice-type]: ch04-03-slices.html#the-slice-type
[reference]: ../reference/items/unions.html
[miri]: https://github.com/rust-lang/miri
[editions]: appendix-05-editions.html
[nightly]: appendix-07-nightly-rust.html
Loading