Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Make models Sendable #829

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jbelkins
Copy link
Contributor

@jbelkins jbelkins commented Sep 20, 2024

Issue #

awslabs/aws-sdk-swift#1768

Description of changes

Generated models are now Sendable. Generated structures, unions, enums, and intEnums all declare Sendable conformance in their definitions. Changes needed to enable this conformance:

  • The SmithyDocument protocol now conforms to Sendable. All of our document implementation types are fully Sendable without modification because their properties are already either immutable or Sendable.
  • The ByteStream enumeration is now Sendable. To support this:
    • ReadableStream and WriteableStream protocols both now conform to Sendable.
    • All of our concrete stream types are marked either Sendable or @unchecked Sendable. Streams were built to be thread-safe so this merely requires us to manually maintain that thread safety on stream types going forward.
  • The @Indirect property wrapper (used for enabling recursive structures) is provided with @unchecked Sendable conformance that uses a lock to enforce exclusive access to its wrapped value.
  • The Header and Headers properties are made Sendable with no changes to implementation.

Also:

  • Fixed some formatting in structure & union rendering.
  • Deleted extensions on CRTError and AsyncThrowingStream that were unused and causing compiler warnings.

Scope

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jbelkins jbelkins changed the title feat: Mark Document and ByteStream as Sendable feat: Make models Sendable Sep 20, 2024
defer { lock.unlock() }
_wrappedValue = newValue
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This property wrapper is mostly same as before, but now stores its value in private storage that is protected by a lock when getting & setting.

Since it enforces exclusive access to its wrapped value, it can now be Sendable.

@@ -8,7 +8,7 @@
import class Foundation.FileHandle
import struct Foundation.Data

public enum ByteStream {
public enum ByteStream: Sendable {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This conformance is possible since Stream has been made Sendable in this PR.

@@ -8,8 +8,9 @@
import struct Foundation.Data
import struct Foundation.Date

public protocol SmithyDocument {
public protocol SmithyDocument: Sendable {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the SmithyDocument implementations are now sendable without any code change, since they only store data that is either immutable and/or Sendable.

public class ChunkedStream {
private var inputStream: Stream
public class ChunkedStream: @unchecked Sendable {
private let inputStream: Stream
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Streams were designed to be thread-safe so they may be marked @unchecked Sendable so long as we maintain that thread safety.

@@ -15,11 +15,3 @@ public enum EventStreamError: Error {
/// This may be due to missing required headers
case invalidMessage(String)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The extension below was unused and causing compiler warnings, so it was deleted.

@@ -26,7 +26,7 @@ public final class FileStream: Stream {
let fileHandle: FileHandle

/// Returns the current position of the stream.
public var position: Data.Index
public private(set) var position: Data.Index
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically this is a breaking API change but it would cause a bug to alter this property from outside this type.

@@ -39,7 +39,7 @@ class EnumGeneratorTests {

val expectedGeneratedEnum = """
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Below changes are Kotlin codegen tests. Changes in test expectations:

  • All generated structs, unions, enums, & intEnums are Sendable.
  • Whitespace around type declarations is made uniform, with an empty line before a declaration & none after.
  • Test strings are left-justified to allow for easy maintenance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Try turning off whitespace changes on the Github diff to see the more substantial part of the test changes.

@@ -5,7 +5,7 @@
// SPDX-License-Identifier: Apache-2.0
//

public struct Headers {
public struct Headers: Sendable {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Headers and Header may be marked Sendable without any further change since they are structs that only store value types.

"}",
SwiftTypes.Protocols.Sendable,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mark generated enums with Sendable.

"}",
SwiftTypes.Protocols.Sendable,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mark generated intEnums with Sendable.

Also adjust spacing a bit.

val equatableConformance = writer.format(": \$N ", SwiftTypes.Protocols.Equatable).takeIf { shape.hasTrait<EquatableConformanceTrait>() } ?: ""
writer.openBlock("public struct \$struct.name:L $equatableConformance{")
val equatableConformance = writer.format(", \$N", SwiftTypes.Protocols.Equatable).takeIf { shape.hasTrait<EquatableConformanceTrait>() } ?: ""
writer.openBlock("public struct \$struct.name:L: \$N$equatableConformance {", SwiftTypes.Protocols.Sendable)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mark generated structs with Sendable.

Also adjust spacing of generated code.

val equatableConformance = (": " + SwiftTypes.Protocols.Equatable + " ").takeIf { shape.hasTrait<EquatableConformanceTrait>() } ?: ""
writer.openBlock("public ${indirectOrNot}enum \$union.name:L $equatableConformance{", "}\n") {
val equatableConformance = writer.format(", \$N", SwiftTypes.Protocols.Equatable).takeIf { shape.hasTrait<EquatableConformanceTrait>() } ?: ""
writer.openBlock("public ${indirectOrNot}enum \$union.name:L: \$N$equatableConformance {", "}", SwiftTypes.Protocols.Sendable) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mark generated unions with Sendable.

Also adjust formatting.

@@ -31,6 +31,7 @@ object SwiftTypes {
val RawRepresentable = builtInSymbol("RawRepresentable", SwiftDeclaration.PROTOCOL)
val CaseIterable = builtInSymbol("CaseIterable", SwiftDeclaration.PROTOCOL)
val CustomDebugStringConvertible = builtInSymbol("CustomDebugStringConvertible", SwiftDeclaration.PROTOCOL)
val Sendable = builtInSymbol("Sendable", SwiftDeclaration.PROTOCOL)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

SwiftSymbol for the Sendable protocol.

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.

1 participant