Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swift external type module config #1404

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

bendk
Copy link
Contributor

@bendk bendk commented Nov 15, 2022

This is based off of #1389. Let's merge that first. Here are the changes compared to that one:

Added support for build processes where each generated .swift file is compiled into its own module rather than compiling all files together. This means that we need imports to make external types work.

  • Added test fixture and updated the testing code to test this
  • Added config code to allow the user to choose their mode
  • Updated the docs to describe the config options

External types require that one swift source file can use the
FfiConverter from another source file.  This means that the read/write
functions can't input a Reader/Writer class:

  - If the class is fileprivate, then this means the read/write methods
    must also be fileprivate since they're inputting that type, but this
    would totally prevent external types from working
  - If the class is internal/public, we'll get compile errors since both
    source files will try define the type.

Instead, read/write now input standard swift types, which allow them to
be shared between different source files.
"Handle" is a common name that's likely to cause name conflicts with
user-defined types.  This is exactly what happened when I tried to run
the ext-types tests, which include a type named "Handle".
This code assumes that all generate files will be compiled into a single
module, which makes external type support quite trivial.

- Updated the swift testing code to handle multiple UDL files.
- Made the custom type, enum, object, and record, FfiConverters public
  to allow them to be used with external types.
- Made the FfiConverter methods public.  I didn't make the protocols
  themselves public, since that would result in duplicate definitions
  when different UniFFI-generated swift files were linked together, but
  I did make each individual method public.
- Added Swift external types tests.
- Fixed a bug in the CustomType template.  I'm pretty sure we should be
  using `type_ffi_lowered` instead of `ffi_type_name` there since it's
  Swift code rather than a C header file.
- Added docs for configuring external types
Added support for build processes where each generated `.swift` file is
compiled into its own module rather than compiling all files together.
This means that we need imports to make external types work.

- Added test fixture and updated the testing code to test this
- Added config code to allow the user to choose their mode
- Updated the docs to describe the config options
@bendk bendk requested a review from a team as a code owner November 15, 2022 19:53
@bendk bendk requested review from badboy and removed request for a team November 15, 2022 19:53
@stan-irl
Copy link
Contributor

Are there any plans to merge this?

I think it would really help to make it a lot simpler to namespace and organise generated swift code. it seems very primitive to have to put everything in a single namespace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants