[stdlib] Documentation revisions #11249
Conversation
|
@swift-ci Please smoke test |
| /// | ||
| /// This relation is a refinement of the less-than-or-equal-to operator | ||
| /// (`<=`) that provides a total order on all values of the type, including | ||
| /// noncanonical encodings, signed zeros, and NaNs. Because it is used much |
There was a problem hiding this comment.
Actually, this isn't quite right. Per the IEEE specification:
NOTE—totalOrder does not impose a total ordering on all encodings in a format. In particular, it does not distinguish among different encodings of the same floating-point representation, as when one or both encodings are non-canonical.
There was a problem hiding this comment.
How would you feel about actually documenting how the total ordering works here?
There was a problem hiding this comment.
Hmmm. So I know that our implementation does distinguish between encodings, so what we're saying here that isTotallyOrdered imposes the IEEE total order relationship and further imposes a total order even on noncanonical encodings.
Is the actual order mandated by the spec? If it's not I don't know that we want to guarantee a specific ordering of the various elements. It's not immediately apparent from the implementation what ends up where.
There was a problem hiding this comment.
The actual order is mandated by the specification in gory detail. The default implementation for BinaryFloatingPoint implements exactly the spec. The result is that some encodings of the same noncanonical encodings may be distinguished from each other, but not all.
There was a problem hiding this comment.
Thanks, got it! Updated that and then noticed that the example (a) had the wrong parameter label and (b) didn't have the correct semantics... 🙄 Going to come back to the Q about specifying the order later.
There was a problem hiding this comment.
I actually wish that method was static func areInIncreasingTotalOrder(_ x: Self, _ y: Self) -> Bool—would play more nicely with sorting.
There was a problem hiding this comment.
Hmm, I see your point, but IEEE totalOrder actually can't be used as-is as a Swift sorting predicate: it is required to return true if two values are equal, whereas areInIncreasingOrder should return false.
| /// (`<=`) that provides a total order on all values of the type, including | ||
| /// noncanonical encodings, signed zeros, and NaNs. Because it is used much | ||
| /// less frequently than the usual comparisons, there is no operator form of | ||
| /// this relation. |
There was a problem hiding this comment.
I don't think justification is required; in general, we should need to justify why certain facilities are operators and not why others are not.
By including this statement, I worry that you're prompting people to wonder whether they actually want to use this operation more often than they thought, as the implication is that an operator spelling was a real possibility that was considered and rejected.
There was a problem hiding this comment.
That's a good point. I think this language might have originated in the floating-point protocols proposal, where it made more sense. I'll look at removing it.
natecook1000
force-pushed
the
nc-rev-77-1
branch
from
f367e86
to
a00c713
Compare
- Revisions to unsafeDowncast and withVaList - Fix the Int64/UInt64 discussion - Buffer pointer revisions - Fix Optional example to use new integer methods - Revise and correct some UnsafeRawBufferPointer docs - Fix symmetricDifference examples - Fix wording in FloatingPoint.nextDown - Update ImplicitlyUnwrappedOptional - Clarify elementsEqual - Minor integer doc fixes - Comment for _AppendKeyPath - Clarification re collection indices - Revise RangeExpression.relative(to:) - Codable revisions
- Clarify StringProtocol conformance - Deprecate ExpressibleByStringInterpolation - String index conversions docs - Describe shared string indices
natecook1000
force-pushed
the
nc-rev-77-1
branch
from
a00c713
to
8329ca9
Compare
|
@swift-ci Please smoke test |
1 similar comment
|
@swift-ci Please smoke test |
| /// If the index passed as `sourcePosition` represents the start of an | ||
| /// extended grapheme cluster---the element type of a string---then the | ||
| /// initializer succeeds. If the index instead represents the position of a | ||
| /// Unicode scalar within an extended grapheme cluster or the position of an |
There was a problem hiding this comment.
I don't think this wording needs to have anything to do with Unicode scalars. The semantics are specifically that if the index lies on a grapheme boundary, then it is valid. Non-grapheme boundary indices (whether on scalar boundaries or sub-scalar indices) return nil.
edit: typos
There was a problem hiding this comment.
One thing that's weird is that the position of a Unicode scalar that is a valid start of a grapheme cluster will convert, even if it's inside what the string is treating as an extended cluster. For example:
let astro = "👩🚀"
// astro.count == 1
let i = astro.unicodeScalars.index(astro.unicodeScalars.startIndex, offsetBy: 2)
// astro[i] == "🚀"
let j = String.Index(i, within: astro)!
// astro[j] == "🚀"
// astro[astro.startIndex] == "👩🚀"That seems a little odd to me, though it still agrees with the letter of the documentation. We'd have to introduce some kind of backtracking scheme (or maybe even start from the beginning of the string again?) to have it not do this. That could easily end up killing the performance for this kind of check…
| /// Initializes the buffer's memory with the given elements, binding the | ||
| /// initialized memory to the elements' type. | ||
| /// | ||
| /// When calling the `initialize(as:from:)` method on a buffer `b`, the |
There was a problem hiding this comment.
Do you mean initializeMemory(as:from:)?
There was a problem hiding this comment.
Definitely, thanks for catching that!
[stdlib] Documentation revisions
These revisions touch on a variety of areas, mostly focusing on recent changes to the stdlib.