Skip to content
This repository was archived by the owner on Jan 28, 2024. It is now read-only.

Conversation

@mannprerak2
Copy link
Contributor

@mannprerak2 mannprerak2 commented Oct 4, 2022

Closes dart-lang/native#539

Summary -

A library can now export the symbols by specifying

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

Exported symbol.yaml file looks like

format_version: "1.0.0"
files:
  ../generated/base_gen.dart:
    used-config:
      ffi-native:false
    symbols:
      c:@E@BaseEnum:
        name: "BaseEnum"
      c:@F@base_func1:
        name: "base_func1"

To import symbols,-

import:
  symbol-files:
    - '../generated/base_symbols.yaml' # Package Uri is also allowed here

Doing so will

  1. Type map the shared types (Structs, Unions, Typedefs) from symbol-file.
  2. Other types with same USRs (such as functions, enums) will not be generated (since they are already defined in the imported library)

@mannprerak2
Copy link
Contributor Author

@dcharkes @mahesh-hegde Just a small POC as of now, checkout - example/shared_bindings.

Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Some high level questions. (Maybe better discussed on https://github.com/dart-lang/ffigen/issues/22 rather than the PR.)

@mannprerak2
Copy link
Contributor Author

mannprerak2 commented Oct 5, 2022

@dcharkes I've added a summary in the top comment (#468 (comment)).

Let me know when you think this is feature ready, so I can add tests, update readme, etc.

@dcharkes
Copy link
Contributor

dcharkes commented Oct 6, 2022

@dcharkes I've added a summary in the top comment (#468 (comment)).

Thanks! That's a very good write up 👍
I've added my reply in the issue dart-lang/native#539, then it'll be easier to find for me later.

@mannprerak2 mannprerak2 force-pushed the symbol_file_import_export branch from 6db63f0 to e06ecd1 Compare October 16, 2022 11:54
Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with comments (and api discussion in issue)

@mannprerak2
Copy link
Contributor Author

@dcharkes And 3 weeks later, this is finally ready 😝 . I've added tests, updated the readme, and bumped the version.

@mannprerak2
Copy link
Contributor Author

mannprerak2 commented Oct 23, 2022

@dcharkes it seems like macros can have a different USR after all. Not sure why this is the case, but the windows test are failing due to this.
Here's a macro USR- c:base.h@496@macro@BASE_MACRO_1. The number (496 here) can possibly change

For now I've simply removed macros from example/shared_bindings to prevent tests from failing on windows.

@dcharkes
Copy link
Contributor

@dcharkes And 3 weeks later, this is finally ready 😝 . I've added tests, updated the readme, and bumped the version.

No problemo! Thanks for all your contributions to this open source project!! 🚀

it seems like macros can have a different USR after all. Not sure why this is the case, but the windows test are failing due to this. Here's a macro USR- c:base.h@496@macro@BASE_MACRO_1. The number (496 here) can possibly change

For now I've simply removed macros from example/shared_bindings to prevent tests from failing on windows.

Should we refuse to emit macros in the symbols file then? And emit an error? (And a test that checks that an error is emitted.)

@mannprerak2
Copy link
Contributor Author

mannprerak2 commented Oct 24, 2022

Should we refuse to emit macros in the symbols file then?

Yeah, I guess we can.

And emit an error?

Do you mean we raise an error and exit without generating the symbol file? Or is it a just warning(or maybe just an info log) which mentions that macros have been removed since they cannot be cross-referenced reliably?


I think we're probably seeing this since C allows the redefinition of macros, (albeit with a warning from the compiler).

/headers/a.h:17:9: warning: 'BASE_MACRO_1' macro redefined [Lexical or Preprocessor Issue]

@dcharkes
Copy link
Contributor

Or is it a just warning(or maybe just an info log) which mentions that macros have been removed since they cannot be cross-referenced reliably?

Maybe that is good enough. Let's go for that, and then when we start using this feature more, we can see how that works out in practise.

Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet stuff!

@dcharkes dcharkes merged commit efa69bc into dart-archive:master Oct 24, 2022
@mannprerak2 mannprerak2 deleted the symbol_file_import_export branch October 24, 2022 10:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Sharing shared definitions

2 participants