Make extern Swift functions public to prevent stripping in Release builds

[jmp-0x7C0]

I had a quick stab at fixing #166, if I have understood the instructions correctly this would resolve the issue at the cost of changing the visibility of functions in the module.

@chinedufn let me know if I'm on the right track here.

So far I have only added tests for the swift-bridge-ir crate, I'm guessing it might make sense to also add a small repro example to run xcodebuild in release mode on CI.

[chinedufn]

I'm guessing it might make sense to also add a small repro example to run xcodebuild in release mode on CI.

This is a very good idea.

Here's one solution off the top of my head

---

• mkdir -p SwiftRustIntegrationTestRunner/test-release

• touch SwiftRustIntegrationTestRunner/test-release/README.md

• In the README mention that we're confirming that the generated FFI glue does not get optimized away in release mode. Mention that we test it in its own crate so that the rests of our integration tests can run in a Debug mode (faster tests)

• For inspiration on how to create a simple Swift+Rust binary, look at https://github.com/chinedufn/swift-bridge/tree/53b118d17f2f1a2922e969de528b99a2ffbc7dde/examples/async-functions

• Confirm that without your changes SwiftRustIntegrationTestRunner/test-release cannot be compiled

• Confirm that with your changes SwiftRustIntegrationTestRunner/test-release can be compiled

• Run ./SwiftRustIntegrationTestRunner/test-release/build.sh at the bottom of

  swift-bridge/test-swift-rust-integration.sh

  Line 1 in 53b118d

  #!/bin/bash

• Now we can be confident that release builds work

Just a quick idea. You might have a better solution in mind.

[jmp-0x7C0]

@chinedufn Ok I've create both a reproduction case that fails to build against the master branch, and a case that builds successfully against this branch.

I'm not sure we want this amount of code duplication for the repro case and the success case. There's probably also a smarter way to specify which versions of the dependencies to build e.g. using the cargo --config flag but I couldn't get it to work so I just created two separate projects. Also if this PR would be merged as is the tests would start failing as I'm currently pointing the failing case at the master branch.

If you have any suggestions for how to structure this code better, avoid the duplication etc. I'd be happy to make those changes.

I did have a go at reproducing the issue by just calling swiftc directly in the same way as the async-functions example but with release optimisation flags and code stripping enabled but I couldn't reproduce the issue. I also tried using swift build -c release, which also built successfully. But I was immediately able to reproduce the issue by creating a Swift Package and calling xcodebuild archive as reported by @bes.

[chinedufn]

Thanks for putting this together.

I'm not sure we want this amount of code duplication for the repro case and the success case.

We don't need to maintain two cases.

The purpose of making the failure case was for us to be sure that we weren't accidentally landing a test that would have always passed.

For example, you found that the swift build -c release example was always passing, so that would have been a bad test.

we just need to land one of the test crates, not both. Then we can have confidence that your code for going from func -> public func is working as intended.

I did have a go at reproducing the issue by just calling swiftc directly in the same way as the async-functions example but with release optimisation flags and code stripping enabled but I couldn't reproduce the issue.
I also tried using swift build -c release, which also built successfully. But I was immediately able to reproduce the issue by creating a Swift Package and calling xcodebuild archive.

Great. Thanks for figuring this out.

In the test crate's README Let's document what we tried and why we think it didn't work.

This helps with:

• future maintainers will know what we've tried so that they don't try and do it themselves unless they think they have a new idea on how to approach it

• someone in the future might have an idea of how to simplify the test by making it not need to build a Swift packages / using xcode

In short, a maintainer should be able to clearly understand why we are using a more complex solution (Swift Package + xcodebuild) instead of a simpler one (swiftc).

[chinedufn]

Great work on this. I left some minor feedback, but overall this looks good.

Thanks for coming up with the idea for testing this and figuring out a nice and clean way to do it.

[chinedufn]

Let's document:

• Why it would infer that they were dead code (as in, mention func vs. public func)

• show an example of a problematic cdecl function. Explain that we now prepend public.

  @_cdecl("__swift_bridge__$add") 
  func __swift_bridge__add (ptr: UnsafeMutableRawPointer) { /* .. */ }

  • This will help orient the reader and give them a better understanding of what we're testing

• How the test works. How are we accomplishing what you've explained here? Maybe explain how the test works step by step.

• How to run the test.

• Mention that this test gets ran during test-swift-rust-integration.sh

All of this will help future maintainers understand what's going on and be in a better position to make changes/improvements to it.

[chinedufn]

Maybe call_rust() to make it clear that we're calling into Rust.

Right now it seems like we're calling a Swift function.

[chinedufn]

Great docs

[chinedufn]

We don't need to maintain two cases.

When I said to make a failing cast I just meant to

1. Make it fail
2. Change swift-bridge
3. Confirm that it now passes

So that we were sure that our changes actually worked and that we didn't:

1. Make a test that already passed (BAD)
2. Make swift-bridge changes
3. Confirm that test passes (BAD, it already passed to begin with. It was a bad test)

Does that make sense?

[chinedufn]

Are you able to complete this pull request? Looks like the main remaining work is to delete one of the test packages.

Is there anything that I can help with?

[extrawurst]

would be awesome to get this merged

[jmp-0x7C0]

@chinedufn

Are you able to complete this pull request?

Thank you for the review. I've been quite busy with other projects recently, but I should have a bit more time to get this finished next week.

[chinedufn]

Hello, hello. Anticipate having any time for this?