unsafe-deep-dive: replace may_overflow with a real safety precondition - #3244
unsafe-deep-dive: replace may_overflow with a real safety precondition#3244rmyndharis wants to merge 1 commit into
Conversation
The "Example: may_overflow" slide taught that signed integer overflow is undefined behavior in release mode and that the function therefore needs the `unsafe` keyword. Both claims are false: integer overflow is well defined in Rust (debug builds panic, release builds wrap), and `a + i32::MAX` does not require `unsafe` at all. Replace the example with an `unsafe fn element_at` built on `slice::get_unchecked`, which has a genuine safety precondition: the caller must keep the index in bounds. This preserves the slide's purpose (a concrete example of responsibility shifting to the programmer) and leads directly into the "Safety Preconditions" segment. The speaker notes state plainly that integer overflow is defined behavior and never requires `unsafe`. Closes google#3122.
| /// | ||
| /// The caller must guarantee that `index < slice.len()`. | ||
| unsafe fn element_at(slice: &[i32], index: usize) -> i32 { | ||
| unsafe { *slice.get_unchecked(index) } |
There was a problem hiding this comment.
Could you add a // Safety comment here as well to show how this unsafe blocks needs a proof - and that we can only prove safety by relying on the # Safety block attached to the function itself.
We show this pattern of shifting the responsibility to the caller in the part about bare-metal Rust, but I think it's good to show it here again.
There was a problem hiding this comment.
Cc @gribozavr and @qwandor, may I suggest that you find someone from Google with strong experience in unsafe Rust to review these slides critically? Perhaps someone in the old 20% group would be up for this?
There was a problem hiding this comment.
Ah, I see @ia0 alrady opened #3021 with a large list of things to fix.
I am unfortunately rather busy at work these days, but I would like to send out PRs for this — or let an LLM do it, rather, as I'm sure they're rather simple to fix now that the robot knows where to look. If someone gets to it before me, by all means!
mgeisler
left a comment
mgeisler
left a comment
There was a problem hiding this comment.
Thanks a lot for cleaning up this big misconception!
I'm happy to merge this as-is, my point about adding another // Safety (or // SAFETY as I normally see) comment can come later.
| @@ -1 +1 @@ | |||
| --- | |||
There was a problem hiding this comment.
The file should also be renamed, it is no longer about overflowing.
3 participants
The "Example: may_overflow" slide taught that signed integer overflow is
undefined behavior in release mode and that the function therefore needs
the
unsafekeyword. Both claims are false: integer overflow is welldefined in Rust (debug builds panic, release builds wrap), and
a + i32::MAXdoes not requireunsafeat all.Replace the example with an
unsafe fn element_atbuilt onslice::get_unchecked, which has a genuine safety precondition: thecaller must keep the index in bounds. This preserves the slide's purpose
(a concrete example of responsibility shifting to the programmer) and
leads directly into the "Safety Preconditions" segment. The speaker notes
state plainly that integer overflow is defined behavior and never requires
unsafe.Closes #3122.