-
Notifications
You must be signed in to change notification settings - Fork 52
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
[ffigen] New idea for enums #1735
Comments
Sounds neater, but this will mean another breaking change to enums, shortly after the last one. Might need to make it opt-in in the config, but then we'd be supporting 3 different representations for enums. |
Agreed it's not great timing. I actually had this idea a little bit ago but held onto it for that reason. Still, if this is something people would want I'm happy to give it a shot. Maybe it could be a good idea to gather some feedback first to see what people would expect from such a feature, and whether there is still benefit to having plain integers even after this (as long as |
Taking a helicopter view here, there are two types of "enum"s in native code:
Our current approach has a dedicated solution for both of them, but the solution for 2. has Using extension types could improve the API for users using flags. (Most likely they would actually need a Using extension types for use case 1. does not have a lot of drawbacks indeed. The current system with a configuration option to select between the two requires more config and rerunning the generator when the config changes, which makes FFIgen less plug-n-play. One of our north star goals is to make it work out of the box as much as we can. 👍 I'd suggest indeed making it a config option. However, we should be aiming the deprecate the existing two approaches then. So print a warning/error on the old two approaches that we will deprecate them after one or two stable releases. And I'd want the input from some users. 😄 @brianquinlan @stuartmorgan WDYT about using extension types on int for |
Unfortunately there is a trade-off. In the case of exhaustive enums, we want to specifically prohibit new instances and allow exhaustive switch cases. In the case of bit flags, we want to allow new instances, which would not allow for exhaustive checks. And as much as I don't love having two options and needing people to pick, I do think having exhaustive switch cases for the enums that warrant it is very valuable. It's not as clean as one unified solution, but if we're thoughtful about a sensible default and keep the config simple, maybe it'll still be a smooth experience |
These are two different usecases, enums and flags. So there is no unified solution for both. If we want to simplify config, FFIgen could look at all the values of an enum and if they are powers of two, log and generate it as an extension type. |
Objective-C enums are (AFAIK) always integral types e.g.
So a So an extension type seems wrong because it limits the sets of operations that you can do on the enum when, in native Objective-C, they are just integers. |
But you can always access the original About the objective C enums, I believe we already had this discussion and it turns out clang doesn't report which type of enum it is in the API we are using, which severely limits how smart we can be by default. I do like the power of two rule though |
I have never used extension types so I didn't know that you could do this. This seems fine to me then - but the extension type is really just a namespace to attach constants to then, right? |
Yep, really just that. It'll help keep the APIs clear, dart docs more helpful, code that tries to use them will have the enum name more often, and users can write extension methods for them. The | and has() methods in my example were really just examples, we don't have to provide them because users will be able to use | and & directly There is something of a question whether we should use |
What happens in the other direction; if it |
Short answer: no, but you can wrap the value, even at runtime. Long answer, extension type MyEnum(int x) implements int { }
void a(MyEnum x);
void b(int x);
void main() {
final intVal = 1;
final enumVal = MyEnum(1);
a(intVal); // error
a(enumVal); // okay
a(MyEnum(intVal)); // also okay
b(intVal); // okay
b(enumVal); // okay
} This raises an interesting point, that enums should be generated as extension types, and APIs should be left as integers for maximum compatibility. But when you think about it, there's no value from doing that over the current approach where we already generate plain integers. The whole benefit of switching to extension types would be that users would see a function and actually know which enum type it accepts, be able to click on it in dart docs, and API authors can write extension methods that only apply to some enums and not others. I think that value is worth the breaking change. |
I disagree; not being able to pass raw ints without explicitly "converting" (wrapping) them is a feature IMO, not a bug. |
(just to clarify, that's my stance too, as well as the other points in my next paragraph) |
I'm back and still thinking about enums! A problem we hit last time in #1217 was that generating Dart enums for native enums isn't always the right choice. As a workaround, we offered an opt-out, but what if we can have the best of both worlds? Instead of enums, how about extension types?
I'm not 100% sure this is inherently helpful, but one pattern I noticed is that packages that opt out of Dart enums lose the custom type names, and their APIs are covered in
int
s. So even if not everyone needsoperator |
orbool has()
, they can at least use the extension type to refer to their constants and wrap new integers at runtime.@dcharkes, thoughts?
The text was updated successfully, but these errors were encountered: