refactor(allocator/vec): clarify comment

[overlookmotel]

Follow-on after #10884. Clarify comment as to why as u32 is valid here.

[overlookmotel]

• perf(allocator/vec): reduce pointer maths in Vec::extend_desugared #11087
• perf(allocator/vec): Vec::extend_desugared set len only at end #11082
• refactor(allocator/vec): access len and cap fields via getters/setters #11081
• refactor(allocator/vec): re-order methods #11080
• perf(allocator/vec): remove SetLenOnDrop #11079
• refactor(allocator/vec): limit scope of #[expect(clippy::cast_possible_truncation)] #11074
• perf(allocator/vec): remove alloc_guard from RawVec::from_raw_parts_in + clarify safety docs #11073
• docs(allocator/vec): correct safety comments for RawVec::append_elements #11072
• refactor(allocator/vec): clarify comment #11071 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codspeed-hq]

CodSpeed Instrumentation Performance Report

Merging #11071 will not alter performance

_{Comparing 05-16-refactor_allocator_vec_clarify_comment (fc2f040) with main (ae15046)}

Summary

✅ 36 untouched benchmarks

[Dunqing]

Thank you! I understand a little bit how to write a caller guarantee related safety comment

[overlookmotel]

I don't know if this helps, but the way I conceptualize it is:

Unsafe functions have a "contract". They require the caller to ensure the terms of the contract are always upheld. In return, the unsafe function promises that UB can't happen, if the caller keeps their side of the bargain.

Like a real-world legal contract, both sides (the function and its caller) need to understand the terms of the contract, and agree to them. When the caller calls the function in an unsafe { ... } block, it "signs" the contract, stating "I agree to these terms, and I solemnly promise never to break them."

The safety comments represent the contract and the signing of it.

• /// # SAFETY comment on the unsafe function is the contract - it should explain exactly what are the terms of that contract.
• // SAFETY: ... comment on code which calls the unsafe function is the "contract signing ceremony" - it should explain exactly how it can be sure that it'll always uphold the terms of the contract in all circumstances.

In addition:

Both safe and unsafe functions can provide guarantees, which should also be documented. e.g. Vec::reserve guarantees that after you call it with n, the Vec will have space for at least n additional elements. If it can't fulfill that promise, the doc comment should explain what it'll do (e.g. return Option::None, or panic).

When code calls an unsafe function, it can list the guarantees other functions provided as "evidence" that it's impossible it will ever break the safety contract.

---

Note: What makes a function unsafe is not that it includes unsafe code. What makes it an unsafe function is that it requires the caller to sign a contract.

Here's a silly example:

pub struct FourBytes {
    bytes: [u8; 4],
    /// Current index.
    /// SAFETY: Must always be less than 4.
    current_index: usize,
}

impl FourBytes {
    pub fn new() -> Self {
        // SAFETY: `current_index: 0` satisfies safety constraint
        // of the type that `current_index < 4`.
        Self { bytes: [11, 22, 33, 44], current_index: 0 }
    }

    /// Set current index, without bounds checks.
    ///
    /// # SAFETY
    /// `index` must be less than 4.
    unsafe fn set_current_index_unchecked(&mut self, index: usize) {
        self.current_index = index;
    }

    /// Get the current byte.
    fn get_current(&self) -> u8 {
        // SAFETY: `new` and `set_current_index_unchecked`
        // are the only methods which set `current_index`.
        // Both ensure `current_index` is never greater than 4,
        // so reading `self.bytes[self.current_index]` cannot be out of bounds.
        unsafe {
            *self.bytes.get_unchecked(self.current_index)
        }
    }
}

set_current_index_unchecked contains no unsafe code, but it's the method which is unsafe, because it is the one which requires the caller to "sign a contract".

get_current does contain unsafe code, but is not an unsafe function because it imposes no contract on the caller. It's soundness relies on the guarantees of the type / other methods.

[overlookmotel]

Merge activity

• May 17, 6:15 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• May 17, 6:15 AM EDT: overlookmotel added this pull request to the Graphite merge queue.
• May 17, 6:22 AM EDT: Merged by the Graphite merge queue.

[Dunqing]

I don't know if this helps, but the way I conceptualize it is:

Unsafe functions have a "contract". They require the caller to ensure the terms of the contract are always upheld. In return, the unsafe function promises that UB can't happen, if the caller keeps their side of the bargain.

Like a real-world legal contract, both sides (the function and its caller) need to understand the terms of the contract, and agree to them. When the caller calls the function in an unsafe { ... } block, it "signs" the contract, stating "I agree to these terms, and I solemnly promise never to break them."

The safety comments represent the contract and the signing of it.

• /// # SAFETY comment on the unsafe function is the contract - it should explain exactly what are the terms of that contract.
• // SAFETY: ... comment on code which calls the unsafe function is the "contract signing ceremony" - it should explain exactly how it can be sure that it'll always uphold the terms of the contract in all circumstances.

In addition:

Both safe and unsafe functions can provide guarantees, which should also be documented. e.g. Vec::reserve guarantees that after you call it with n, the Vec will have space for at least n additional elements. If it can't fulfill that promise, the doc comment should explain what it'll do (e.g. return Option::None, or panic).

When code calls an unsafe function, it can list the guarantees other functions provided as "evidence" that it's impossible it will ever break the safety contract.

Note: What makes a function unsafe is not that it includes unsafe code. What makes it an unsafe function is that it requires the caller to sign a contract.

Here's a silly example:

pub struct FourBytes {
    bytes: [u8; 4],
    /// Current index.
    /// SAFETY: Must always be less than 4.
    current_index: usize,
}

impl FourBytes {
    pub fn new() -> Self {
        // SAFETY: `current_index: 0` satisfies safety constraint
        // of the type that `current_index < 4`.
        Self { bytes: [11, 22, 33, 44], current_index: 0 }
    }

    /// Set current index, without bounds checks.
    ///
    /// # SAFETY
    /// `index` must be less than 4.
    unsafe fn set_current_index_unchecked(&mut self, index: usize) {
        self.current_index = index;
    }

    /// Get the current byte.
    fn get_current(&self) -> u8 {
        // SAFETY: `new` and `set_current_index_unchecked`
        // are the only methods which set `current_index`.
        // Both ensure `current_index` is never greater than 4,
        // so reading `self.bytes[self.current_index]` cannot be out of bounds.
        unsafe {
            *self.bytes.get_unchecked(self.current_index)
        }
    }
}

set_current_index_unchecked contains no unsafe code, but it's the method which is unsafe, because it is the one which requires the caller to "sign a contract".

get_current does contain unsafe code, but is not an unsafe function because it imposes no contract on the caller. It's soundness relies on the guarantees of the type / other methods.

I finally found time to review your comment. It's very helpful, for sure! Your explanation and example are very easy to understand! I am looking forward to writing more quite safe "unsafe code" 😂