Skip to content

Conversation

@JakubGonera
Copy link
Contributor

Description

Make the model host function installation generic, so that you can easily introduce additional methods that resolve with a promise.

Tested on

  • iOS
  • Android

Related issues

#255

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings

@JakubGonera JakubGonera self-assigned this May 23, 2025
@JakubGonera JakubGonera requested a review from Copilot May 23, 2025 08:22
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors host function installation to be more generic by allowing promise-based function resolution. It updates the macro in JsiHostObject.h to accept a custom name and refactors ModelHostObject.h to use a templated host function for forwarding calls.

  • Updated JSI_EXPORT_FUNCTION macro to include a NAME parameter.
  • Refactored ModelHostObject to use a templated promiseHostFunction and updated error handling.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
common/rnexecutorch/jsi/JsiHostObject.h Updated macro to include a NAME parameter for improved clarity.
common/rnexecutorch/host_objects/ModelHostObject.h Refactored the host function to a generic templated version and adjusted argument count checks and error message formatting.
Comments suppressed due to low confidence (2)

common/rnexecutorch/host_objects/ModelHostObject.h:37

  • [nitpick] Consider using a std::string or another dynamic string formatting approach to construct the error message to avoid fixed buffer size constraints and potential overflow issues.
char errorMessage[100];

common/rnexecutorch/jsi/JsiHostObject.h:18

  • [nitpick] Ensure that all invocations of JSI_EXPORT_FUNCTION in the codebase are updated to pass the additional NAME parameter so that the changes remain consistent and clear.
#define JSI_EXPORT_FUNCTION(CLASS, FUNCTION, NAME)

@JakubGonera JakubGonera requested a review from chmjkb May 23, 2025 08:22
@chmjkb chmjkb merged commit 0442aa3 into @jakubgonera/cpp May 23, 2025
@chmjkb chmjkb deleted the @jakubgonera/cpp-host-function branch May 23, 2025 08:49
JakubGonera added a commit that referenced this pull request May 28, 2025
## Description

Make the model host function installation generic, so that you can
easily introduce additional methods that resolve with a promise.

### Tested on

- [x] iOS
- [x] Android

### Related issues

#255 

### Checklist

- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have updated the documentation accordingly
- [x] My changes generate no new warnings
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.

3 participants