Assorted allocator nitpicks - #161115
Conversation
rustbot
added
S-waiting-on-review
|
r=me minus const bound note. (either remove unused const bound on both, or constify both) |
|
New changes look good too, but will hold merge in case you have anything else to add. (r=me) |
This comment has been minimized.
This comment has been minimized.
| /// each other. | ||
| /// | ||
| /// The following conditions are sufficient conditions for allocators to be equivalent. | ||
| /// The following conditions are necessary for allocators to be equivalent: |
There was a problem hiding this comment.
"sufficient conditions" is correct. There are other ways for allocators to become equivalent: a library can just "declare" extra equivalences.
There was a problem hiding this comment.
but such an equivalence still needs to uphold the points below, no? are we saying a library can declare e.g. a nontransitive equivalence?
There was a problem hiding this comment.
As per conversation elsewhere, we've agreed on the wording of "Users of allocators may assume the following are true of equivalent allocators, and implementors must ensure these rules are upheld:"
There was a problem hiding this comment.
Maybe something like: "Allocators must be equivalent if all the following conditions hold"
Sufficiency is correct, but the ambiguity is that the combination of all of them is sufficient, and not just any individual point by itself. In other words, all of the conditions are necessary if you want to use this method to ensure equivalence, but you can also just say they're equivalent otherwise.
| /// * All (equivalent) allocators that this memory block is allocated with, | ||
| /// each has one of the following happen to them: | ||
| /// * *All* (equivalent) allocators that this memory block is allocated with has one of | ||
| /// the following happen to it: |
There was a problem hiding this comment.
My initial wording, with "each", was intentional — I wanted to specify that "one allocator gets dropped while another gets mutated" is enough for the memory to be invalidated.
| /// the following happen to it: | ||
| /// * The allocator's destructor runs. | ||
| /// * The allocator is mutated through public API taking `&mut` access. | ||
| /// * The allocator is mutated through an untrusted API taking `&mut` access. |
There was a problem hiding this comment.
My previous wording was specifying that: if an allocator has a public method that doesn't specify what it does, then it must be assumed to potentially invalidate the allocations.
This comment has been minimized.
This comment has been minimized.
| /// Some of the methods require that a `layout` *fits* a memory block or vice versa. This means | ||
| /// that the following conditions must hold: | ||
| /// * the memory block must be *currently allocated* with alignment of [`layout.align()`], and | ||
| /// * [`layout.size()`] must fall in the range `min ..= max`, where: | ||
| /// - `min` is the size of the layout used to allocate the block, and | ||
| /// - `max` is the actual size returned from [`allocate`], [`allocate_zeroed`], | ||
| /// [`grow`], [`grow_zeroed`], or [`shrink`]. |
There was a problem hiding this comment.
is it worth reminding users here that max <= isize::MAX? actually not sure this matters here specifically tbh
6 participants
Small things that got missed in #157428, doc language cleanup for allocator, and a rename that closes #158344. cc @rust-lang/wg-allocators. pending libs bikeshed decision on the naming of
into_raw_parts_with_allocr? clarfonthey