Skip to content

Fix hash/isEqual interop conditionals, update tests #71852

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 5 commits into from
Feb 29, 2024

Conversation

tbkka
Copy link
Contributor

@tbkka tbkka commented Feb 23, 2024

Github PR #71620 mixed up one of the bincompat conditionals. It also had some errors in the tests for ObjC interop.

For now, this leaves the legacy behavior in place for all Apple platforms.

Resolves rdar://123422591

@tbkka tbkka requested a review from bnbarham February 23, 2024 19:48
@tbkka tbkka requested review from a team, mikeash and al45tair as code owners February 23, 2024 19:48
@tbkka
Copy link
Contributor Author

tbkka commented Feb 23, 2024

@swift-ci Please test

// Legacy behavior: Equatable in Swift => ObjC hashes with identity
TestSwiftObjectNSObjectDefaultHashValue(e)
let msg = "Obj-C `-hash` ... type `SwiftObjectNSObject.\(type(of: e))` ... Equatable but not Hashable\n"
fputs(msg, stderr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Leftover debugging stuff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alas, no. Maybe I should comment it a bit more, though.

There's some FileCheck stuff here to verify that the runtime is really emitting some warning logs for some of these cases. So when we don't take those new paths, we have to emit a fake log message to make FileCheck happy. Maybe there's a better way to do this....

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see, I didn't realize FileCheck was involved in this one. Makes sense. I think the alternative would be to use --check-prefix to check for different things in different contexts, but that probably wouldn't be an improvement here.

@tbkka
Copy link
Contributor Author

tbkka commented Feb 23, 2024

@swift-ci Please test

1 similar comment
@tbkka
Copy link
Contributor Author

tbkka commented Feb 26, 2024

@swift-ci Please test

Github PR swiftlang#71620 mixed up one of the bincompat conditionals.
It also had some errors in the tests for ObjC interop.

For now, this leaves the legacy behavior in place for
all Apple platforms.
@tbkka tbkka force-pushed the tbkka-rdar123422591 branch from 737c95a to 2be0f04 Compare February 28, 2024 18:43
@tbkka
Copy link
Contributor Author

tbkka commented Feb 28, 2024

@swift-ci Please test

@tbkka tbkka merged commit 69ef3ad into swiftlang:main Feb 29, 2024
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.

2 participants