Skip to content

Conversation

@georgefst
Copy link

@georgefst georgefst commented Dec 2, 2024

This makes it possible to use deriving via Generically T instance Enum T for many more types T.

In the process of opening this PR, I have realised why one might not want to make this change, namely because of the restrictions outlined in the docs:

The Enum instances of the field types need to start enumerating from 0. In particular, Int is an unfit field type, because the enumeration of the negative values starts before 0.

This one could fairly easily be lifted by doing a bit of extra arithmetic with minBound and maxBound to shift things to zero, though in some cases that would exacerbate the second restriction...

There can only be up to maxBound :: Int values (because the implementation represents the cardinality explicitly as an Int ). This restriction makes Word an invalid field type as well. Notably, it is insufficient for each individual field types to stay below this limit. Instead it applies to the generic type as a whole.

This unfortunately is a bit problematic, and I don't think there's enough information available at the type level for us to produce a custom compile-time error. On the other hand, one could perhaps make the argument that users are unlikely to want to use these sorts of types, or that the restriction is pretty obvious given the use of Int, and that Enum is not the safest abstraction anyway.

I also haven't looked in to backwards compatibility, i.e. whether FiniteEnum strictly generalises StandardEnum by assigning the same indices.

@andreasabel andreasabel closed this Dec 6, 2024
@andreasabel andreasabel reopened this Dec 6, 2024
@andreasabel
Copy link
Collaborator

Thanks for the PR @georgefst .

What kind of action / feedback / input are you looking for (from the side of the maintainers)?

This makes it possible to use deriving via Generically T instance Enum T for many more types T.

If I was the main responsible person here, I would want to see evidence for this in form of extensions to the testsuite that demonstrate the reach of the new technique.

I also haven't looked in to backwards compatibility, i.e. whether FiniteEnum strictly generalises StandardEnum by assigning the same indices.

Also, I would want to see evidence for backwards compatibility.

@georgefst
Copy link
Author

georgefst commented Dec 8, 2024

What kind of action / feedback / input are you looking for (from the side of the maintainers)?

First and foremost, whether this would in principle be seen as a positive change, despite the restriction that types must be smaller than Int (and that we can't warn about this). Everything else can be overcome, but I won't be putting in the work if the PR wouldn't get merged anyway!

@Lysxia
Copy link
Owner

Lysxia commented Dec 9, 2024

As much as I'd like deriving Enum to be more powerful, I did intentionally keep FiniteEnum separate from Generically for the reasons you cited, namely that it produces nonsense instances for types involving Int and Word. Detecting these situations at compile time is also tricky with its own edge cases. So I chose to make FiniteEnum an opt-in with documented edge cases instead.

Although Generically doesn't use FiniteEnum, did you know that there is the newtype FiniteEnumeration for deriving via?

deriving via FiniteEnumeration T instance Enum T

@georgefst
Copy link
Author

Although Generically doesn't use FiniteEnum, did you know that there is the newtype FiniteEnumeration for deriving via?

Oh, cool, no, I hadn't noticed that.

@Lysxia Lysxia closed this Dec 21, 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.

3 participants