-
Notifications
You must be signed in to change notification settings - Fork 102
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
alternative == method linkage fix #140
base: main
Are you sure you want to change the base?
Conversation
@swift-ci please test |
@@ -72,6 +72,16 @@ extension FilePath { | |||
} | |||
} | |||
|
|||
extension FilePath.Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extension FilePath.Component { | |
extension FilePath.Component: Hashable { |
@lorentey would this be possible to do (without availability) as the ABI-stable build picked up the symbols?
Also, would we still need the public func
below if we added the conformance explicitly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awaiting @lorentey input
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sinking of the Equatable/Hashable conformances from _StrSlice
to Component
is probably fine, ABI-wise. If any other public type conformed to _StrSlice
, then it'll need to do the same.
_StrSlice
will need to stop conforming to Equatable
/Hashable
.
@@ -166,15 +176,22 @@ extension _StrSlice { | |||
internal var _systemString: SystemString { SystemString(_slice) } | |||
} | |||
extension _StrSlice { | |||
public static func == (lhs: Self, rhs: Self) -> Bool { | |||
public static func strSliceEqual(lhs: Self, rhs: Self) -> Bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to go this way, let's at least put an end to adding any new public
members of internal types. That only leads to sadness.
public static func strSliceEqual(lhs: Self, rhs: Self) -> Bool { | |
internal static func strSliceEqual(lhs: Self, rhs: Self) -> Bool { |
@@ -72,6 +72,16 @@ extension FilePath { | |||
} | |||
} | |||
|
|||
extension FilePath.Component { | |||
public static func == (lhs: Self, rhs: Self) -> Bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this would be a newly exported symbol, so it needs to come with an availability declaration.
To clean up the mess that resulted from adding public
members to an internal
type, we should add this implementation in a form that is back-deployable.
return Self.strSliceEqual(lhs: lhs, rhs: rhs) | ||
} | ||
|
||
public func hash(into hasher: inout Hasher) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same problem here -- this is a new symbol, so it needs to either have an @available
attribute combined with @backDeployed(before:)
, or it needs to be declared @_alwaysEmitIntoClient
.
lhs._slice.elementsEqual(rhs._slice) | ||
} | ||
public func hash(into hasher: inout Hasher) { | ||
public static func == (lhs: Self, rhs: Self) -> Bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove this function altogether. (After verifying that it doesn't end up getting exported somehow in the ABI stable binary.)
hasher.combine(_slice.count) // discriminator | ||
for element in _slice { | ||
hasher.combine(element) | ||
} | ||
} | ||
public func hash(into hasher: inout Hasher) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here -- this function should not exist.
return Self.strSliceEqual(lhs: lhs, rhs: rhs) | ||
} | ||
|
||
public func strSliceHash(into hasher: inout Hasher) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public func strSliceHash(into hasher: inout Hasher) { | |
internal func strSliceHash(into hasher: inout Hasher) { |
I'll see if I can add a commit to this PR to make it work in all configurations of System later today! |
alternative (to #139) fix for #138