[RNE Rewrite] feat: add input validation and JSI conversion utilities#1296
[RNE Rewrite] feat: add input validation and JSI conversion utilities#1296barhanc wants to merge 21 commits into
Conversation
|
@msluszniak This is still a draft and there are still quite some rough edges to polish, but could you take a look if this approach is something you would agree on and if there are any other aspects I missed. I was thinking that we could also use this PR for #1268 since adding the validation/conversion helpers modifies quite a lot of core code so it would be good to review and streamline also the other aspects. |
msluszniak
left a comment
There was a problem hiding this comment.
I like this concept. I combines elegance of short macros with flexibility of manual code. Just very small nits, will dive into it later today / tomorrow. I like usage of std::format, we previously tried use it in old codebase, but because it required some version of iOS we eventually dropped it, but I don't think it will be a case now.
…ntime error handling
…operty for ArrayBuffer
… switch braces in model.cpp
… error message in model.cpp
|
The |
…ary asType template
…ness in TensorHostObject
…p and improve fromJs error reporting
…s::asType cleanups, and organize includes
…hape constraints syntax
Description
Refactors the C++ layer by pulling two families of scattered, copy-pasted utilities out of individual translation units and into purpose-built, reusable modules:
cpp/core/conversions.{h,cpp}JSI -> C++ type coercionscpp/core/tensor_helpers.{h,cpp}: tensor acquisition, locking, and shape/dtype validationMotivation
The old code had two recurring problems that became impossible to ignore as the extension surface grew:
JSI coercion was inlined everywhere
Every file that needed to read a number, string, or array out of a
jsi::Valuehad its own copy of the same defensive checks, cast chains, and error-message construction. A bug or policy change (e.g. hownullis treated, or which format an error message takes) had to be hunted down and patched in every translation unit independently.Tensor validation was duplicated across all extension operations
Every operation in
box_ops,image_ops,operations, andtokenizerthat accepted aTensorargument re-implemented the same sequence: unwrap thejsi::Value, cast toTensorHostObject, acquire the appropriateshared/exclusive lock, and assert dtype + shape constraints. This led to subtle inconsistencies in error messages and locking strategies between operations.
Introduces a breaking change?
Type of change
Tested on
Testing instructions
Screenshots
Related issues
Closes #1270
Closes #1268
Checklist
Additional notes
Since this PR changes quite a lot of stuff in
core/it can also be used for #1268