Enum names

[dcharkes]

We don't compile C enums to Dart enums, because in Dart we cannot define the integer value of enum items.

However, with our current compilation scheme we don't have access to the string enum names.

abstract class Cronet_RESULT {
  static const int Cronet_RESULT_SUCCESS = 0;
  static const int Cronet_RESULT_ILLEGAL_ARGUMENT = -100;
  // etc.
}

How about we add:

abstract class Cronet_RESULT {
  // <what we are already generating>

  // Supports having duplicate integers with the same key.
  static const Map<String, int> _nameToValue = {
    'Cronet_RESULT_SUCCESS': 0,
    'Cronet_RESULT_ILLEGAL_ARGUMENT': -100,
    // etc.
  };

  // Does not support having duplicate integers with the same key, picks the (first? last?) name.
  static final Map<int, String> _valueToName = Map.fromEntries(
      _nameToValue.entries.map((e) => MapEntry(e.value, e.key)));

  static String toEnumName(int enumValue) {
    return _valueToName[enumValue]!;
  }

  static int toEnumValue(String enumName) {
    return _nameToValue[enumName]!;
  }
}

Any preference on what the static methods should be named?

And of course they should have proper documentation. 😄

We could make it optional behind a flag (some libraries like sqlite have an API that you pass in the integer and it gives you a full string sentence explaining what's going on, so you might not want to generate the String enum names for that library). But maybe it should be enabled by default.

cc @mannprerak2 @unsuitable001

Context: https://github.com/google/cronet.dart/pull/2/files

More background: toString() on enum values in Dart gives you access to the programmatic name: https://stackoverflow.com/questions/29567236/dart-how-to-get-the-value-of-an-enum/29567669

[TimWhiting]

Alternatively why not:

enum Cronet_RESULT {
  Cronet_RESULT_SUCCESS,
  Cronet_RESULT_ILLEGAL_ARGUMENT,
  // etc.
}

extension CronetResultValue on Cronet_RESULT {
  int get value {
    switch (this) {
      case Cronet_RESULT_SUCCESS:
         return 0;
      case Cronet_RESULT_ILLEGAL_ARGUMENT:
         return -100;
    }
  }
 // ... name etc
}

This is where a static extension to map from int to enum would be helpful:

extension CronetResultValue on Cronet_RESULT {
  static Cronet_RESULT fromInt(int val) {
    switch (val) {
      case 0:
         return Cronet_RESULT_SUCCESS;
      case -100:
         return Cronet_RESULT_ILLEGAL_ARGUMENT;
    }
  }
}

Unfortunately static extension members are not supported yet by dart so it would have to be a top level method currently https://github.com/dart-lang/language/issues/723

[dcharkes]

The problem is that in C/C++ you can define multiple enum names with the same value:

enum Cronet_RESULT {
    Cronet_RESULT_SUCCESS = 10,
    Cronet_RESULT_ILLEGAL_ARGUMENT = 10,
    // ...
};

Using a dart enum that is not possible.

That would cause Cronet_RESULT_SUCCESS == Cronet_RESULT_ILLEGAL_ARGUMENT in C, but Cronet_RESULT_SUCCESS != Cronet_RESULT_ILLEGAL_ARGUMENT in Dart.

[unsuitable001]

Can we take help from package:build_value? See: https://github.com/google/built_value.dart#enum-class

[mannprerak2]

I think we can take design inspiration, but since we are already generating code we shouldn't depend on this library.

[mannprerak2]

Since dart 2.17 now has enhanced enums with members, we can change the way we generate enums to be more type safe as well.

[mannprerak2]

I was trying utilize enhanced enums but I found a usability concern when using it in switch by value -

switch (cursor.kind) {
    case clang_types.CXCursorKind.CXCursor_ObjCSuperClassRef.value: // Error: case must be constant.
      break;
  }

One solution to this is to introduce a defaultcase in all enums, and have a static function on each enum fromValue(), the user can then change the input of the switch statement.

E.g for an enum A

enum A{
  a,
  b
};

Generated code can be

enum A {
  a(0, 'a'),
  b(1, 'b'),
  defaultcase(2, 'defaultcase');

  final int value;
  final String name;
  const A(this.value, this.name);

  static A fromValue(int value) {
    return A.values
        .firstWhere((e) => e.value == value, orElse: () => A.defaultcase);
  }
}

wdyt? @dcharkes Or can we maybe expect the user to switch to using if-else instead?

[dcharkes]

The following would work, but we would still have an issue with duplicate definitions:

enum CXCursorKind {
  CXCursor_UnexposedDecl(1, 'CXCursor_UnexposedDecl'),
  CXCursor_StructDecl(2, 'CXCursor_StructDecl'),
  CXCursor_FirstDecl(1, 'CXCursor_FirstDecl');

  final int value;
  final String name;
  const CXCursorKind(this.value, this.name);
}

void main() {
  CXCursorKind a = CXCursorKind.CXCursor_UnexposedDecl;
  switch (a) {
    case CXCursorKind.CXCursor_UnexposedDecl:
    case CXCursorKind.CXCursor_FirstDecl:
      // both value 1
      break;
    case CXCursorKind.CXCursor_StructDecl:
      break;
  }
}

We cannot know whether to instantiate to CXCursor_UnexposedDecl or CXCursor_FirstDecl when we get an 1 from C.

I'm not sure how the new enums help with this.

@mannprerak2 What is the goal with the new approach? What can we improve over the current situation?

[mannprerak2]

The goal would probably be accessing the enum names, as you mentioned above

However, with our current compilation scheme we don't have access to the string enum names.

We can also make enums being passed to/from functions more typesafe.

[dcharkes]

Okay, how about we do use a single enum value for duplicate definitions, but we make name be a list instead:

enum CXCursorKind {
  CXCursor_UnexposedDecl(1, [
    'CXCursor_UnexposedDecl',
    'CXCursor_FirstDecl',
  ]),
  CXCursor_StructDecl(2, [
    'CXCursor_StructDecl',
  ]);

  static const CXCursor_FirstDecl = CXCursor_UnexposedDecl;

  final int value;
  final List<String> names;
  const CXCursorKind(this.value, this.names);
}

void main() {
  CXCursorKind a = CXCursorKind.CXCursor_UnexposedDecl;
  switch (a) {
    case CXCursorKind.CXCursor_UnexposedDecl:
    case CXCursorKind.CXCursor_FirstDecl:
      // both value 1
      break;
    case CXCursorKind.CXCursor_StructDecl:
      break;
  }
}

Possibly we could generate name as a String if we don't detect any duplicates. (Then the users' code would break if a duplicate definition is added, but maybe that's better then forcing them to always do .names.single everywhere.)

I'm just thinking out loud here. What do you think?

[mannprerak2]

I guess that sounds fine, we'd also need to add a defaultcase.

I wonder if this solves more than it disrupts, maybe we should wait for someone with a genuine usecase of enhanced enums, and even then we should put this behind a flag.

[TimWhiting]

I don't think it would disrupt too much if it only generated a list of names when there is a duplicate definition.

[dcharkes]

we'd also need to add a default case.

For invalid values? Shouldn't we just throw a sensible exception in this case? Or is it for a different reason?

[mannprerak2]

I found enhanced enums to be a bit verbose to be used in switch statements. (when switch condition is an Integer value) The defaultcase helps make that somewhat less verbose, another benefit is we can add type safety to enum return types on C functions.

[dcharkes]

The defaultcase helps make that somewhat less verbose

People are always free to add a default case with default: in the enum itself.

another benefit is we can add type safety to enum return types on C functions

I think we should throw when we get an integer from C land that is not defined as an enum case. Inventing a new case (which also requires us to put an int value in the value field) will make errors propagate. Throwing is also type safe.

An alternative could be to return a nullable value, that is also type safe. But since a wrong integer value as return value form C is an exceptional case, throwing an Exception is the right thing to do I believe.