Skip to content

Added Hashable conformance to Async(Throwing)Stream.Continuation #79457

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

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Added explicit availability attribute for each protocol requirement
To fix failing test/api-digester/stability-concurrency-abi.test
  • Loading branch information
nickolas-pohilets committed Apr 13, 2025
commit d8cbaebbd48cc0327f44dd8c6126750768437a3b
6 changes: 6 additions & 0 deletions stdlib/public/Concurrency/AsyncStream.swift
Original file line number Diff line number Diff line change
Expand Up @@ -477,9 +477,15 @@ extension AsyncStream.Continuation.YieldResult: Sendable where Element: Sendable

@available(SwiftStdlib 9999, *)
extension AsyncStream.Continuation: Hashable {
@available(SwiftStdlib 9999, *)
public func hash(into hasher: inout Hasher) {
return hasher.combine(ObjectIdentifier(storage))
}
@available(SwiftStdlib 9999, *)
public var hashValue: Int {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why that, isn't the hash function enough?

Copy link
Contributor Author

@nickolas-pohilets nickolas-pohilets Apr 14, 2025

Choose a reason for hiding this comment

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

Apparently no. If omitted it gets synthesised as expected, but then test complains that it does not have availability annotations. I'm not sure if this is expected or a bug in code gen of the Hashable conformance.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this expected @lorentey ?

Copy link
Member

Choose a reason for hiding this comment

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

This definitely is a bug! Conformances with availability are a relatively recent development, and evidently we have not yet tried to combine them with synthesized requirements. The expected behavior is that the synthesized requirement would match the availability of the conformance itself.

It feels like this might be a good candidate for a starter task in compiler development.

(The hashValue as implemented matches the synthesized one; when the compiler issue is resolved, then it can simply be deleted to have it be replaced with the implicit default.)

Copy link
Member

Choose a reason for hiding this comment

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

Oh hang on -- can you tell me what test is failing?

The compiler itself seems fine with it; this compiles with no diagnostic:

@available(macOS 11, *)
public struct Foo {
  public var value: Int
}
@available(macOS 12, *)
extension Foo: Hashable {
  @available(macOS 12, *)
  public func hash(into hasher: inout Hasher) {
    hasher.combine(value)
  }
}

The generated .swiftinterface indeed doesn't set availability directly on the synthesized members (== and hashValue), but they still inherit that from the extension's own context:

import Swift
import _Concurrency
import _StringProcessing
import _SwiftConcurrencyShims
@available(macOS 11, *)
public struct Foo {
  public var value: Swift.Int
}
@available(macOS 12, *)
extension foo.Foo : Swift.Hashable {
  @available(macOS 12, *)
  public func hash(into hasher: inout Swift.Hasher)
  public static func == (a: foo.Foo, b: foo.Foo) -> Swift.Bool
  public var hashValue: Swift.Int {
    get
  }
}

This sounds like it might actually be due to an overzealous test that attempts to analyze the interface. (If my diagnosis is correct, then the choice is between seeing if the test can be adjusted, or changing the compiler to generate the extra @available attributes anyway.)

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 failing test was test/api-digester/stability-concurrency-abi.test

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, that makes sense.

return _hashValue(for: self)
}
@available(SwiftStdlib 9999, *)
public static func == (lhs: Self, rhs: Self) -> Bool {
return lhs.storage === rhs.storage
}
Expand Down
6 changes: 6 additions & 0 deletions stdlib/public/Concurrency/AsyncThrowingStream.swift
Original file line number Diff line number Diff line change
Expand Up @@ -523,9 +523,15 @@ extension AsyncThrowingStream.Continuation.YieldResult: Sendable where Element:

@available(SwiftStdlib 9999, *)
extension AsyncThrowingStream.Continuation: Hashable {
@available(SwiftStdlib 9999, *)
public func hash(into hasher: inout Hasher) {
return hasher.combine(ObjectIdentifier(storage))
}
@available(SwiftStdlib 9999, *)
public var hashValue: Int {
return _hashValue(for: self)
}
@available(SwiftStdlib 9999, *)
public static func == (lhs: Self, rhs: Self) -> Bool {
return lhs.storage === rhs.storage
}
Expand Down