dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
155 stars 43 forks source link

How will YAML change as we add more languages? #289

Open liamappelbe opened 2 years ago

liamappelbe commented 2 years ago

We need to figure out how to adapt the YAML config as we add more languages (eg ObjC, Java, Swift, Kotlin).

For the input source files, for Java/Kotlin/Swift we can just look at the file extension, but ObjC also uses .h files. The easiest solution would just be to add an objc-headers key, at the same level as headers, and I think that would also make sense for the other languages.

The main issue I see is that most of the other top level config keys are either not applicable to some languages (eg structs/typedefs/unions/macros), or we'd want them separated by language (eg functions might have different naming conventions in different languages, so the regexes would look different).

Should we just add a bunch more top level keys, but with language specific prefixes? Eg objc-functions, java-classes? Or maybe we should totally restructure the format so that the language is at the top level:

ffigen:
  output:
  c:
    headers:
    functions:
    ...
  objc:
    headers:
    classes:
    ...
mannprerak2 commented 2 years ago

How about adding a top level lang key to the config.

#  pubspec.yaml
ffigen:
  lang: 'c'
  headers: ...
  functions: ...

This still doesn't provide a way to specify multiple configurations in a single file, but that seems rare since bindings would usually be abstracted away as packages. And one can always specify multiple config files in that case. (Each for a different language/binding)

dcharkes commented 2 years ago

We already have multiple config files in case of conceptually separate binding sets in one language, see for example 2 ffigen.yamls in:

If we'd want to support generating multiple binding sets from a single config, we should maybe also allow that for multiple times the same language.

@liamappelbe do you have a use case where you'd want two languages in one config?

Both yaml designs seem fine to me for supporting different keys for different languages.

liamappelbe commented 2 years ago

I don't have a use case where we'd need multiple languages in one package, but maybe it'll come up when we support both ObjC and Swift. In any case, if we can have multiple config files, then one for each language, with a lang key, sounds reasonable to me. The only issue I see is that each language is only going to support a subset of the other fields. I suppose that's mostly just a documentation and validation issue though.

liamappelbe commented 2 years ago

@mannprerak2 @dcharkes After Prerak's comment on dart-lang/ffigen#313, I think that methods need a different approach. Similar to how member-rename allows renaming a member of a specific struct, I think users are going to want to be able to include/exclude methods of specific interfaces. So having a top level objcMethods config isn't a good idea, as it would apply to all methods in all interfaces. I'm thinking of doing something like this instead:

objcInterfaces:
  member-rename:
    'NSString':
      ...
  member-include:
    'NSString':
      - '.*'
  member-exclude:
    'NSString':
      - 'length'

So member-rename applies to interface methods the same way it applies to struct members. And member-include/exclude have the same sort of string keys as member-rename, but full include/exclude lists as values.

I might try to land dart-lang/ffigen#313 with just the objcInterfaces config key for now, and just remove the objcMethods config, until we figure out the details of how to hand methods.

dcharkes commented 2 years ago

That does make a lot of sense indeed. Methods are nested in interfaces/classes in the same way fields are nested in structs.

Given that the member rename for structs is structs -> member-rename -> [struct match] -> [member match] : [new name], we can follow the same pattern as there as you suggested.

structs:
  rename:
    # Removes prefix underscores
    # from all structures.
    '_(.*)': '$1'
  member-rename:
    '.*': # Matches any struct.
      # Removes prefix underscores
      # from members.
      '_(.*)': '$1'

(And yes, then we can have include/exclude members as you suggest, which don't make sense for structs, but do for methods in interfaces.)

I'm wondering if it is worth considering changing up the nesting to: structs -> [struct match] -> member-rename -> [member match] : [new name]. That would keep all configuration for a single struct, or in this case a single objectiveC interface closer together. cc @mannprerak2, can you remember why we chose the nesting we did for structs? One problem with reversing the order is that rename is at the same level as the [struct match].

@liamappelbe wdyt about the nesting order for the config?

mannprerak2 commented 2 years ago

The nesting order was mostly because we have regexp matching support. So if one wants to do a member rename across all structs, that would be possible (commonly used for removing the prefix underscore)

liamappelbe commented 2 years ago

I think nesting rename, include, and exclude under [struct-match] makes logical sense. This would also allow us to unify the Renamer and MemberRenamer, and reuse the Includer rather than building a new MemberIncluder.

We'd still be able to do regexp matching on the structs.

dcharkes commented 2 years ago

It would be a breaking change for structs API. Though I'm fine with that if we give people a nice migration message on the old syntax.

It is possibly best to do the breaking change for structs first before landing objInterfaces, to keep things consistent.

mannprerak2 commented 2 years ago

It is possibly best to do the breaking change for structs first before landing objInterfaces, to keep things consistent.

So are we planning remove the nesting in config then?

dcharkes commented 2 years ago

I was thinking something like:

structs:
  rename:
    # Removes prefix underscores
    # from all structures.
    '_(.*)': '$1'
  '.*': # Matches any struct.
    member-rename:
      # Removes prefix underscores
      # from members.
      '_(.*)': '$1'

Of course, that means we cannot match a struct called rename anymore. Or did you have something different in mind @liamappelbe

liamappelbe commented 2 years ago

Yeah, that was what I was thinking. But I hadn't considered that we wouldn't be able to match a rename struct. Tbh, I don't know enough about YAML to know when it decides what is a string and what isn't (I thought it was the quotes).

Maybe the rename limitation doesn't matter too much, because in the unlikely event that this happens, they would get an error because the stuff nested under the rename key would have the wrong structure. It wouldn't silently do unexpected stuff. And in that case they could switch to a regex like '[r]ename':.