Sharing shared definitions

[dcharkes]

Consider the following scenario:

• header1 - with a struct
• header2 which imports header1 - having that struct in a signature
• header3 which imports header1 - having also that struct in the signature

Now we run the generator on header 2 and 3: they both generate the same struct.
These two structs will be Dart classes with the same name but incompatible.
You can work around that with Pointer.cast<>(), but that's not safe.
Also, this increases code size.

It would be better to be able to run the generator on all 3 headers files and let the generated files for header 2 and 3 import the generated file for header 1.

We need to think about a good design for this, especially if headers 2 and 3 are in different dart packages.

(I am not sure how common this scenario is. For example libclang and sqlite3 stand very much on themselves. But I can imagine structs defined in OS APIs fall into this category, so it would be nice to address this.)

[mannprerak2]

Notes from today's discussion (@dcharkes)

• We need a way to specify declarations dependencies that are already satisfied. (Maybe something like passing the config.yaml file of a binding as it is to some other package)
• Specifying imports needed for generated bindings.
• An option to disable automatically adding struct (Windows dart ci #23) (May not be needed if it's already part of the dependency)

[dcharkes]

Notes from today's discussion (@mannprerak2)

• We could have a serializable intermediate format, to avoid reparsing the dependencies.
  • This would change our architecture from parsing -> AST -> generating (resolves conflicting names) to parsing -> AST -> resolving conflicts -> AST2 (serializable) -> generating.

[dcharkes]

Some high level design questions/remarks.

We have two main use cases:

1. Having reusable library bindings in a separate package (For example standard library bindings.)
2. Having multiple generation steps in a single library. (For example the bindings for a native library and the bindings of some utility C code that the package author likes to keep separate from the native library. For example in package:webcrypto).

These two use cases mean we might both have import urls to other packages as well imports to relative paths.

Then we need to consider the users of reusable libraries, what is the minimal information they need to specify in their ffigen.yaml?

1. The absolute minimum is the path to the symbols.yaml (possibly in another package).

shared-bindings:
  base_h: # we don't actually need this name 
    package: 'foo' # optional, if omitted its a path in the package itself
    path: 'generated/base_symbols.yaml'

This means the generated file would need to contain the file paths.

2. We could refer to the ffigen.yaml instead, and fish the output path from there. (This would assume the ffigen invocation and the config are always in the same folder.)

[mannprerak2]

@dcharkes how are we planning to serve the symbol.yaml file to the user? Does pub allow packaging these?

And perhaps another wild idea could be to use annotations here somehow? Wdyt?

[dcharkes]

how are we planning to serve the symbol.yaml file to the user? Does pub allow packaging these?

Yes, anything that is not in .gitignore is packaged with a package on pub.

We can find a packageRoot of dependent packages by parsing .dart_tool/package_config.json with package:package_config, and then find the yaml within the package.

And perhaps another wild idea could be to use annotations here somehow? Wdyt?

That's a neat idea!

We could generate an annotation with the data in Dart maps/lists/sets/strings instead of putting it in a yaml. We could put it on every individual definition instead of building a map. I don't have so much experience with annotation processing to know how easy it is to find it on all definitions. Alternatively we could put an annotation on the top-class with the full mapping.

One potential downside of annotations is annotation parsing speed vs yaml parsing. We'd need to parse the whole Dart file. I'm not sure if this is a concern, we should check the speed for that.

Another potential downside of annotations is that we can't use a re-export mechanism. For example, I expect to generate bindings in lib/src/.../*.dart which should not be imported by other packages. And then re-export those bindings in lib/<my-package>.dart.

[dcharkes]

Reply to design in dart-archive/ffigen#468.

A library can now export the symbols by specifying

symbol-file: 
  output: '../generated/base_symbols.yaml'
  import-path: '../generated/base_gen.dart' # Warns if not using a package: URI.

lgtm

We could make import-path optional if we generate into lib/**.dart and not lib/src/**.dart, but then ffigen has to detect where the packageRoot is. If we're being run by dart run ffigen ... it should be fairly easy to detect the package root.

Exported symbol.yaml file looks like

import-path: ../generated/base_gen.dart
symbols:
  "c:@E@BaseEnum":
    name: BaseEnum
  "c:@F@base_func1":
    name: base_func1
  "c:@S@BaseStruct1":

Would we be ever in a situation in which we would want to have multiple import paths in a single mapping file? If so, it would be better to nest the symbols under the file path.

"package:foo/foo.dart" :
  symbols:
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

Maybe we should keep the option open that we might generate multiple files out of a single ffigen run in the future. (If we come up with some reason that our generated files are too big and we want to split them based on C source file structure for example.)
Also, maybe someone would like to combine multiple symbol.yamls into a single file for some reason.

This way we don't have to worry about a breaking change to the format later.

Thinking about breaking changes to this format, should we encode a format version in the file? Since packages can have different major versions of ffigen in their dev dependencies we can't rely on ffigen package versioning.

format_version: 1.0.0
"../generated/base_gen.dart" :
  symbols:
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

Now there is a question if we just care about the format itself, or also about the structure of the generated files. If we're also dependent on the structure of the generated files, it should probably encode the ffigen version instead. (At the downside that people might have to ping their dependencies to upgrade their ffigen and regenerate the file.)

ffigen_version: 7.0.0
symbols:
  "../generated/base_gen.dart" :
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

Or even:

"../generated/base_gen.dart" :
  ffigen_version: 7.0.0 # Enables combining 7.0.0 and 7.0.1 etc in a single file. But now we don't have a version at the top, so maybe a bad idea.
  symbols:
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

Also, we have config options that influence how the output is generated. For example the @FfiNatives put the symbols in the top level, so those config options need to be reflected in the yaml as well. For these options its probably best to just mirror the config for ffigen itself.

format_version: 1.0.0
"../generated/base_gen.dart" :
  used_config:
    ffi-native: # present or not
  symbols:
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

To import symbols, library-imports are used -

library-imports:
  old_format: 'old/format/still/works.dart'
  base_h:
    symbol-file: '../generated/base_symbols.yaml'
    import-path: '../generated/base_gen.dart' # Optional, can be used to override the value in symbol-file.

lgtm

Points to decide:

• Do we want to version with ffigen or do we want to version the format by itself.
  • With ffigen can lead to spurious version mismatches that would work in practise, but is easy.
  • It's own versioning we could forget to bump the symbols.yaml major version even though we do a breaking change to generated code.
• Do we want to support multi-file: I say yes. Even though right now we don't.

Points to fix:

• Add some kind of versioning.
• Add the config items that influence the output file. (ffi-native:)

wdyt @mannprerak2 (fyi @mahesh-hegde)

[mannprerak2]

This seems to be a good enough symbol file format to me

format_version: 1.0.0
"../generated/base_gen.dart" :
  used_config:
    ffi-native: true # or false
  symbols:
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

---

Do we want to version with ffigen or do we want to version the format by itself

How about putting the version of the least supported ffigen version in the generated symbols file? Each ffigen release will have to ensure that if there's a breaking change in symbols file or the generated bindings, we increment the least supported version.

Do we want to support multi-file: I say yes. Even though right now we don't.

The above symbol file lgtm. We can always increment the format version later if need be.

For example the @FfiNatives put the symbols in the top level,

How are we planning to use this information?

For libraries with different ffi-native flags (true/false)

1. Type sharing is only possible for NoLookupBindings so the imports will still work
2. However since we are also not generating the shared LookupBindings (since they are part of the shared bindings) libraries which have different ffi-native: flag are not compatible. The library author needs to either generate 2 bindings, (for true/false) and 2 symbol files each. Or in case the ffi-native flags are incompatible, we generate all the NoLookupBindings instead of excluding them.

[dcharkes]

How about putting the version of the least supported ffigen version in the generated symbols file? Each ffigen release will have to ensure that if there's a breaking change in symbols file or the generated bindings, we increment the least supported version.

We could do that, at the cost of newer versions of ffigen having to be able to keep parsing older format versions. If we need to update that number manually, I think it makes more sense to have a separate version for the format. We could also add a test to ensure we don't forget to bump the number of we break something by:

1. creating a temp folder
2. write a pubspec.yaml with a dev dependency on an older version of ffigen (we pin it in the test)
3. do dart pub get
4. write a symbols.yaml and ffigen.yaml
5. do dart run ffigen
   Then the test should fail if that older ffigen fails to run on the yaml.

1. Type sharing is only possible for NoLookupBindings so the imports will still work

Ah, you're correct. We're not actually invoking any of the functions bound to. 🙃

2. However since we are also not generating the shared LookupBindings (since they are part of the shared bindings) libraries which have different ffi-native: flag are not compatible.

I think mixing and matching FfiNatives with lookup is not going to work from a bundling libraries with a Dart of Flutter application point of view. Maybe the lookup ones could depend on the FfiNative ones, e.g. the bundling question should be solved by the Dart infrastructure later for FfiNatives, so layering lookup with which bundling/finding the dynamic library is manual could work.

How are they not compatible in the generated code? Can you paste a snippet with an example?

[dcharkes]

Maybe it would be slightly cleaner to put the map with files under its own key.

format_version: 1.0.0
files:
  "../generated/base_gen.dart" :
    used_config:
      ffi-native: true # or false
    symbols:
      "c:@E@BaseEnum":
        name: BaseEnum
      "c:@F@base_func1":
        name: base_func1

Then we don't have to disambiguate format_version and any other top level keys we add from file paths.