Skip to content
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

[SR-9932] CGPoint, CGSize, CGRect should conform to Hashable #3533

Open
lilyball mannequin opened this issue Feb 14, 2019 · 8 comments
Open

[SR-9932] CGPoint, CGSize, CGRect should conform to Hashable #3533

lilyball mannequin opened this issue Feb 14, 2019 · 8 comments

Comments

@lilyball
Copy link
Mannequin

lilyball mannequin commented Feb 14, 2019

Previous ID SR-9932
Radar None
Original Reporter @lilyball
Type Bug
Environment

Apple Swift version 4.2.1 (swiftlang-1000.11.42 clang-1000.11.45.1)
Target: x86_64-apple-darwin18.2.0

Additional Detail from JIRA
Votes 0
Component/s Foundation
Labels Bug
Assignee None
Priority Medium

md5: 56a960925d38d3ac1424af3a012c2459

Issue Description:

It looks like CGPoint, CGSize, and CGRect do not currently conform to Hashable. I see no reason why they shouldn't, so conformance should be added.

@belkadan
Copy link

cc @moiseev. The CoreGraphics team gets to decide whether they want this, rather than Foundation. There's also a backwards-deployment concern here, since it's likely too late for Swift 5.

@moiseev
Copy link
Mannequin

moiseev mannequin commented Feb 15, 2019

Conditionally available conformances do not exit, IIRC. So there is little we can do other than create trivial Hashable wrappers for the said types.

@lilyball
Copy link
Mannequin Author

lilyball mannequin commented Feb 15, 2019

Are we talking about the scenario where someone releases a Swift 5 app and then runs it on a system that provides updated standard libraries that includes this conformance? I assume this would only actually be an issue if the app itself provided its own conformances.

It's unclear to me what the runtime behavior here would actually be. Would the app statically dispatch to its own conformance when calling hashValue while the stdlib would dispatch to the stdlib conformance, thereby potentially creating incompatible hashes for the same value? If so, that seems like a general problem that we should solve because surely this isn't the only time we'll want to add a missing conformance to a stdlib type that apps may have already provided themselves. Though surely this problem has already existed before, if two different dependencies declare the same conformance on a stdlib type. I have no idea how this is handled today in that case.

@moiseev
Copy link
Mannequin

moiseev mannequin commented Feb 15, 2019

I may be totally wrong, but my understanding is: if you try to re-compile the code that adds a conformance that is now a duplicate to the one provided by the standard library, that'd fail, because we don't allow multiple conformances. But if you just run the previously compiled binary with to the new standard library, the symbols for the stdlib conformance will not clash with symbols from the app, since the module name is included in a mangled name. So this case should be fine. But this is nevertheless API breaking.

@belkadan
Copy link

I think we downgraded this to a warning for when we added Collection to String, so it might not be a source compat break. But the ABI issue is that this gets into 5.1, and then someone tries to deploy it on a system with Swift 5 only, and the Hashable conformance is missing.

@lilyball
Copy link
Mannequin Author

lilyball mannequin commented Feb 15, 2019

If someone develops against Swift 5.1 and deploys against a system with Swift 5, isn't that a problem even without API changes, because they'll be expecting any bugfixes or behavioral changes from Swift 5.1? I haven't researched this but my assumption was that we would continue to bundle the library in the app whenever the app is deployed to a system that doesn't include the standard library the app was developed with (or to put it another way, app thinning would only strip out the stdlib from the app if the system includes the correct version). Is that not actually the plan?

@moiseev
Copy link
Mannequin

moiseev mannequin commented Feb 15, 2019

Oh.. You are right. It is a warning now. No questions about the back-deployment ABI break, I was speculating only about the forward deployment.

@belkadan
Copy link

People build against macOS 10.11 and deploy to macOS 10.10 all the time.

The system Swift is in use by system libraries, and we can't have two different versions of Swift running around in the same process. Consequently, the bundled libraries are only going to be present on OSs that don't have a Swift stdlib in /usr/lib/. To avoid a version gap, the backwards-deployment strategy is to bundle the Swift 5 stdlib in the app, not whatever you built with. You can read more about this in Joe's recent blog post: https://swift.org/blog/abi-stability-and-apple/.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant