NS_ENUM does not allow integer assignment

[brianquinlan]

The NS_ENUM macro allows for assignment of enum values other than those declared.

But the Objective-C ffigen does not allow that. For example:

typedef NS_ENUM(NSInteger, NSURLSessionWebSocketCloseCode)
{
    NSURLSessionWebSocketCloseCodeInvalid =                             0,
    NSURLSessionWebSocketCloseCodeNormalClosure =                    1000,
    NSURLSessionWebSocketCloseCodeGoingAway =                        1001,
    NSURLSessionWebSocketCloseCodeProtocolError =                    1002,
    NSURLSessionWebSocketCloseCodeUnsupportedData =                  1003,
    NSURLSessionWebSocketCloseCodeNoStatusReceived =                 1005,
    NSURLSessionWebSocketCloseCodeAbnormalClosure =                  1006,
    NSURLSessionWebSocketCloseCodeInvalidFramePayloadData =          1007,
    NSURLSessionWebSocketCloseCodePolicyViolation =                  1008,
    NSURLSessionWebSocketCloseCodeMessageTooBig =                    1009,
    NSURLSessionWebSocketCloseCodeMandatoryExtensionMissing =        1010,
    NSURLSessionWebSocketCloseCodeInternalServerError =              1011,
    NSURLSessionWebSocketCloseCodeTLSHandshakeFailure =              1015,
} API_AVAILABLE(macos(10.15), ios(13.0), watchos(6.0), tvos(13.0));

to

enum NSURLSessionWebSocketCloseCode {
  NSURLSessionWebSocketCloseCodeInvalid(0),
  NSURLSessionWebSocketCloseCodeNormalClosure(1000),
  NSURLSessionWebSocketCloseCodeGoingAway(1001),
  NSURLSessionWebSocketCloseCodeProtocolError(1002),
  NSURLSessionWebSocketCloseCodeUnsupportedData(1003),
  NSURLSessionWebSocketCloseCodeNoStatusReceived(1005),
  NSURLSessionWebSocketCloseCodeAbnormalClosure(1006),
  NSURLSessionWebSocketCloseCodeInvalidFramePayloadData(1007),
  NSURLSessionWebSocketCloseCodePolicyViolation(1008),
  NSURLSessionWebSocketCloseCodeMessageTooBig(1009),
  NSURLSessionWebSocketCloseCodeMandatoryExtensionMissing(1010),
  NSURLSessionWebSocketCloseCodeInternalServerError(1011),
  NSURLSessionWebSocketCloseCodeTLSHandshakeFailure(1015);

  final int value;
  const NSURLSessionWebSocketCloseCode(this.value);

  static NSURLSessionWebSocketCloseCode fromValue(int value) => switch (value) {
        0 => NSURLSessionWebSocketCloseCodeInvalid,
        1000 => NSURLSessionWebSocketCloseCodeNormalClosure,
        1001 => NSURLSessionWebSocketCloseCodeGoingAway,
        1002 => NSURLSessionWebSocketCloseCodeProtocolError,
        1003 => NSURLSessionWebSocketCloseCodeUnsupportedData,
        1005 => NSURLSessionWebSocketCloseCodeNoStatusReceived,
        1006 => NSURLSessionWebSocketCloseCodeAbnormalClosure,
        1007 => NSURLSessionWebSocketCloseCodeInvalidFramePayloadData,
        1008 => NSURLSessionWebSocketCloseCodePolicyViolation,
        1009 => NSURLSessionWebSocketCloseCodeMessageTooBig,
        1010 => NSURLSessionWebSocketCloseCodeMandatoryExtensionMissing,
        1011 => NSURLSessionWebSocketCloseCodeInternalServerError,
        1015 => NSURLSessionWebSocketCloseCodeTLSHandshakeFailure,
        _ => throw ArgumentError(
            "Unknown value for NSURLSessionWebSocketCloseCode: $value"),
      };
}

In Objective-C, it would be possible to use your own close code (e.g. 3000), but this is not possible in ffigened Objective-C APIs.

[liamappelbe]

The enum codegen was recently overhauled: #1202

Can you just use the NSURLSessionWebSocketCloseCode constructor directly?

If that doesn't work maybe we should put the new enum codegen behind a config flag.

[brianquinlan]

Can you just use the NSURLSessionWebSocketCloseCode constructor directly?

That won't work because of https://dart.dev/tools/diagnostic-messages#invalid_reference_to_generative_enum_constructor

[liamappelbe]

@Levi-Lesches Can you make the new style enums configurable? Ideally I think we'd be able to configure specific enums to use the old integer style. But if that's too much effort I suppose you could just add a global bool flag that switches all enums to the old style. This would also resolve this comment.

[Levi-Lesches]

Wow, I didn't know you could just do that. I guess I assumed a sort of runtime failure would just happen, but I never confirmed for myself. Yeah, that would entirely preclude using Dart-style enums.

Should we add a section to the config yaml? I think that's the best answer. I can try to have that done this week. Something like:

enum-classes:
  # Anything in this section gets an enum-like class instead of a true enum
  - NSURLSessionWebSocketCloseCode

[liamappelbe]

Should we add a section to the config yaml? I think that's the best answer. I can try to have that done this week. Something like:

enum-classes:
  # Anything in this section gets an enum-like class instead of a true enum
  - NSURLSessionWebSocketCloseCode

Yeah, something like that. Instead of adding a new top-level config option though, you can add a sub-option to the enums: config. Eg:

enums:
  enum-class:
    # These enums generate a class with ordinary int fields instead of a real Dart enum.
    include:
      - 'NSURLSessionWebSocketCloseCode'

For an example of adding a sub-option, you can check out functions.leaf. You can basically just copy/paste the existing Includer leafFunctions plumbing.

[brianquinlan]

One downside of using Dart enums by default is that, if Apple adds additional values, it may break the generated Dart code in two ways:

1. if the bindings are not regenerated then you'll get an ArgumentError exception if a new value is passed to fromValue.
2. if the bindings are regenerated an new enum values appear, then Dart code may break because a previously exhaustive switch is now non-exhaustive.

Mapping NS_CLOSED_ENUM to Dart enum and NS_ENUM to integer constants would make sense to me but I don't have strong feelings about this.

[dcharkes]

Config option sgtm. include sgtm

Name of the config option, maybe enums -> as-int?

enums:
  as-int:
    # These enums generate a class with ordinary int fields instead of a real Dart enum.
    include:
      - 'NSURLSessionWebSocketCloseCode'

Or enums - > open-enum?

[Levi-Lesches]

@brianquinlan

1. if the bindings are not regenerated then you'll get an ArgumentError exception if a new value is passed to fromValue.

I'm not too concerned here because if you upgrade the binary, it's expected you might need to regenerate the headers, since anything else may have changed as well.

2. if the bindings are regenerated an new enum values appear, then Dart code may break because a previously exhaustive switch is now non-exhaustive.

That's actually what I was intending. If you want exhaustive switch statements, that means adding new cases when you have to. It'd be more confusing imo if you tried writing an exhaustive switch and didn't have all your values covered. In general, I don't think regenerating bindings for headers that have changed is necessarily a non-breaking change.

Also, as an aside (since I don't use Objective-C), is this a common case? I was under the impression that if a function takes an enum value in its signature, then that's a signal from the author that it is only supposed to take those enumerated values, no? Is it common for API authors to specify an enum type but also accept any integer as well?

Mapping NS_CLOSED_ENUM to Dart enum and NS_ENUM to integer constants would make sense to me but I don't have strong feelings about this.

@dcharkes Based on what I read in the documentation, this seems reasonable, but I don't know all the nuances. Would you prefer this or the as-int config option?

[dcharkes]

I'm not too concerned here because if you upgrade the binary, it's expected you might need to regenerate the headers, since anything else may have changed as well.

People might be trying to use a system library already present on a users' machine instead of bundling it with an app. But even in that case one should probably just open an issue and regen the bindings for the newest version of the system library.

Mapping NS_CLOSED_ENUM to Dart enum and NS_ENUM to integer constants would make sense to me but I don't have strong feelings about this.

@dcharkes Based on what I read in the documentation, this seems reasonable, but I don't know all the nuances. Would you prefer this or the as-int config option?

If we can go without a config option because the source code has enough info I prefer that! 👌 (The best API is no API! The best configuration is no configuration! Less complexity for users.)

[Levi-Lesches]

Hm, I looked through the parsing code... Seems we're reliant on clang here to pick this up. I found EnumDecl.isClosed, but as I don't have a Mac, @liamappelbe can you test this on an NS_ENUM and NS_CLOSED_ENUM to check if it returns false and true, respectively?

If that works, I think we can use that but given the earlier comment from the PR, maybe it's best to go with a config option as well, to cover the plain C case.

[liamappelbe]

@Levi-Lesches EnumDecl is from clang's C++ API, but we only have access to the C API. I spent some time trying to find an equivalent flag in the C API, but I couldn't find anything (their documentation is a bit of a mess, so it's possible that it exists but I just didn't find it). I also asked an LLM how to get this flag from a CXCursor using the C API but it just hallucinated some nonsense.

If you can find a way of getting that flag in the clang C API from the enum's CXCursor then I can try again. Otherwise just go with the config approach.