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

Get external errors working for Swift and Python #2390

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

mhammond
Copy link
Member

@mhammond mhammond commented Jan 7, 2025

Almost - Swift+Python work. Kotlin doesn't work (#2392), however it works around failing simply by declining to render any functions with external errors, which seems reasonable for now.

@mhammond mhammond marked this pull request as ready for review January 9, 2025 15:23
@mhammond mhammond requested a review from a team as a code owner January 9, 2025 15:23
@mhammond mhammond requested review from gruberb and removed request for a team January 9, 2025 15:23
@@ -576,6 +576,15 @@ impl<T: AsType> AsCodeType for T {
}
}

// A work around for #2392 - we can't handle functions with external errors.
fn can_render_callable(callable: &dyn Callable, ci: &ComponentInterface) -> bool {
Copy link
Member Author

Choose a reason for hiding this comment

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

@bendk this is what I was thinking re our discussions around tests etc - at some point it seems reasonable for binding generators to simply decline to render things they can't handle - eg, I could imagine a new bindgen might start by declining to implement functions which try and use anything other than primitive types as they are starting out etc.

@mhammond mhammond changed the title Get external enum errors working. Get external errors working for Swift and Python Jan 9, 2025
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.

LGTM, hopefully we can get kotlin working somewhat soon, do you think that's a requirement for the release?

@mhammond
Copy link
Member Author

do you think that's a requirement for the release?

I don't think so, no. I'm actually becoming convinced it's really not at all trivial to fix given how our "companion" objects and external rust buffers are setup (but I could be very wrong about that)

@mhammond mhammond merged commit 9eb0bae into mozilla:main Jan 10, 2025
5 checks passed
@mhammond mhammond deleted the push-tutvkrzuvoks branch January 11, 2025 01:17
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