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-7934] Hashable changes result in shifting behavior between Swift 4.1 and Swift 4.2 #50469

Closed
gfontenot opened this issue Jun 8, 2018 · 4 comments
Labels

Comments

@gfontenot
Copy link

gfontenot commented Jun 8, 2018

Previous ID SR-7934
Radar None
Original Reporter @gfontenot
Type Bug
Status Closed
Resolution Won't Do

Attachment: Download

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

md5: b235e0139b72a163f9bf4f9006d850ef

Issue Description:

Preface: I'm totally aware that this code is using undefined behavior and honestly I was surprised it ever worked. That being said, I figured I'd open up a bug in the off-chance this was indicative of a larger breaking change, or if this change wasn't known/intended.

In Swift 4.0 and 4.1, we were able to define a function that, given a type that conforming to Hashable and RawRepresentable, could return an array containing all of the cases of that type. This served as a hacky custom implementation of what 4.2 ships with CaseIterable.

When running this same code in Swift 4.2, even in compatibility mode, the behavior of the hashing algorithm seems to change, and so this code starts returning an empty array.

I've attached a playground that includes the code to reproduce the change. You can flip between toolsets and see that small assertion pass/fail accordingly.

@belkadan
Copy link
Contributor

belkadan commented Jun 9, 2018

Yeah, I don't think it was ever safe to rely on hashValue producing raw values. @lorentey, @airspeedswift?

@beccadax
Copy link
Contributor

beccadax commented Jun 10, 2018

We noticed during the development of SE-0194 that some sources have suggested using the hashValue to access the discriminator and even discussed casting hashValues back to the enum type using `unsafeBitCast(_:to🙂`. The more responsible sources warned that this was an implementation detail and shouldn't actually be used. For instance, Erica Sadun said:

Basic enumerations (the first two cases) are Hashable, that is, they provide an integer hashValue that is unique to each value. Unsurprisingly, the hash values for enumerations start at zero and increase monotonically. (Yes, this is an implementation detail. Alarm bell. Warning blare.) [...]

Now we wander into the shadow of the valley of doom, so you should start to fear some evil. Swift is no longer with you, and unsafe bitcasts lie ahead.

SE-0194 handles this use case in a supported way, and for backwards compatibility users can provide an explicit `allCases` array. Personally, I think people had enough warning that they shouldn't do this in production.

@lorentey
Copy link
Member

lorentey commented Jun 11, 2018

Jordan & Brent are correct; Swift documentation has always been careful to point out that the hashValue may change not just between Swift releases, but also between executions of the same program.

The case enumerator code relies on unstable implementation details on multiple levels – it's extremely clever, but it's much too fragile to expect it to keep working across compiler releases. Here is a (probably partial) list of its assumptions:

  • enums don't have associated values

  • the code runs on a little-endian machine

  • enum cases are internally represented as integer values, numbered sequentially from zero

  • the automatically generated hashValue implementation simply returns the case number of each case

  • the switch statement in the hashValue implementation won't be optimized away into a single integer cast

  • switch statements covering all cases without a default clause won't trap when given an illegal value

"Fixing" the original code to work with Swift 4.2 can be done by a one-line change; however, I highly, highly recommend switching to the new CaseIterable protocol instead. Your code sample is an excellent way of doing that!

@lorentey
Copy link
Member

lorentey commented Sep 14, 2018

I'm closing this with Won't Do; the original code is using a number of overly fragile constructs that rely on unsafe assumptions about the compiler and the standard library – unfortunately it could never be expected to keep working for more than a few compiler releases.

We recommend existing code that incorporates this code snippet to be updated to use CaseIterable from SE-0194. It provides the same functionality in a robust, officially supported way.

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

No branches or pull requests

4 participants