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

Derive Equatable & Hashable for uninhabited types #17756

Merged
merged 1 commit into from Jul 12, 2018

Conversation

Projects
None yet
5 participants
@mdiep
Copy link
Contributor

mdiep commented Jul 5, 2018

The original proposal did explicitly say this wouldn't be done:

The compiler does not synthesize P's requirements for an enum with no cases because it is not possible to create instances of such types.

But I'm hoping that this change doesn't need to go through the evolution process:

  1. This is clearly possible

  2. It's useful when your enum is part of a larger protocol, you expect it to gain cases, or you're using it with generics.

I've been running into this a lot at work. It's often confusing for people to figure out why these aren't derived and what they should do about it.

@rudkx

This comment has been minimized.

Copy link
Member

rudkx commented Jul 5, 2018

@mdiep Given that it's specifically called out in the original proposal I think it would make sense to ping the Evolution Discussion and ask for guidance. If this does get merged with should amend the original proposal in some fashion to indicate the change.

@xwu

This comment has been minimized.

Copy link
Collaborator

xwu commented Jul 5, 2018

Since we don't have special treatment for Never, with all behaviors applying equally to all uninhabited types, isn't SE-0215 effectively calling for all uninhabited types to conform to Hashable and Equatable?

@jrose-apple

This comment has been minimized.

Copy link
Member

jrose-apple commented Jul 5, 2018

  1. No, SE-0215 as written is still just proposing that Never conform to those two protocols, presumably explicitly.
  2. SE-0215 may not pass, so this is still relevant. (I agree with Mark that it does need guidance, though.)
@mdiep

This comment has been minimized.

Copy link
Contributor

mdiep commented Jul 5, 2018

No, SE-0215 as written is still just proposing that Never conform to those two protocols, presumably explicitly.

Right. This might change the implementation of SE-0215, but is a separate question.

SE-0215 may not pass, so this is still relevant. (I agree with Mark that it does need guidance, though.)

I've started a discussion.

@mdiep mdiep force-pushed the mdiep:synthesize-equatable-hashable-for-uninhabited-types branch Jul 5, 2018

@allevato

This comment has been minimized.

Copy link
Contributor

allevato commented Jul 5, 2018

To briefly repeat what I said in my reply on the forum, I'm in support this. The limitation was arbitrary in retrospect because it seemed "natural", but the generic use cases shown in the discussion thread are convincing. Thanks, Matt!

@mdiep

This comment has been minimized.

Copy link
Contributor

mdiep commented Jul 7, 2018

The discussion has received positive feedback from core members and no negative feedback. I think that means this can proceed?

@rudkx

This comment has been minimized.

Copy link
Member

rudkx commented Jul 7, 2018

@swift-ci Please smoke test

@mdiep

This comment has been minimized.

Copy link
Contributor

mdiep commented Jul 10, 2018

bump

@jrose-apple

This comment has been minimized.

Copy link
Member

jrose-apple commented Jul 10, 2018

There's a conflict now, so you'll have to fix that before we can merge.

@mdiep mdiep force-pushed the mdiep:synthesize-equatable-hashable-for-uninhabited-types branch to 30949c7 Jul 10, 2018

@mdiep

This comment has been minimized.

Copy link
Contributor

mdiep commented Jul 10, 2018

There's a conflict now, so you'll have to fix that before we can merge.

Sorry, I didn't see that on my phone. 🙈 I've rebased and fixed the conflicts.

@jrose-apple

This comment has been minimized.

Copy link
Member

jrose-apple commented Jul 10, 2018

@swift-ci Please smoke test

@mdiep

This comment has been minimized.

Copy link
Contributor

mdiep commented Jul 12, 2018

bump

@rudkx rudkx merged commit 85115dd into apple:master Jul 12, 2018

2 checks passed

Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform (smoke test)
Details

@mdiep mdiep deleted the mdiep:synthesize-equatable-hashable-for-uninhabited-types branch Jul 12, 2018

@mdiep

This comment has been minimized.

Copy link
Contributor

mdiep commented Jul 14, 2018

I'd like to get this in 4.2 if possible. Should I just cherry-pick and open a PR against the 4.2 branch?

@jrose-apple

This comment has been minimized.

Copy link
Member

jrose-apple commented Jul 16, 2018

Yep, then include all the information in https://swift.org/blog/4-2-release-process/#pull-requests-for-release-branch and assign it to Ted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment