Skip to content

Commit 85d9507

Browse files
authored
Merge pull request swiftlang#39608 from lorentey/readable-structs
2 parents b69eac9 + 87cfcb8 commit 85d9507

32 files changed

+797
-564
lines changed

docs/StandardLibraryProgrammersManual.md

Lines changed: 223 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,232 @@ In this document, "stdlib" refers to the core standard library (`stdlib/public/c
1212

1313
### Formatting Conventions
1414

15-
The stdlib currently has a hard line length limit of 80 characters. To break long lines, please closely follow the indentation conventions you see in the existing codebase. (FIXME: Describe.)
15+
The Standard Library codebase has some uniformly applied formatting conventions. While these aren't currently automatically enforced, we still expect these conventions to be followed in every PR, including draft PRs. (PRs are first and foremost intended to be read/reviewed by people, and it's crucial that trivial formatting issues don't get in the way of understanding proposed changes.)
16+
17+
Some of this code is very subtle, and its presentation matters greatly. Effort spent on getting formatting _just right_ is time very well spent: new code we add is going to be repeatedly read and re-read by many people, and it's important that code is presented in a way that helps understanding it.
18+
19+
#### Line Breaking
20+
21+
The stdlib currently has a hard line length limit of 80 characters. This allows code to be easily read in environments that don't gracefully handle long lines, including (especially!) code reviews on GitHub.
1622

1723
We use two spaces as the unit of indentation. We don't use tabs.
1824

25+
To break long lines, please closely follow the indentation conventions you see in the existing codebase. (FIXME: Describe in detail.)
26+
27+
Our primary rule is that if we need to insert a line break anywhere in the middle of a list (such as arguments, tuple or array/dictionary literals, generic type parameters, etc.), then we must go all the way and put each item on its own line, indented by +1 unit, even if some of the items would fit on a single line together.
28+
29+
The rationale for this is that line breaks tend to put strong visual emphasis on the item that follows them, risking subsequent items on the same line to be glanced over during review. For example, see how easy it is to accidentally miss `arg2` in the second example below.
30+
31+
```swift
32+
// BAD (completely unreadable)
33+
@inlinable public func foobar<Result>(_ arg1: Result, arg2: Int, _ arg3: (Result, Element) throws -> Result) rethrows -> Result {
34+
...
35+
}
36+
37+
// BAD (arg2 is easily missed)
38+
@inlinable
39+
public func foobar<Result>(
40+
_ arg1: Result, arg2: Int, // ☹️
41+
_ arg3: (Result, Element) throws -> Result
42+
) rethrows -> Result {
43+
44+
// GOOD
45+
@inlinable
46+
public func foobar<Result>(
47+
_ arg1: Result,
48+
arg2: Int,
49+
_ arg3: (Result, Element) throws -> Result
50+
) rethrows -> Result {
51+
...
52+
}
53+
```
54+
55+
As a special case, function arguments that are very tightly coupled together are sometimes kept on the same line. The typical example for this is a pair of defaulted file/line arguments that track the caller's source position:
56+
57+
```swift
58+
// OK
59+
internal func _preconditionFailure(
60+
_ message: StaticString = StaticString(),
61+
file: StaticString = #file, line: UInt = #line
62+
) -> Never {
63+
...
64+
}
65+
66+
// Also OK
67+
internal func _preconditionFailure(
68+
_ message: StaticString = StaticString(),
69+
file: StaticString = #file,
70+
line: UInt = #line
71+
) -> Never {
72+
...
73+
}
74+
```
75+
76+
(When in doubt, err on the side of adding more line breaks.)
77+
78+
79+
For lists that have delimiter characters (`(`/`)`, `[`/`]`, `<`/`>`, etc.), we prefer to put a line break both *after* the opening delimiter, and *before* the closing delimiter.
80+
However, within function bodies, it's okay to omit the line break before the closing delimiter.
81+
82+
```swift
83+
// GOOD:
84+
func foo<S: Sequence, T>(
85+
input: S,
86+
transform: (S.Element) -> throws T
87+
) -> [S.Element] { // Note: there *must* be a line break before the ')'
88+
...
89+
someLongFunctionCall(
90+
on: S,
91+
startingAt: i,
92+
stride: 32) // Note: the break before the closing paren is optional
93+
}
94+
```
95+
96+
If the entire contents of a list fit on a single line, it is okay to only break at the delimiters. That said, it is also acceptable to put breaks around each item:
97+
98+
```swift
99+
// GOOD:
100+
@_alwaysEmitIntoClient
101+
internal func _parseIntegerDigits<Result: FixedWidthInteger>(
102+
ascii codeUnits: UnsafeBufferPointer<UInt8>, radix: Int, isNegative: Bool
103+
) -> Result? {
104+
...
105+
}
106+
107+
// ALSO GOOD:
108+
@_alwaysEmitIntoClient
109+
internal func _parseIntegerDigits<Result: FixedWidthInteger>(
110+
ascii codeUnits: UnsafeBufferPointer<UInt8>,
111+
radix: Int,
112+
isNegative: Bool
113+
) -> Result? {
114+
...
115+
}
116+
```
117+
118+
The rules typically don't require breaking lines that don't exceed the length limit; but if you find it helps understanding, feel free to do so anyway.
119+
120+
```swift
121+
// OK
122+
guard let foo = foo else { return false }
123+
124+
// Also OK
125+
guard let foo = foo else {
126+
return false
127+
}
128+
```
129+
130+
Historically, we had a one (1) exception to the line limit, which is that we allowed string literals to go over the margin. Now that Swift has multi-line string literals, we could start breaking overlong ones. However, multiline literals can be a bit heavy visually, while in most cases the string is a precondition failure message, which doesn't necessarily need to be emphasized as much -- so the old exception still applies:
131+
132+
```swift
133+
// OK
134+
_precondition( |
135+
buffer.baseAddress == firstElementAddress, |
136+
"Can't reassign buffer in Array(unsafeUninitializedCapacity:initializingWith:)"
137+
) |
138+
|
139+
// Also OK, although spending 4 lines on the message is a bit much |
140+
_precondition( |
141+
buffer.baseAddress == firstElementAddress, |
142+
"""" |
143+
Can't reassign buffer in \ |
144+
Array(unsafeUninitializedCapacity:initializingWith:) |
145+
"""" |
146+
) |
147+
```
148+
149+
In every other case, long lines must be broken up. We expect this rule to be strictly observed.
150+
151+
152+
#### Presentation of Type Definitions
153+
154+
To ease reading/understanding type declarations, we prefer to define members in the following order:
155+
156+
1. Crucial type aliases and nested types, not exceeding a handful of lines in length
157+
2. Stored properties
158+
3. Initializers
159+
4. Any other instance members (methods, computed properties, etc)
160+
161+
Please keep all stored properties together in a single uninterrupted list, followed immediately by the type's most crucial initializer(s). Put these as close to the top of the type declaration as possible -- we don't want to force readers to scroll around to find these core definitions.
162+
163+
We also have some recommendations for defining other members. These aren't strict rules, as the best way to present definitions varies; but it usually makes sense to break up the implementation into easily digestable, logical chunks.
164+
165+
- In general, it is a good idea to keep the main `struct`/`class` definiton as short as possible: preferably it should consist of the type's stored properties and a handful of critical initializers, and nothing else.
166+
167+
- Everything else should go in standalone extensions, arranged by logical theme. For example, it's often nice to define protocol conformances in dedicated extensions. If it makes sense, feel free to add a comment to title these sectioning extensions.
168+
169+
- Think about what order you present these sections -- put related conformances together, follow some didactic progression, etc. E.g, conformance definitions for closely related protocols such as `Equatable`/`Hashable`/`Comparable` should be kept very close to each other, for easy referencing.
170+
171+
- In some cases, it can also work well to declare the most essential protocol conformances directly on the type definition; feel free to do so if it helps understanding. (You can still implement requirements in separate extensions in this case, or you can do it within the main declaration.)
172+
173+
- It's okay for the core type declaration to forward reference large nested types or static members that are defined in subsequent extensions. It's often a good idea to define these in an extension immediately following the type declaration, but this is not a strict rule.
174+
175+
Extensions are a nice way to break up the implementation into easily digestable chunks, but they aren't the only way. The goal is to make things easy to understand -- if a type is small enough, it may be best to list every member directly in the `struct`/`class` definition, while for huge types it often makes more sense to break them up into a handful of separate source files instead.
176+
177+
```swift
178+
// BAD (a jumbled mess)
179+
struct Foo: RandomAccessCollection, Hashable {
180+
var count: Int { ... }
181+
182+
struct Iterator: IteratorProtocol { /* hundreds of lines */ }
183+
184+
class _Storage { /* even more lines */ }
185+
186+
static func _createStorage(_ foo: Int, _ bar: Double) -> _Storage { ... }
187+
188+
func hash(into hasher: inout Hasher) { ... }
189+
190+
func makeIterator() -> Iterator { ... }
191+
192+
/* more stuff */
193+
194+
init(foo: Int, bar: Double) {
195+
_storage = Self._createStorage(foo, bar)
196+
}
197+
198+
static func ==(left: Self, right: Self) -> Bool { ... }
199+
200+
var _storage: _Storage
201+
}
202+
203+
// GOOD
204+
struct Foo {
205+
var _storage: _Storage
206+
207+
init(foo: Int, bar: Double) { ... }
208+
}
209+
210+
extension Foo {
211+
class _Storage { /* even more lines */ }
212+
213+
static func _createStorage(_ foo: Int, _ bar: Double) -> _Storage { ... }
214+
}
215+
216+
extension Foo: Equatable {
217+
static func ==(left: Self, right: Self) -> Bool { ... }
218+
}
219+
220+
extension Foo: Hashable {
221+
func hash(into hasher: inout Hasher) { ... }
222+
}
223+
224+
extension Foo: Sequence {
225+
struct Iterator: IteratorProtocol { /* hundreds of lines */ }
226+
227+
func makeIterator() -> Iterator { ... }
228+
...
229+
}
230+
231+
extension Foo: RandomAccessCollection {
232+
var count: Int { ... }
233+
...
234+
}
235+
236+
extension Foo {
237+
/* more stuff */
238+
}
239+
```
240+
19241
### Public APIs
20242
21243
#### Core Standard Library

stdlib/public/core/ArrayBuffer.swift

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ internal typealias _ArrayBridgeStorage
2525
@usableFromInline
2626
@frozen
2727
internal struct _ArrayBuffer<Element>: _ArrayBufferProtocol {
28+
@usableFromInline
29+
internal var _storage: _ArrayBridgeStorage
30+
31+
@inlinable
32+
internal init(storage: _ArrayBridgeStorage) {
33+
_storage = storage
34+
}
2835

2936
/// Create an empty buffer.
3037
@inlinable
@@ -74,15 +81,6 @@ internal struct _ArrayBuffer<Element>: _ArrayBufferProtocol {
7481
// NSArray's need an element typecheck when the element type isn't AnyObject
7582
return !_isNativeTypeChecked && !(AnyObject.self is Element.Type)
7683
}
77-
78-
//===--- private --------------------------------------------------------===//
79-
@inlinable
80-
internal init(storage: _ArrayBridgeStorage) {
81-
_storage = storage
82-
}
83-
84-
@usableFromInline
85-
internal var _storage: _ArrayBridgeStorage
8684
}
8785

8886
extension _ArrayBuffer {

stdlib/public/core/BridgingBuffer.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
internal struct _BridgingBufferHeader {
14-
internal init(_ count: Int) { self.count = count }
1514
internal var count: Int
15+
16+
internal init(_ count: Int) { self.count = count }
1617
}
1718

1819
// NOTE: older runtimes called this class _BridgingBufferStorage.

stdlib/public/core/Codable.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3369,8 +3369,8 @@ public enum DecodingError: Error {
33693369
// The following extensions allow for easier error construction.
33703370

33713371
internal struct _GenericIndexKey: CodingKey, Sendable {
3372-
internal var stringValue: String
3373-
internal var intValue: Int?
3372+
internal var stringValue: String
3373+
internal var intValue: Int?
33743374

33753375
internal init?(stringValue: String) {
33763376
return nil

stdlib/public/core/Collection.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,18 +72,18 @@ public struct IndexingIterator<Elements: Collection> {
7272
@usableFromInline
7373
internal var _position: Elements.Index
7474

75+
/// Creates an iterator over the given collection.
7576
@inlinable
7677
@inline(__always)
77-
/// Creates an iterator over the given collection.
7878
public /// @testable
7979
init(_elements: Elements) {
8080
self._elements = _elements
8181
self._position = _elements.startIndex
8282
}
8383

84+
/// Creates an iterator over the given collection.
8485
@inlinable
8586
@inline(__always)
86-
/// Creates an iterator over the given collection.
8787
public /// @testable
8888
init(_elements: Elements, _position: Elements.Index) {
8989
self._elements = _elements

0 commit comments

Comments
 (0)