Update Swift tools version to 5.9 #861

d-ronnqvist · 2024-03-18T13:24:52Z

Bug/issue #, if applicable: rdar://116871051

Summary

This updates SwiftDocC to Swift 5.9 and

uses shorthand syntax for if let (SE-0345)
modernizes generics syntax (SE-0328, SE-0341, SE-0361)
removes a no longer needed pack-port of pointer API (SE-0370)
replaces FileManagerProtocol SPI with package access level (SE-0386)

There are lots more that we can do now, but I feel that those are better addressed in separate PRs. I opted for these changes because they each make the code more readable and have small (but very frequent) and understandable diffs.

Dependencies

None

Testing

Nothing in particular. There should be

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

~~[] Added tests~~
Ran the ./bin/test script and it succeeded
~~[ ] Updated documentation if necessary~~

…om Swift 5.7 This also leverages "primary associated types" (SE-0358) available from Swift 5.7

…of SPI for FileManagerProtocol

…ce Swift 5.7

…s available)

d-ronnqvist · 2024-03-18T14:14:48Z

I made each change as its own commit, to make it easier to review. If you prefer I can break it apart into separate PRs but I didn't feel that that would make it easier to review.

patshaughnessy

Great improvement! Thanks a lot.

I found some additional fixes you can make that follow the same patterns, if you're interested in adding those now. Also various code questions for you.

patshaughnessy · 2024-03-18T21:01:46Z

Sources/SwiftDocC/Model/Rendering/Diffing/DifferenceBuilder.swift

    /// Adds the difference between two properties to the DifferenceBuilder.
-    mutating func addDifferences<E>(atKeyPath keyPath: KeyPath<T, E>, forKey codingKey: CodingKey)
-    where E: Equatable & Encodable {
+    mutating func addDifferences(atKeyPath keyPath: KeyPath<T, some Equatable & Encodable>, forKey codingKey: CodingKey)


What's the precedence order for some Equatable & Encodable ? We don't need parentheses here?

I can't find the specific precedence order but from what I've understood some A & B is the same as some (A & B). In other cases I needed to add parentheses around the full expression to write (some A)?

patshaughnessy · 2024-03-18T22:03:41Z

Sources/SwiftDocC/Infrastructure/Diagnostics/DiagnosticConsoleWriter.swift


-    @_spi(FileManagerProtocol)
-    public init(
+    package init(


Can we also use package access level for ExternalLinks? For example:

extension OutOfProcessReferenceResolver: ConvertServiceFallbackResolver { @_spi(ExternalLinks) public func entityIfPreviouslyResolved(with reference: ResolvedTopicReference) -> LinkResolver.ExternalEntity? {

Could this become:

extension OutOfProcessReferenceResolver: ConvertServiceFallbackResolver { package func entityIfPreviouslyResolved(with reference: ResolvedTopicReference) -> LinkResolver.ExternalEntity? {

I couldn't because it's used in a public protocol and I couldn't use different access levels in the same protocol.

We will eventually settle on a stable API for it and then we can change the SPI to public API.

patshaughnessy · 2024-03-18T22:07:04Z

Sources/SwiftDocC/Indexing/Indexable.swift

    }
 }

-extension Sequence where Element == RenderBlockContent {


I found a couple more examples of where we could use generic syntax for extending collections...

DocCSymbolRepresentable.swift

public extension Collection where Element: DocCSymbolRepresentable {

Collection+ConcurrentPerform.swift

extension Collection where Index == Int {

The first example can't be written as Collection<DocCSymbolRepresentable> because that would change its meaning from where Element: DocCSymbolRepresentable to where Element == DocCSymbolRepresentable (which also doesn't compile). In other places this would be described by Collection<some DocCSymbolRepresentable> but syntax wasn't supported in extensions.

Regarding extension Collection where Index == Int {: The primary associated type of Collection is the element, so

Collection<Int> would mean Collection where Element == Int, not Collection where Index == Int

Got it - I see the new generic syntax refers to the associated type of the parent type.

Specifically the primary associated type. Collection is generic over both the Element and Index but only Element is the primary associated type.

patshaughnessy · 2024-03-18T23:02:46Z

Sources/SwiftDocC/Model/Identifier.swift

        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encode(url.absoluteString, forKey: .url)

        let sourceLanguageIDVariants = DocumentationDataVariants<String>(


Found a few other places where we could use modern syntax for dictionaries:

DocumentationContext.swift

/// The documentation bundles that are currently registered with the context. public var registeredBundles: Dictionary<String, DocumentationBundle>.Values { return dataProvider.bundles.values }

BidirectionalMap.swift

func makeIterator() -> Dictionary<Value1, Value2>.Iterator { return forward.makeIterator() }

GroupedSequence.swift

/// Returns an iterator over the members of the sequence. func makeIterator() -> Dictionary<Key, Element>.Values.Iterator { storage.values.makeIterator() }

Some of these can be written using opaque types (the some keyword) to not define a specific type. For example:

- public var registeredBundles: Dictionary<String, DocumentationBundle>.Values + public var registeredBundles: some Collection<DocumentationBundle>

/// Returns an iterator over the members of the sequence. - func makeIterator() -> Dictionary<Key, Element>.Values.Iterator { + func makeIterator() -> some IteratorProtocol<Element> {

but for the bidirectional map I don't feel like some IteratorProtocol<(key: Value1, value: Value2)> is any more readable than [Value1: Value2].Iterator

I agree - some IteratorProtocol<Element> is actually quite readable even though it uses the verbose generic syntax. Calling this will return some something that can iterate over Element's.

But yea some IteratorProtocol<(key: Value1, value: Value2)> seems confusing either because there's a key and a value in the tuple, or just because of the unhelpful names Value1 and Value2.

patshaughnessy · 2024-03-18T23:04:14Z

Sources/SwiftDocC/Model/Rendering/Diffing/DifferenceBuilder.swift

    }

    /// Determines the difference between the two dictionaries mapping strings to diffable objects at the KeyPaths given.
-    mutating func addDifferences<Element>(atKeyPath keyPath: KeyPath<T, Array<Element>>, forKey codingKey: CodingKey)


A few other examples of modern syntax for arrays:

MarkupContainer.swift

extension MarkupContainer: RandomAccessCollection { public subscript(position: Array<Markup>.Index) -> Markup { return elements[position] } public var startIndex: Array<Markup>.Index { return elements.startIndex } public var endIndex: Array<Markup>.Index { return elements.endIndex } }

patshaughnessy · 2024-03-18T23:09:22Z

Sources/SwiftDocC/Checker/Checker.swift


    /// Creates a checker that performs the combined work of the given checkers.
-    public init<Checkers: Sequence>(_ checkers: Checkers) where Checkers.Element: Checker {
+    public init(_ checkers: some Sequence<Checker>) {


Found a few more places where we could simplify the generic syntax using some:

SemanticAnalysis.swift

func analyze<Children: Sequence>( _ directive: BlockDirective, children: Children, source: URL?, for bundle: DocumentationBundle, in context: DocumentationContext, problems: inout [Problem] ) -> Result where Children.Element == Markup

to:

func analyze( _ directive: BlockDirective, children: some Sequence<Markup>, source: URL?, for bundle: DocumentationBundle, in context: DocumentationContext, problems: inout [Problem] ) -> Result

DocumentationConverter.swift

mutating func convert<OutputConsumer: ConvertOutputConsumer>( outputConsumer: OutputConsumer ) throws -> (analysisProblems: [Problem], conversionProblems: [Problem])

to:

mutating func convert( outputConsumer: some ConvertOutputConsumer ) throws -> (analysisProblems: [Problem], conversionProblems: [Problem])

DirectiveArgumentWrapper.swift

func setProperty<T: AutomaticDirectiveConvertible>( on containingDirective: T, named propertyName: String, to any: Any )

to:

func setProperty( on containingDirective: some AutomaticDirectiveConvertible, named propertyName: String, to any: Any )

There are many other places in the code where we have generic constraints that appear to be places where we could add some - but actually don't compile. The most common example of these are the places where we use the visitor pattern:

Symbol.swift

public override func accept<V: SemanticVisitor>(_ visitor: inout V) -> V.Result { return visitor.visitSymbol(self) }

Here it appears we could write

public override func accept(_ visitor: inout some SemanticVisitor) -> SemanticVisitor.Result { return visitor.visitSymbol(self) }

.... but Swift returns an error: Cannot access associated type 'Result' from 'SemanticVisitor'; use a concrete type or generic parameter base instead

Any idea why? Why can't Swift access the Result associated type? Is there a better way to write the function declaration in this example?

I didn't realize that I could update the declaration in the protocols themselves. I got some error message about that earlier and thought that it wasn't supported but it looks like it is.

.... but Swift returns an error: Cannot access associated type 'Result' from 'SemanticVisitor'; use a concrete type or generic parameter base instead

V.Result is referring to the specific Result associated type of V. However SemanticVisitor.Result isn't connected to the some SemanticVisitor parameter, so it doesn't define a return type that's associated with the parameter type.

V.Result is referring to the specific Result associated type of V. However SemanticVisitor.Result isn't connected to the some SemanticVisitor parameter, so it doesn't define a return type that's associated with the parameter type.

Ok. But don't all types that conform to SemanticVisitor have the same associated type Result? It seems to me that Swift should be able to make this compile and run.

Each SemanticVisitor has an associate Result but it doesn't have to be the same across different visitors. That's why this needs to label the visitor as V to specifically be able to refer to its Result type

d-ronnqvist · 2024-03-19T09:38:48Z

@swift-ci please test

d-ronnqvist · 2024-03-20T10:55:56Z

Hm.. the Linux CI seems to be pinned to SWIFT_VERSION=5.8.1 which causes it to fail.

d-ronnqvist · 2024-03-25T10:08:10Z

This is blocked on the CI (rdar://125077916)

…instead of SPI for FileManagerProtocol" f1ca44c

d-ronnqvist · 2024-03-27T17:52:23Z

@swift-ci please test

d-ronnqvist · 2024-03-27T18:03:09Z

The team talked about this today and decided to revert the package access level commit and add it back in a follow up PR to unblock this.

* Update Swift tools version to 5.9 rdar://116871051 * Use if-let shorthand (SE-0345) available from Swift 5.7 * Modernize generics with opaque types (SE-0328 & SE-0341) available from Swift 5.7 This also leverages "primary associated types" (SE-0358) available from Swift 5.7 * Remove unused AnyCommunicationBridge type-erasure wrapper * Remove back-port of pointer API (SE-0370) available in Swift 5.8 * Use package access level (SE-0386, available from Swift 5.9) instead of SPI for FileManagerProtocol * Simplify extensions using bound generic types (SE-0361) available since Swift 5.7 * Use shorthand syntax for array, dictionary, and optional types (always available) * Define primary associated type (SE-0346) for DirectiveArgument protocol * Use opaque parameter types (SE-0341) in protocol definitions * Use opaque return types (SE-0328) for some properties * Use array and dictionary shorthand syntax in more places * Use bound generic types (SE-0361) for extensions in more places * Revert "Use package access level (SE-0386, available from Swift 5.9) instead of SPI for FileManagerProtocol" f1ca44c

d-ronnqvist added 8 commits March 18, 2024 10:53

Update Swift tools version to 5.9

2eccdb5

Use if-let shorthand (SE-0345) available from Swift 5.7

e648779

Modernize generics with opaque types (SE-0328 & SE-0341) available fr…

721d959

…om Swift 5.7 This also leverages "primary associated types" (SE-0358) available from Swift 5.7

Remove unused AnyCommunicationBridge type-erasure wrapper

11b848e

Remove back-port of pointer API (SE-0370) available in Swift 5.8

03ffe37

Use package access level (SE-0386, available from Swift 5.9) instead …

f1ca44c

…of SPI for FileManagerProtocol

Simplify extensions using bound generic types (SE-0361) available sin…

adb24c0

…ce Swift 5.7

Use shorthand syntax for array, dictionary, and optional types (alway…

3ab37af

…s available)

d-ronnqvist added the code cleanup Code cleanup *without* user facing changes label Mar 18, 2024

patshaughnessy approved these changes Mar 18, 2024

View reviewed changes

d-ronnqvist added 6 commits March 19, 2024 09:36

Merge branch 'main' into update-to-swift-5.9

f5fa832

Define primary associated type (SE-0346) for DirectiveArgument protocol

3175893

Use opaque parameter types (SE-0341) in protocol definitions

52fb7bb

Use opaque return types (SE-0328) for some properties

c62cdb9

Use array and dictionary shorthand syntax in more places

3458053

Use bound generic types (SE-0361) for extensions in more places

abc996a

patshaughnessy mentioned this pull request Mar 21, 2024

Collect external links from various symbol section types #855

Merged

d-ronnqvist added 2 commits March 27, 2024 18:49

Merge branch 'main' into update-to-swift-5.9

1d3f6cd

Revert "Use package access level (SE-0386, available from Swift 5.9) …

41ea1a2

…instead of SPI for FileManagerProtocol" f1ca44c

d-ronnqvist merged commit 8f04a33 into swiftlang:main Mar 27, 2024

d-ronnqvist deleted the update-to-swift-5.9 branch March 27, 2024 18:03

This was referenced Mar 27, 2024

[6.0] Update Swift tools version to 5.9 #869

Merged

Use package access level (SE-0386) instead of SPI for FileManagerProtocol #878

Merged

patshaughnessy mentioned this pull request Apr 16, 2024

Add logic to look for overload groups when emitting warnings #885

Merged

Update Swift tools version to 5.9 #861

Update Swift tools version to 5.9 #861

Uh oh!

Conversation

d-ronnqvist commented Mar 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Dependencies

Testing

Checklist

Uh oh!

d-ronnqvist commented Mar 18, 2024

Uh oh!

patshaughnessy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d-ronnqvist commented Mar 19, 2024

Uh oh!

d-ronnqvist commented Mar 20, 2024

Uh oh!

d-ronnqvist commented Mar 25, 2024

Uh oh!

d-ronnqvist commented Mar 27, 2024

Uh oh!

d-ronnqvist commented Mar 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

d-ronnqvist commented Mar 18, 2024 •

edited

Loading