Skip to content

[SortedCollections] Name the documented arguments the way the declarations name them - #705

Open
karpovantonme wants to merge 1 commit into
apple:mainfrom
karpovantonme:doc/sorted-collections
Open

[SortedCollections] Name the documented arguments the way the declarations name them#705
karpovantonme wants to merge 1 commit into
apple:mainfrom
karpovantonme:doc/sorted-collections

Conversation

@karpovantonme

@karpovantonme karpovantonme commented Aug 16, 2026

Copy link
Copy Markdown

#701 covers the shipping modules. This is the same thing in SortedCollections, which sits behind the UnstableSortedCollections trait and so is not in a default build

17 doc comments here name an argument that the declaration below them does not have, mostly renames the comment did not follow:

  • SortedDictionary.Keys and .Values carry 8 blocks documenting - Parameter i while every one of those declarations takes index
  • _Node.UnsafeHandle documents newHandle where the argument is target, and newElement where it is newChild
  • _Node.Splinter documents node, the initializer takes leftChild
  • _BTree.Index documents index, the initializer takes offset
  • _BTree+UnsafeCursor documents key where takeCursor(at:) takes index

Where a - Returns line carried the old name too, it came along, so each block reads consistently. index(after:) and index(before:) in Keys and Values returned "the index immediately after i", and takeCursor(at:) returned "a cursor to the key or where the key should be inserted", which is the other overload's behaviour

The i in the neighbouring blocks is left alone on purpose: index(after i:), formIndex(_ i:) and index(_ i:, offsetBy:) really are named that, and only these overloads were renamed without the comments following

One block is removed rather than renamed. _BTree.swift documents "a path to the key at absolute offset" with - Parameter offset, - Returns and - Complexity, then comes a blank line and then the doc comment for startIndex(forKey:), so it binds to nothing

Comments only, no API change. Fine by me to close it if you would rather leave this module alone

Checklist

  • I've read the Contribution Guidelines
  • My contributions are licensed under the Swift license.
  • I've followed the coding style of the rest of the project.
  • I've added tests covering all new code paths my change adds to the project (if appropriate).
  • I've added benchmarks covering new functionality (if appropriate).
  • I've verified that my change does not break any existing tests or introduce unexplained benchmark regressions.
  • I've updated the documentation if necessary.

…tions name them

Same species as apple#701, in the module behind the UnstableSortedCollections
trait. 16 doc comments name an argument the declaration does not have, and
one block documents a function that is no longer there.

The Keys and Values views carry the same four each: - Parameter i where the
argument is index, and - Parameter index on subscript(position:).

In the B-tree internals the descriptions usually name the right thing while
the key does not: - node on toNode(leftChild:) says the argument becomes the
left child, and - newElement on exchangeChild(atSlot:with newChild:) says it
is the new child.

_BTree.swift keeps a doc block for a function that was removed: it documents
an offset parameter and sits above an unrelated declaration.

@lorentey lorentey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@lorentey lorentey added this to the 1.7.0 milestone Aug 18, 2026
@lorentey

lorentey commented Aug 19, 2026

Copy link
Copy Markdown
Member

The failing check is due to a CI issue; #706 will eventually resolve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants