From 8fb36bd37cb8860785e0a743276c49789da38027 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Wed, 14 Jul 2021 16:33:48 +0200 Subject: [PATCH 1/2] Add SAFETY comments presentation --- src/documentation/safety-comments.md | 79 ++++++++++++++++++++++++++++ src/documentation/summary.md | 1 + 2 files changed, 80 insertions(+) create mode 100644 src/documentation/safety-comments.md diff --git a/src/documentation/safety-comments.md b/src/documentation/safety-comments.md new file mode 100644 index 0000000..c13ae96 --- /dev/null +++ b/src/documentation/safety-comments.md @@ -0,0 +1,79 @@ +# Safety comments + +Using [`unsafe`] blocks in often required in the Rust compiler or standard +library, but this is not done without rules: each `unsafe` block should have +a `SAFETY:` comment explaining why the block is safe, which invariants are +used and must be respected. Below are some examples taken from the standard +library: + +[`unsafe`]: https://doc.rust-lang.org/stable/std/keyword.unsafe.html + +## Inside `unsafe` elements + +This one shows how an `unsafe` function can pass the requirements through to its +caller with the use of documentation in a `# Safety` section while still having +more invariants needed that are not required from callers. `clippy` has a +lint for `# Safety` sections by the way. + +```rust +/// Converts a mutable string slice to a mutable byte slice. +/// +/// # Safety +/// +/// The caller must ensure that the content of the slice is valid UTF-8 +/// before the borrow ends and the underlying `str` is used. +/// +/// Use of a `str` whose contents are not valid UTF-8 is undefined behavior. +/// +/// ... +pub unsafe fn as_bytes_mut(&mut self) -> &mut [u8] { + // SAFETY: the cast from `&str` to `&[u8]` is safe since `str` + // has the same layout as `&[u8]` (only libstd can make this guarantee). + // The pointer dereference is safe since it comes from a mutable reference which + // is guaranteed to be valid for writes. + unsafe { &mut *(self as *mut str as *mut [u8]) } +} +``` + +[See the function on github][as_bytes_mut] + +This example is for a function but the same principle applies to `unsafe trait`s +like [`Send`] or [`Sync`] for example, though they have no `# Safety` section +since their entire documentation is about why they are `unsafe`. + +Note that in the Rust standard library, [`unsafe_op_in_unsafe_fn`] is active +and so each `unsafe` operation in an `unsafe` function must be enclosed in an +`unsafe` block. This makes it easier to review such functions and to document +their `unsafe` parts. + +[`Send`]: https://doc.rust-lang.org/stable/std/marker/trait.Send.html +[`Sync`]: https://doc.rust-lang.org/stable/std/marker/trait.Sync.html +[as_bytes_mut]: https://github.com/rust-lang/rust/blob/a08f25a7ef2800af5525762e981c24d96c14febe/library/core/src/str/mod.rs#L278 +[`unsafe_op_in_unsafe_fn`]: https://doc.rust-lang.org/rustc/lints/listing/allowed-by-default.html#unsafe-op-in-unsafe-fn + +## Inside *safe* elements + +Inside safe elements, a `SAFETY:` comment must not depend on anything from the +caller beside properly contruscted types and values (i.e, if your function +receive a reference that is unaligned or null, it's the caller fault if it fails +and not yours). + +`SAFETY` comments in *safe* elements often rely on checks that are done before +the `unsafe` block or on type invariants, like a division by `NonZeroU8` would +not check for `0` before dividing. + +```rust +pub fn split_at(&self, mid: usize) -> (&str, &str) { + // is_char_boundary checks that the index is in [0, .len()] + if self.is_char_boundary(mid) { + // SAFETY: just checked that `mid` is on a char boundary. + unsafe { (self.get_unchecked(0..mid), self.get_unchecked(mid..self.len())) } + } else { + slice_error_fail(self, 0, mid) + } +} +``` + +[See the function on github][split_at] + +[split_at]: https://github.com/rust-lang/rust/blob/a08f25a7ef2800af5525762e981c24d96c14febe/library/core/src/str/mod.rs#L570 diff --git a/src/documentation/summary.md b/src/documentation/summary.md index 1fb6e93..686bcac 100644 --- a/src/documentation/summary.md +++ b/src/documentation/summary.md @@ -1 +1,2 @@ - [doc alias policy](./doc-alias-policy.md) +- [safety comments policy](./safety-comments.md) From d91d00502f877ed94f5958832ab93b06bbc3169a Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sat, 17 Jul 2021 14:38:27 +0200 Subject: [PATCH 2/2] Fix typos, move example link above the example --- src/documentation/safety-comments.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/documentation/safety-comments.md b/src/documentation/safety-comments.md index c13ae96..5cbe19d 100644 --- a/src/documentation/safety-comments.md +++ b/src/documentation/safety-comments.md @@ -1,6 +1,6 @@ # Safety comments -Using [`unsafe`] blocks in often required in the Rust compiler or standard +Using [`unsafe`] blocks is often required in the Rust compiler or standard library, but this is not done without rules: each `unsafe` block should have a `SAFETY:` comment explaining why the block is safe, which invariants are used and must be respected. Below are some examples taken from the standard @@ -15,6 +15,8 @@ caller with the use of documentation in a `# Safety` section while still having more invariants needed that are not required from callers. `clippy` has a lint for `# Safety` sections by the way. +[See the example on github][as_bytes_mut] + ```rust /// Converts a mutable string slice to a mutable byte slice. /// @@ -35,8 +37,6 @@ pub unsafe fn as_bytes_mut(&mut self) -> &mut [u8] { } ``` -[See the function on github][as_bytes_mut] - This example is for a function but the same principle applies to `unsafe trait`s like [`Send`] or [`Sync`] for example, though they have no `# Safety` section since their entire documentation is about why they are `unsafe`. @@ -54,14 +54,16 @@ their `unsafe` parts. ## Inside *safe* elements Inside safe elements, a `SAFETY:` comment must not depend on anything from the -caller beside properly contruscted types and values (i.e, if your function -receive a reference that is unaligned or null, it's the caller fault if it fails -and not yours). +caller beside properly constructed types and values (i.e, if your function +receives a reference that is unaligned or null, it is the caller fault if it +fails and not yours). `SAFETY` comments in *safe* elements often rely on checks that are done before the `unsafe` block or on type invariants, like a division by `NonZeroU8` would not check for `0` before dividing. +[See the example on github][split_at] + ```rust pub fn split_at(&self, mid: usize) -> (&str, &str) { // is_char_boundary checks that the index is in [0, .len()] @@ -74,6 +76,4 @@ pub fn split_at(&self, mid: usize) -> (&str, &str) { } ``` -[See the function on github][split_at] - [split_at]: https://github.com/rust-lang/rust/blob/a08f25a7ef2800af5525762e981c24d96c14febe/library/core/src/str/mod.rs#L570