Skip to content

Conversation

@brendandahl
Copy link
Collaborator

Embind's subclass implement methods were generated as returning Class | null after the changes to pointer types in #22184. This could be considered a regression as the implement method would never return null.

Previously, we had special handling so constructors were marked as nonnull so in the TS definitions we didn't add | null. I've generalized this approach to work for all function bindings so they can now use a nonnull<ret_val> policy too avoid the | null.

Embind's subclass `implement` methods were generated as returning
`Class | null` after the changes to pointer types in emscripten-core#22184. This could
be considered a regression as the implement method would never return
null.

Previously, we had special handling so constructors were marked as
nonnull so in the TS definitions we didn't add `| null`. I've generalized
this approach to work for all function bindings so they can now use
a `nonnull<ret_val>` policy too avoid the `| null`.
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

I am not quite sure how this is meant to be used. Are there docs for it somewhere?

@kripken
Copy link
Member

kripken commented Sep 16, 2024

(lgtm otherwise!)

@kripken
Copy link
Member

kripken commented Sep 16, 2024

(also maybe mention in the changelog? but that can be with the docs - if docs are in a later PR, can wait on that)

@brendandahl
Copy link
Collaborator Author

I added some docs and a changelog entry.

@brendandahl brendandahl merged commit 2c44617 into emscripten-core:main Sep 16, 2024
1 of 2 checks passed
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