Skip to content

Conversation

@harlanhaskins
Copy link
Contributor

This replaces the 2-7ary variadic gyb'd tuple comparison implementations with unified variadic implementations that short-circuit when the first element fails the test.

Unfortunately, the existing gyb'd tuple comparison entry points are all ABI, so we need to leave them in. However, we can just forward them along to the variadic implementation and rename them so they no longer conflict from an overload perspective.

@harlanhaskins
Copy link
Contributor Author

@swift-ci please smoke test

@slavapestov
Copy link
Contributor

@swift-ci Please benchmark

@harlanhaskins
Copy link
Contributor Author

@swift-ci please smoke test
@swift-ci please benchmark

@harlanhaskins
Copy link
Contributor Author

@swift-ci please smoke test

@harlanhaskins
Copy link
Contributor Author

harlanhaskins commented Nov 7, 2025

00:47:36 /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/swift/stdlib/public/core/LegacyInt128.swift.gyb:763:23: error: binary operator '>' cannot be applied to operands of type '_Wide2<F>' (aka '(high: F, low: F.Magnitude)') and '(F, F.Magnitude)'

Ah, this is a fun one -- @slavapestov This seems related to the concessions here: https://forums.swift.org/t/variadic-generics-and-tuple-shuffle-conversions/60694

It looks like we explicitly are not performing the (high: F, low: F.Magnitude) -> (F, F.Magnitude) conversion which would match correctly against the variadic function here. Are there any plans to support this? Or is this expected to work as-is, and this is just a bug?

Comment on lines +64 to +70
public func !=<each B: Equatable>(lhs: (repeat each B), rhs: (repeat each B)) -> Bool {
for (lhs, rhs) in repeat (each lhs, each rhs) {
if lhs == rhs {
return false
}
}
return true
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be nicer if we forwarded to the same member-wise operations, e.g.

    guard lhs != rhs else {
      return false
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed

/// - lhs: A tuple of `Equatable` elements.
/// - rhs: Another tuple of elements of the same type as `lhs`.
@_alwaysEmitIntoClient
public func ==<each B: Equatable>(lhs: (repeat each B), rhs: (repeat each B)) -> Bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

aside: does the choice of B as the generic parameter name have some particular significance or prior art?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, no, this was leftover from originally writing it as <A, each B> thinking it might make the algorithm simpler. It didn’t. I’ll rename and make it more verbose!

Copy link
Contributor

Choose a reason for hiding this comment

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

the verbosity budget should be flush, what with all the boilerplate removal! 😄

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.

5 participants