[stdlib][NFC] Put stored properties & primary initializers before other members in all struct declarations

docs/StandardLibraryProgrammersManual.md

@@ -12,10 +12,232 @@ In this document, "stdlib" refers to the core standard library (`stdlib/public/c
 
 ### Formatting Conventions
 
-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.)
+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.)
+
+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.
+
+#### Line Breaking
+
+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.
 
 We use two spaces as the unit of indentation. We don't use tabs.
 
+To break long lines, please closely follow the indentation conventions you see in the existing codebase. (FIXME: Describe in detail.)
+
+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.
+
+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.
+
+```swift
+// BAD (completely unreadable)
+@inlinable public func foobar<Result>(_ arg1: Result, arg2: Int, _ arg3: (Result, Element) throws -> Result) rethrows -> Result {
+  ...
+}
+
+// BAD (arg2 is easily missed)
+@inlinable 
+public func foobar<Result>(
+  _ arg1: Result, arg2: Int,             // ☹️
+  _ arg3: (Result, Element) throws -> Result
+) rethrows -> Result {
+
+// GOOD
+@inlinable
+public func foobar<Result>(
+  _ arg1: Result, 
+  arg2: Int, 
+  _ arg3: (Result, Element) throws -> Result
+) rethrows -> Result {
+  ...
+}
+```
+
+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:
+
+```swift
+// OK
+internal func _preconditionFailure(
+  _ message: StaticString = StaticString(),
+  file: StaticString = #file, line: UInt = #line
+) -> Never {
+  ...
+}
+
+// Also OK
+internal func _preconditionFailure(
+  _ message: StaticString = StaticString(),
+  file: StaticString = #file, 
+  line: UInt = #line
+) -> Never {
+  ...
+}
+```
+
+(When in doubt, err on the side of adding more line breaks.)
+
+
+For lists that have delimiter characters (`(`/`)`, `[`/`]`, `<`/`>`, etc.), we prefer to put a line break both *after* the opening delimiter, and *before* the closing delimiter.
+However, within function bodies, it's okay to omit the line break before the closing delimiter.
+
+```swift
+// GOOD:
+func foo<S: Sequence, T>(
+  input: S,
+  transform: (S.Element) -> throws T
+) -> [S.Element] {     // Note: there *must* be a line break before the ')'
+  ...
+  someLongFunctionCall(
+    on: S,
+    startingAt: i,
+    stride: 32)        // Note: the break before the closing paren is optional
+}
+```
+
+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:
+
+```swift
+// GOOD:
+@_alwaysEmitIntoClient
+internal func _parseIntegerDigits<Result: FixedWidthInteger>(
+  ascii codeUnits: UnsafeBufferPointer<UInt8>, radix: Int, isNegative: Bool
+) -> Result? {
+  ...
+}
+
+// ALSO GOOD:
+@_alwaysEmitIntoClient
+internal func _parseIntegerDigits<Result: FixedWidthInteger>(
+  ascii codeUnits: UnsafeBufferPointer<UInt8>, 
+  radix: Int, 
+  isNegative: Bool
+) -> Result? {
+  ...
+}
+```
+
+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.
+
+```swift
+// OK
+guard let foo = foo else { return false }
+
+// Also OK
+guard let foo = foo else {
+  return false
+}
+```
+
+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:
+
+```swift
+      // OK
+      _precondition(                                                           |
+        buffer.baseAddress == firstElementAddress,                             |
+        "Can't reassign buffer in Array(unsafeUninitializedCapacity:initializingWith:)"
+      )                                                                        |
+                                                                               |
+      // Also OK, although spending 4 lines on the message is a bit much       |
+      _precondition(                                                           |
+        buffer.baseAddress == firstElementAddress,                             |
+        """"                                                                   |
+        Can't reassign buffer in \                                             |
+        Array(unsafeUninitializedCapacity:initializingWith:)                   |
+        """"                                                                   |
+      )                                                                        |
+```
+
+In every other case, long lines must be broken up. We expect this rule to be strictly observed.
+
+
+#### Presentation of Type Definitions
+
+To ease reading/understanding type declarations, we prefer to define members in the following order:
+
+1. Crucial type aliases and nested types, not exceeding a handful of lines in length
+2. Stored properties
+3. Initializers
+4. Any other instance members (methods, computed properties, etc)
+
+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.
+
+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.
+
+- 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. 
+
+- 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.
+
+- 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.
+
+- 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.)
+
+- 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. 
+
+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. 
+
+```swift
+// BAD (a jumbled mess)
+struct Foo: RandomAccessCollection, Hashable {
+  var count: Int { ... }
+
+  struct Iterator: IteratorProtocol { /* hundreds of lines */ }
+
+  class _Storage { /* even more lines */ }
+
+  static func _createStorage(_ foo: Int, _ bar: Double) -> _Storage { ... }
+
+  func hash(into hasher: inout Hasher) { ... }
+
+  func makeIterator() -> Iterator { ... }
+
+  /* more stuff */
+
+  init(foo: Int, bar: Double) { 
+    _storage = Self._createStorage(foo, bar) 
+  }
+
+  static func ==(left: Self, right: Self) -> Bool { ... }
+
+  var _storage: _Storage
+}
+
+// GOOD
+struct Foo {
+  var _storage: _Storage
+
+  init(foo: Int, bar: Double) { ... }
+}
+
+extension Foo {
+  class _Storage { /* even more lines */ }
+
+  static func _createStorage(_ foo: Int, _ bar: Double) -> _Storage { ... }
+}
+
+extension Foo: Equatable {
+  static func ==(left: Self, right: Self) -> Bool { ... }
+}
+
+extension Foo: Hashable {
+  func hash(into hasher: inout Hasher) { ... }
+}
+
+extension Foo: Sequence {
+  struct Iterator: IteratorProtocol { /* hundreds of lines */ }
+
+  func makeIterator() -> Iterator { ... }
+  ...
+}
+
+extension Foo: RandomAccessCollection {
+  var count: Int { ... }
+  ...
+}
+
+extension Foo {
+  /* more stuff */
+}
+```
+
 ### Public APIs
 
 #### Core Standard Library

stdlib/public/core/ArrayBuffer.swift

@@ -25,6 +25,13 @@ internal typealias _ArrayBridgeStorage
 @usableFromInline
 @frozen
 internal struct _ArrayBuffer<Element>: _ArrayBufferProtocol {
+  @usableFromInline
+  internal var _storage: _ArrayBridgeStorage
+
+  @inlinable
+  internal init(storage: _ArrayBridgeStorage) {
+    _storage = storage
+  }
 
   /// Create an empty buffer.
   @inlinable
@@ -74,15 +81,6 @@ internal struct _ArrayBuffer<Element>: _ArrayBufferProtocol {
     // NSArray's need an element typecheck when the element type isn't AnyObject
     return !_isNativeTypeChecked && !(AnyObject.self is Element.Type)
   }
-
-  //===--- private --------------------------------------------------------===//
-  @inlinable
-  internal init(storage: _ArrayBridgeStorage) {
-    _storage = storage
-  }
-
-  @usableFromInline
-  internal var _storage: _ArrayBridgeStorage
 }
 
 extension _ArrayBuffer {

stdlib/public/core/BridgingBuffer.swift

@@ -11,8 +11,9 @@
 //===----------------------------------------------------------------------===//
 
 internal struct _BridgingBufferHeader {
-  internal init(_ count: Int) { self.count = count }
   internal var count: Int
+
+  internal init(_ count: Int) { self.count = count }
 }
 
 // NOTE: older runtimes called this class _BridgingBufferStorage.

stdlib/public/core/Codable.swift

@@ -3369,8 +3369,8 @@ public enum DecodingError: Error {
 // The following extensions allow for easier error construction.
 
 internal struct _GenericIndexKey: CodingKey, Sendable {
-    internal var stringValue: String
-    internal var intValue: Int?
+  internal var stringValue: String
+  internal var intValue: Int?
 
   internal init?(stringValue: String) {
     return nil

stdlib/public/core/Collection.swift

@@ -72,18 +72,18 @@ public struct IndexingIterator<Elements: Collection> {
   @usableFromInline
   internal var _position: Elements.Index
 
+  /// Creates an iterator over the given collection.
   @inlinable
   @inline(__always)
-  /// Creates an iterator over the given collection.
   public /// @testable
   init(_elements: Elements) {
     self._elements = _elements
     self._position = _elements.startIndex
   }
 
+  /// Creates an iterator over the given collection.
   @inlinable
   @inline(__always)
-  /// Creates an iterator over the given collection.
   public /// @testable
   init(_elements: Elements, _position: Elements.Index) {
     self._elements = _elements