unsafe keyword docs: emphasize that an unsafe fn in a trait does not get to choose its safety contract #141471
Conversation
rustbot
added
S-waiting-on-review
|
@traviscross maybe you could take a look at this? It came out of discussion in #139368 which you also participated in. |
library/std/src/keyword_docs.rs
Outdated
| /// its callers must uphold -- the fact that `idx < LEN`. | ||
| /// to contend with. Note that unlike normal `unsafe fn`, an `unsafe fn` in a trait implementation | ||
| /// does not get to just pick an arbitrary safety contract! It *has* to use the safety contract | ||
| /// defined by the trait (or a stronger contract, i.e., weaker preconditions). |
There was a problem hiding this comment.
| /// defined by the trait (or a stronger contract, i.e., weaker preconditions). | |
| /// defined by the trait or one with strictly weaker preconditions. |
This is both shorter and avoids getting into the question of "from whose perspective is the contract stronger?"
library/std/src/keyword_docs.rs
Outdated
| /// obligations of its callees. (We enabled `unsafe_op_in_unsafe_fn`, so the body of `idx_unchecked` | ||
| /// is not implicitly an unsafe block.) For that purpose it can make use of the contract that all | ||
| /// its callers must uphold -- the fact that `idx < LEN`. | ||
| /// to contend with. Note that unlike normal `unsafe fn`, an `unsafe fn` in a trait implementation |
There was a problem hiding this comment.
but the implementation of
get_uncheckedhas no proof obligation to contend with.
I might suggest a parenthetical here that hints at what will follow in the next paragraph. Almost always, such an implementation will have an unsafe block within.
There was a problem hiding this comment.
Then the next paragraph might as well be removed entirely as that's the only thing it says.
There was a problem hiding this comment.
I reordered things so the point about "there'll be an unsafe block within" comes first. Do you think that is better?
library/std/src/keyword_docs.rs
Outdated
| /// `unsafe` *block* to indicate it discharged the proof obligations of its callees. (We enabled | ||
| /// `unsafe_op_in_unsafe_fn`, so the body of `idx_unchecked` is not implicitly an unsafe block.) For |
There was a problem hiding this comment.
We turned this lint on by default in Rust 2024. This should no longer be written as though it needs to be turned on (there are separately other updates that should be made in this chapter due to this).
There was a problem hiding this comment.
Seems like feature creep to extend my PR to also account for this update :/
This is not new text.
RalfJung
force-pushed
the
unsafe-fn-in-trait
branch
from
e55f37a to
66fd846
Compare
|
Looks good. Thanks @RalfJung. Rebase if you want, then r? traviscross |
…get to choose its safety contract
RalfJung
force-pushed
the
unsafe-fn-in-trait
branch
from
66fd846 to
910a59d
Compare
|
Thanks for the review! @bors r=traviscross rollup |
bors
added
S-waiting-on-bors
Rollup of 11 pull requests Successful merges: - #140418 (Reexport types from `c_size_t` in `std`) - #141471 (unsafe keyword docs: emphasize that an unsafe fn in a trait does not get to choose its safety contract) - #141603 (Reduce `ast::ptr::P` to a typedef of `Box`) - #142043 (Verbose suggestion to make param `const`) - #142086 (duduplicate more AST visitor methods) - #142103 (Update `InterpCx::project_field` to take `FieldIdx`) - #142105 (remove extraneous text) - #142112 (fix typo) - #142113 (Reduce confusion of some drop order tests) - #142114 (Compute number of digits instead of relying on constant value for u128 display code) - #142118 (rustc_lexer: typo fix + small cleanups) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 11 pull requests Successful merges: - rust-lang/rust#140418 (Reexport types from `c_size_t` in `std`) - rust-lang/rust#141471 (unsafe keyword docs: emphasize that an unsafe fn in a trait does not get to choose its safety contract) - rust-lang/rust#141603 (Reduce `ast::ptr::P` to a typedef of `Box`) - rust-lang/rust#142043 (Verbose suggestion to make param `const`) - rust-lang/rust#142086 (duduplicate more AST visitor methods) - rust-lang/rust#142103 (Update `InterpCx::project_field` to take `FieldIdx`) - rust-lang/rust#142105 (remove extraneous text) - rust-lang/rust#142112 (fix typo) - rust-lang/rust#142113 (Reduce confusion of some drop order tests) - rust-lang/rust#142114 (Compute number of digits instead of relying on constant value for u128 display code) - rust-lang/rust#142118 (rustc_lexer: typo fix + small cleanups) r? `@ghost` `@rustbot` modify labels: rollup
Inspired by discussion in #139368.
Cc @hanna-kruppe