Skip to content

Fix compiling issue in Swift bindings for async throw functions #2456

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

Merged

Conversation

crazytonyli
Copy link
Contributor

<ErrorType>.lift Swift functions are private API and are referenced in errorHandler: {{ e|ffi_error_converter_name }}.lift. That would cause compiling issues in Swift bindings of a crate that uses external error types in async functions.

The first commit reproduces the issue, and the second commit fixes the problem.

@crazytonyli crazytonyli requested a review from a team as a code owner February 26, 2025 23:56
@crazytonyli crazytonyli requested review from bendk and removed request for a team February 26, 2025 23:56
@crazytonyli
Copy link
Contributor Author

BTW, this PR is potentially related to #2153. I'm not entirely sure because I haven't read the entire discussion there.

@bendk
Copy link
Contributor

bendk commented Mar 3, 2025

This looks good to me, thanks for the patch!

Can you merge this together into 1 commit and re-push? I think 1 commit is better to merge, since it works better with git-bisect. Also, I'm hoping that a re-push will unbreak the circle CI pipeline.

@crazytonyli crazytonyli force-pushed the bugfix/throw-external-error-in-async-func branch from 6262cf4 to c354ae0 Compare March 3, 2025 21:24
@crazytonyli
Copy link
Contributor Author

@bendk Done. All changes are in one commit now.

@bendk
Copy link
Contributor

bendk commented Mar 4, 2025

We're really close here:

  • Can you rebase one more time? We just merged a fix for the rustup breakage. Hopefully that will make CI run.
  • When I run this with Swift 6, I get errors from calling an async function outside of an async context. I updated the test to use a task here: 78cdd16 how does that look to you? Feel free to merge it or make some other changes, I'm not tied to that code.

@crazytonyli crazytonyli force-pushed the bugfix/throw-external-error-in-async-func branch from c354ae0 to f3442a0 Compare March 5, 2025 05:59
@crazytonyli
Copy link
Contributor Author

@bendk I have added your changes to this PR. It's probably an "issue" of Swift Linux, because I can run the test just fine without Task on my mac.

Copy link
Contributor

@bendk bendk left a comment

Choose a reason for hiding this comment

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

Still not sure what's going on with CI, but this change looks good to me.

@bendk bendk merged commit 2f31e34 into mozilla:main Mar 5, 2025
1 check failed
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