[stdlib] Revise unsafe pointers documentation #6275
Conversation
This revises and expands upon documentation for the standard library's unsafe pointer types. This includes typed and raw pointers and buffers, the MemoryLayout type, and some other top-level functions.
|
@swift-ci Please smoke test |
There was a problem hiding this comment.
A few thoughts from a quick read over while busing. I like the way you have really expanded the documentation.
My comments are mainly related to some places where there is a "jump in thought" that could be made explicit or clearer. I understood what you were trying to say, but I had to make an inference. IMO technical docs should minimize such inference as much as possible in the favor of absolute clarity.
+1!
| /// | ||
| /// - To convert an integer value from one type to another, use an initializer |
There was a problem hiding this comment.
IMO, this is a bit of a jump. I think you need a connective that communicates that you are provided a list of "other means" for common operations when people instinctively reach for unsafeBitCast.
There is no reason not to be more explicit... thoughts?
There was a problem hiding this comment.
That's a good note, thanks!
| /// The result does not include any dynamically allocated or remote | ||
| /// storage. In particular, `MemoryLayout.size(ofValue: x)`, when `x` is an | ||
| /// instance of a class `C`, is the same regardless of how many stored | ||
| /// properties `C` has. |
There was a problem hiding this comment.
To me this is a bit weird. It is skirting around a class being a pointer. Why not just call that out in a subsequent sentence? Perhaps use the term "out of line storage"? Just a thought.
| /// | ||
| /// - Complexity: O(1). | ||
| /// - Returns: An iterator over the elements of this buffer. |
There was a problem hiding this comment.
Do we really want to get rid of the Complexity marker here?
There was a problem hiding this comment.
That's the expected complexity for makeIterator()—we only document when it deviates.
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test Linux platform |
| /// `Pointee` type. To access the stride, use | ||
| /// `MemoryLayout<Pointee>.stride`. | ||
| /// | ||
| /// - SeeAlso: `MemoryLayout` |
There was a problem hiding this comment.
@atrick Do we need to say something about pointer alignment here? If lhs and rhs aren't properly aligned the result won't accurately describe the distance between them. (The same also applies to distance(to:).) Maybe something like:
If the two pointers are not properly aligned for the
Pointeetype, the result oflhs - rhsmay not accurately measure the distance between them. To find the distance in bytes between two pointers, convert them toUnsafeRawPointerinstances before subtracting.
There was a problem hiding this comment.
The requirement seems redundant since any instance of these typed pointers already need to be aligned. Although I don't see any debug alignment checks, so it doesn't hurt to reiterate this requirement. Can you word it in a way that doesn't sound specific to this operator: e.g. "The two pointers are required to be properly aligned for their Pointee type. This ensures that the result accurately measures the distance..."
| // to avoid a hang in Foundation, which has the following setup: | ||
| // struct A { struct B { let x: UnsafeMutableBufferPointer<...> } let b: B } | ||
| /// You can use an `${Self}` instance in low level operations to eliminate | ||
| /// uniqueness checks and release mode bounds checks. Bounds checks are always |
There was a problem hiding this comment.
"release mode bounds checks" -> "bounds checks in release mode" maybe?
|
@swift-ci Please smoke test and merge |
|
Both tests passed, but the bot had an error during its call to merge: https://ci.swift.org/job/swift-PR-smoke-merge/1454/consoleText |
This revises and expands upon documentation for the standard library's
unsafe pointer types. This includes typed and raw pointers and buffers,
the MemoryLayout type, and some other top-level functions.