[RNE Rewrite] feat: add input validation and JSI conversion utilities by barhanc · Pull Request #1296 · software-mansion/react-native-executorch

barhanc · 2026-07-01T14:38:10Z

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 coercions
cpp/core/tensor_helpers.{h,cpp}: tensor acquisition, locking, and shape/dtype validation

Motivation

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::Value had its own copy of the same defensive checks, cast chains, and error-message construction. A bug or policy change (e.g. how null is 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, and tokenizer that accepted a Tensor argument re-implemented the same sequence: unwrap the jsi::Value, cast to TensorHostObject, acquire the appropriate
shared/exclusive lock, and assert dtype + shape constraints. This led to subtle inconsistencies in error messages and locking strategies between operations.

Introduces a breaking change?

Yes
No

Type of change

Bug fix (change which fixes an issue)
New feature (change which adds functionality)
Documentation update (improves or adds clarity to existing documentation)
Other (chores, tests, code style improvements etc.)

Tested on

iOS
Android

Testing instructions

Build all example apps on both Android and iOS and test that nothing is broken.

Screenshots

Related issues

Closes #1270

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

Additional notes

Since this PR changes quite a lot of stuff in core/ it can also be used for #1268

barhanc · 2026-07-01T15:08:55Z

@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

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.

msluszniak · 2026-07-01T16:17:28Z

+template <>
+uint64_t asType<uint64_t>(jsi::Runtime &rt, const std::string &ctx, const jsi::Value &val) {
+    double v = asType<double>(rt, ctx, val);
+    if (std::isnan(v) || v < 0.0) {
+        throw jsi::JSError(rt, ctx + " must be a non-negative integer");
+    }
+    return static_cast<uint64_t>(v);
+}


there should also be check if double is in range of uint64_t, otherwise cast is undefined behaviour. This is the case for all analogical casts scenarios.

msluszniak · 2026-07-01T16:35:46Z

+void checkNotSameTensor(jsi::Runtime &rt,
+                        const std::string &name1, const std::shared_ptr<TensorHostObject> &t1,
+                        const std::string &name2, const std::shared_ptr<TensorHostObject> &t2) {
+    if (t1.get() == t2.get()) {


Why not t1 == t2?

msluszniak · 2026-07-01T16:40:14Z

+            if (i > 0) {
+                shapeStr += ", ";
+            }
+            const auto &dim = expectedShape->at(i);
+            if (std::holds_alternative<int32_t>(dim)) {
+                shapeStr += std::to_string(std::get<int32_t>(dim));
+            } else {
+                shapeStr += std::get<std::string>(dim);
+            }
+        }
+        shapeStr += "]";


Suggested change

if (i > 0) {

shapeStr += ", ";

}

const auto &dim = expectedShape->at(i);

if (std::holds_alternative<int32_t>(dim)) {

shapeStr += std::to_string(std::get<int32_t>(dim));

} else {

shapeStr += std::get<std::string>(dim);

}

}

shapeStr += "]";

const auto &dim = expectedShape->at(i);

if (std::holds_alternative<int32_t>(dim)) {

shapeStr += std::to_string(std::get<int32_t>(dim));

} else {

shapeStr += std::get<std::string>(dim);

}

shapeStr += ", ";

}

shapeStr.pop_back();

shapeStr += "]";

msluszniak

Another batch of comments.

msluszniak · 2026-07-01T17:08:12Z

+                    throw jsi::JSError(rt, std::format("{}: Failed to get input tag: {}", ctx, errorMsg));
                }

                switch (tag.get()) {


This input switch dropped the old Tag::None case (accept null → EValue()), so None-tagged/optional inputs now hit default: and throw "Unsupported input type". The output switch below still handles None. Was dropping it intentional?

msluszniak · 2026-07-01T17:08:12Z

-                    copyLen = static_cast<size_t>(optsObj.getProperty(rt, "length").asNumber());
-                }
-            }
+            const jsi::Object optsObj = (count == 2) ? args[1].asObject(rt) : jsi::Object(rt);


If count == 2 but args[1] isn't an object, asObject(rt) throws a generic JSI error; the old code silently ignored a non-object opts arg. Minor, but worth a contextual check to match the rest of the PR.

msluszniak · 2026-07-01T17:08:12Z

-                byteLength = static_cast<size_t>(byteLengthValue.asNumber());
-            }
+            const auto dataObj = args[0].asObject(rt);
+            const auto buffer = dataObj.getProperty(rt, "buffer").asObject(rt).getArrayBuffer(rt);


getData/setData lost the old "Expected a TypedArray with a 'buffer' property" message — a non-TypedArray now yields a generic JSI error. Slightly ironic for a PR standardizing errors; consider an asTypedArray helper in conversions.

msluszniak · 2026-07-01T17:08:12Z

-                throw jsi::JSError(rt, "through: First argument must be a function");
-            }
-
            auto fn = args[0].asObject(rt).asFunction(rt);


through/throughIf lost the explicit "must be a function" guard; a non-function arg now throws a generic asFunction error instead of the contextual one.

msluszniak · 2026-07-01T17:08:12Z

+    }
+
+    if (expectedShape) {
+        std::string shapeStr = "[";


shapeStr is built unconditionally on every call, including the success path — an avoidable allocation for per-frame CV ops. Move it into the error branches (or a lazy lambda).

msluszniak · 2026-07-01T17:08:12Z

+            } else {
+                const auto &symbol = std::get<std::string>(dim);
+                if (symbolMap.contains(symbol) && shape[i] != symbolMap[symbol]) {
+                    throw jsi::JSError(rt, std::format("{} must have shape {} (dim '{}' mismatch: expected {}, got {})",


The message says dim '{}' but is fed i, so it prints the index (dim '2') rather than the symbol name (dim 'N'). Pass symbol instead of i.

barhanc added 2 commits July 1, 2026 16:31

cpp/core: extract conversions and tensor_helpers modules

a507270

cpp: migrate callers to conversions and tensor_helpers

cff792a

barhanc self-assigned this Jul 1, 2026

barhanc added the refactoring label Jul 1, 2026

barhanc linked an issue Jul 1, 2026 that may be closed by this pull request

[RNE Rewrite] Add macros for input validation in native C++ implementation #1270

Open

cpp/core: fix size_t/uint64_t duplicate specialization on Linux

9c889c0

barhanc requested a review from msluszniak July 1, 2026 15:09

msluszniak reviewed Jul 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[RNE Rewrite] feat: add input validation and JSI conversion utilities#1296

[RNE Rewrite] feat: add input validation and JSI conversion utilities#1296
barhanc wants to merge 3 commits into
rne-rewritefrom
@bh/issue-1270

barhanc commented Jul 1, 2026 •

edited

Loading

Uh oh!

barhanc commented Jul 1, 2026

Uh oh!

msluszniak left a comment

Uh oh!

msluszniak Jul 1, 2026

Uh oh!

msluszniak Jul 1, 2026

Uh oh!

msluszniak Jul 1, 2026

Uh oh!

msluszniak left a comment •

edited

Loading

Uh oh!

msluszniak Jul 1, 2026

Uh oh!

msluszniak Jul 1, 2026

Uh oh!

msluszniak Jul 1, 2026

Uh oh!

msluszniak Jul 1, 2026

Uh oh!

msluszniak Jul 1, 2026

Uh oh!

msluszniak Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

barhanc commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation

JSI coercion was inlined everywhere

Tensor validation was duplicated across all extension operations

Introduces a breaking change?

Type of change

Tested on

Testing instructions

Screenshots

Related issues

Checklist

Additional notes

Uh oh!

barhanc commented Jul 1, 2026

Uh oh!

msluszniak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msluszniak left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

barhanc commented Jul 1, 2026 •

edited

Loading

msluszniak left a comment •

edited

Loading