Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .github/workflows/clang-tidy.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
name: clang-tidy

on:
pull_request:
branches:
- main
- rne-rewrite
paths:
- 'packages/react-native-executorch/cpp/**'
- 'packages/react-native-executorch/.clang-tidy'
- 'packages/react-native-executorch/compile_flags.txt'
- 'packages/react-native-executorch/scripts/clang-tidy.sh'
- '.github/workflows/clang-tidy.yml'
workflow_dispatch:

jobs:
clang-tidy:
runs-on: ubuntu-latest
# Dormant until the on-demand header artifact is published (#1283): clang-tidy
# must parse the sources, which needs the ExecuTorch/torch headers that are not
# committed. Enable by setting the repo variable ENABLE_CLANG_TIDY=true once a
# release carrying headers.tar.gz exists for the package version.
if: ${{ vars.ENABLE_CLANG_TIDY == 'true' }}
steps:
- name: Checkout
uses: actions/checkout@v6

- name: Setup
uses: ./.github/actions/setup

- name: Install clang-tidy
run: |
sudo apt-get update
sudo apt-get install -y clang-tidy
clang-tidy --version

- name: Provision ExecuTorch headers
# Reuse the on-demand download flow (#1283). clang-tidy only needs headers
# (it syntax-checks, no linking), so RNET_HEADERS_ONLY fetches just
# headers.tar.gz; RNET_TARGET avoids host platform auto-detection.
working-directory: packages/react-native-executorch
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
RNET_HEADERS_ONLY: '1'
RNET_TARGET: android-arm64-v8a
run: node scripts/download-libs.js

- name: Run clang-tidy
run: yarn workspace react-native-executorch lint:cpp
14 changes: 14 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,20 @@ don't conflict. clangd discovers `compile_flags.txt`/`.clangd` automatically fro
open file's directory. If you produce a `compile_commands.json` from a native build,
clangd will prefer it — it's gitignored and takes precedence over `compile_flags.txt`.

## clang-tidy

`packages/react-native-executorch/.clang-tidy` adds a conservative, high-signal set of
static checks (bugprone / performance / clang-analyzer) on top of the same database. The
clangd extension surfaces these inline; you can also run them from the command line:

```
yarn workspace react-native-executorch lint:cpp # all C++ sources
CLANG_TIDY=$(brew --prefix llvm)/bin/clang-tidy yarn workspace react-native-executorch lint:cpp
```

It needs the same provisioned headers as clangd. Suppress an intentional, reviewed finding
with a commented `// NOLINTNEXTLINE(check-name)` rather than loosening the shared config.

# Creating a Pull Request

Before writing any code reach out to us to make sure no one is currently working on it, you can always open an issue first.
Expand Down
74 changes: 74 additions & 0 deletions packages/react-native-executorch/.clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# clang-tidy configuration for the react-native-executorch C/C++ sources.
#
# A high-signal set focused on correctness/performance bugs and modern C++
# guidelines (issue #1271). Check groups are enabled one at a time and the
# findings fixed; it is expected to stay green on our own code.
#
# clang-tidy reads the include flags / standard / defines from compile_flags.txt
# (the same database clangd uses), so it needs the same prerequisites: a provisioned
# third-party/include and `yarn install`. The clangd extension also surfaces these
# checks inline in the editor.
#
# Excluded checks (incompatible with a native tensor/buffer library that interops
# with ExecuTorch over raw pointers, or purely opinionated):
# pro-bounds-pointer-arithmetic / -avoid-unchecked-container-access /
# -constant-array-index — raw buffer pointer math and hot-loop indexing are by design
# pro-type-reinterpret-cast — type-erased byte buffers cast to typed pointers
# avoid-magic-numbers / easily-swappable-parameters / enum-size — opinionated / noisy
# llvm-header-guard — the project uses #pragma once by convention
# llvm-prefer-static-over-anonymous-namespace — anonymous namespaces are a valid
# idiom here and also hold the helpers' shared types/constants
# misc-include-cleaner — ExecuTorch umbrella headers make it misreport includes
# misc-multiple-inheritance — the JSI HostObject + enable_shared_from_this pattern
# misc-non-private-member-variables-in-classes — HostObjects are deliberate public
# data carriers read across extensions
# misc-const-correctness — applied once as a cleanup, but too aggressive to gate on
# (it flags RAII lock guards as const-able); not enforced going forward
# modernize-use-trailing-return-type — the codebase uses traditional return types
# modernize-return-braced-init-list — `return {…}` hides the (JSI) return type
# readability-identifier-length — short names (i, rt, h, w, …) are idiomatic here
# readability-math-missing-parentheses — flags standard array-offset arithmetic
# readability-function-cognitive-complexity — the JSI get() dispatchers and the
# image/tensor kernels are inherently above the threshold
# readability-uppercase-literal-suffix — the codebase uses lowercase (1.0f) consistently
# readability-magic-numbers — opinionated (same as cppcoreguidelines-avoid-magic-numbers)
Checks: >
-*,
bugprone-*,
performance-*,
clang-analyzer-*,
cppcoreguidelines-*,
google-*,
llvm-*,
misc-*,
modernize-*,
readability-*,
-bugprone-easily-swappable-parameters,
-performance-enum-size,
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
-cppcoreguidelines-pro-bounds-avoid-unchecked-container-access,
-cppcoreguidelines-pro-bounds-constant-array-index,
-cppcoreguidelines-pro-type-reinterpret-cast,
-cppcoreguidelines-avoid-magic-numbers,
-llvm-header-guard,
-llvm-prefer-static-over-anonymous-namespace,
-misc-include-cleaner,
-misc-multiple-inheritance,
-misc-non-private-member-variables-in-classes,
-misc-const-correctness,
-modernize-use-trailing-return-type,
-modernize-return-braced-init-list,
-readability-identifier-length,
-readability-math-missing-parentheses,
-readability-function-cognitive-complexity,
-readability-uppercase-literal-suffix,
-readability-magic-numbers

# Restrict analysis to this package's own cpp/ tree. Anchored on the full package
# path so it can never match an unrelated directory that merely contains "cpp"
# (e.g. third-party/.../abseil-cpp/, or another package's cpp/). Third-party headers
# are also -isystem in compile_flags.txt, so clang-tidy does not analyze them anyway.
HeaderFilterRegex: 'packages/react-native-executorch/cpp/.*\.(h|hpp)$'

# Treat findings as warnings (CI decides failure via --warnings-as-errors).
WarningsAsErrors: ''
2 changes: 1 addition & 1 deletion packages/react-native-executorch/cpp/RnExecutorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "extensions/math/install.h"
#include "extensions/nlp/install.h"

using namespace facebook;
namespace jsi = facebook::jsi;

namespace rnexecutorch {
void install(jsi::Runtime &jsiRuntime) {
Expand Down
2 changes: 2 additions & 0 deletions packages/react-native-executorch/cpp/core/dtype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ size_t elementSize(DType dtype) {
switch (dtype) {
case DType::uint8:
return 1;
// int32 and float32 are both 4 bytes; the identical branches are intentional.
// NOLINTNEXTLINE(bugprone-branch-clone)
case DType::int32:
return 4;
case DType::float32:
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-executorch/cpp/core/dtype.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include <string>

namespace rnexecutorch::core::types {
enum DType {
enum class DType {
uint8,
int32,
float32
Expand Down
36 changes: 18 additions & 18 deletions packages/react-native-executorch/cpp/core/model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {

auto methodNames = self->etModule_->method_names();
if (!methodNames.ok()) {
std::string errorMsg = executorch::runtime::to_string(methodNames.error());
const std::string errorMsg = executorch::runtime::to_string(methodNames.error());
throw jsi::JSError(rt, "getMethodNames: Failed to get method names: " + errorMsg);
}

Expand Down Expand Up @@ -82,15 +82,15 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
auto methodName = args[0].asString(rt).utf8(rt);
auto methodMeta = self->etModule_->method_meta(methodName);
if (!methodMeta.ok()) {
std::string errorMsg = executorch::runtime::to_string(methodMeta.error());
const std::string errorMsg = executorch::runtime::to_string(methodMeta.error());
throw jsi::JSError(rt, "getMethodMeta: Failed to get method meta: " + errorMsg);
}

auto inputTagsArray = jsi::Array(rt, methodMeta->num_inputs());
for (size_t i = 0; i < methodMeta->num_inputs(); ++i) {
auto tag = methodMeta->input_tag(i);
if (!tag.ok()) {
std::string errorMsg = executorch::runtime::to_string(tag.error());
const std::string errorMsg = executorch::runtime::to_string(tag.error());
throw jsi::JSError(rt, "getMethodMeta: Failed to get input tag for input " + std::to_string(i) + ": " + errorMsg);
}
inputTagsArray.setValueAtIndex(rt, i, jsi::String::createFromUtf8(rt, executorch::runtime::tag_to_string(tag.get())));
Expand All @@ -100,7 +100,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
for (size_t i = 0; i < methodMeta->num_outputs(); ++i) {
auto tag = methodMeta->output_tag(i);
if (!tag.ok()) {
std::string errorMsg = executorch::runtime::to_string(tag.error());
const std::string errorMsg = executorch::runtime::to_string(tag.error());
throw jsi::JSError(rt, "getMethodMeta: Failed to get output tag for output " + std::to_string(i) + ": " + errorMsg);
}
outputTagsArray.setValueAtIndex(rt, i, jsi::String::createFromUtf8(rt, executorch::runtime::tag_to_string(tag.get())));
Expand All @@ -110,7 +110,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
for (size_t i = 0; i < methodMeta->num_backends(); ++i) {
auto backendName = methodMeta->get_backend_name(i);
if (!backendName.ok()) {
std::string errorMsg = executorch::runtime::to_string(backendName.error());
const std::string errorMsg = executorch::runtime::to_string(backendName.error());
throw jsi::JSError(rt, "getMethodMeta: Failed to get backend name for backend " + std::to_string(i) + ": " + errorMsg);
}
usesBackendMap.setProperty(rt, backendName.get(), methodMeta->uses_backend(backendName.get()));
Expand All @@ -123,7 +123,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
jsTensorMeta.setProperty(rt, "nbytes", static_cast<double>(tensorMeta.nbytes()));

try {
std::string dtypeStr = rnexecutorch::core::types::toString(rnexecutorch::core::types::fromScalarType(tensorMeta.scalar_type()));
const std::string dtypeStr = rnexecutorch::core::types::toString(rnexecutorch::core::types::fromScalarType(tensorMeta.scalar_type()));
jsTensorMeta.setProperty(rt, "dtype", jsi::String::createFromUtf8(rt, dtypeStr));
} catch (const std::exception &) {
jsTensorMeta.setProperty(rt, "dtype", jsi::String::createFromUtf8(rt, "not supported"));
Expand All @@ -142,7 +142,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
for (size_t i = 0; i < methodMeta->num_inputs(); ++i) {
auto tensorMeta = methodMeta->input_tensor_meta(i);
if (!tensorMeta.ok()) {
std::string errorMsg = executorch::runtime::to_string(tensorMeta.error());
const std::string errorMsg = executorch::runtime::to_string(tensorMeta.error());
throw jsi::JSError(rt, "getMethodMeta: Failed to get tensor meta for input " + std::to_string(i) + ": " + errorMsg);
}
inputTensorMetaArray.setValueAtIndex(rt, i, tensorMetaToJS(rt, tensorMeta.get()));
Expand All @@ -152,7 +152,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
for (size_t i = 0; i < methodMeta->num_outputs(); ++i) {
auto tensorMeta = methodMeta->output_tensor_meta(i);
if (!tensorMeta.ok()) {
std::string errorMsg = executorch::runtime::to_string(tensorMeta.error());
const std::string errorMsg = executorch::runtime::to_string(tensorMeta.error());
throw jsi::JSError(rt, "getMethodMeta: Failed to get tensor meta for output " + std::to_string(i) + ": " + errorMsg);
}
outputTensorMetaArray.setValueAtIndex(rt, i, tensorMetaToJS(rt, tensorMeta.get()));
Expand Down Expand Up @@ -207,14 +207,14 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
auto inputsArray = args[1].asObject(rt).asArray(rt);

if (!methodMeta.ok()) {
std::string errorMsg = executorch::runtime::to_string(methodMeta.error());
const std::string errorMsg = executorch::runtime::to_string(methodMeta.error());
throw jsi::JSError(rt, "execute: Failed to get method meta for '" + methodName + "': " + errorMsg);
}

if (inputsArray.size(rt) != methodMeta->num_inputs()) {
std::string errorMsg = "execute: Incorrect size for inputs: got " +
std::to_string(inputsArray.size(rt)) +
", expected " + std::to_string(methodMeta->num_inputs());
const std::string errorMsg = "execute: Incorrect size for inputs: got " +
std::to_string(inputsArray.size(rt)) +
", expected " + std::to_string(methodMeta->num_inputs());
throw jsi::JSError(rt, errorMsg);
}

Expand Down Expand Up @@ -252,7 +252,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
auto val = inputsArray.getValueAtIndex(rt, i);

if (!tag.ok()) {
std::string errorMsg = executorch::runtime::to_string(tag.error());
const std::string errorMsg = executorch::runtime::to_string(tag.error());
throw jsi::JSError(rt, "execute: Failed to get input tag for inputs[" +
std::to_string(i) + "]: " + errorMsg);
}
Expand Down Expand Up @@ -290,7 +290,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
auto tensorMeta = methodMeta->input_tensor_meta(i);

if (!tensorMeta.ok()) {
std::string errorMsg = executorch::runtime::to_string(tensorMeta.error());
const std::string errorMsg = executorch::runtime::to_string(tensorMeta.error());
throw jsi::JSError(rt, "execute: Failed to get tensor meta for inputs[" +
std::to_string(i) + "]: " + errorMsg);
}
Expand Down Expand Up @@ -343,7 +343,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
#endif

if (!result.ok()) {
std::string errorMsg = executorch::runtime::to_string(result.error());
const std::string errorMsg = executorch::runtime::to_string(result.error());
throw jsi::JSError(rt, "execute: Method '" + methodName + "' execution failed: " + errorMsg +
". This may be due to missing required backends - use getMethodMeta()" +
" to check required backends and getExecuTorchRegisteredBackends()" +
Expand Down Expand Up @@ -392,7 +392,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
auto tensorMeta = methodMeta->output_tensor_meta(index);

if (!tensorMeta.ok()) {
std::string errorMsg = executorch::runtime::to_string(tensorMeta.error());
const std::string errorMsg = executorch::runtime::to_string(tensorMeta.error());
throw jsi::JSError(rt, "execute: Failed to get tensor meta for output at index " +
std::to_string(index) + ": " + errorMsg);
}
Expand Down Expand Up @@ -467,7 +467,7 @@ std::vector<facebook::jsi::PropNameID> ModelHostObject::getPropertyNames(jsi::Ru
}

void install_loadModel(jsi::Runtime &rt, jsi::Object &module) {
auto name = "loadModel";
const auto *name = "loadModel";
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
if (count != 1) {
throw jsi::JSError(rt, "loadModel: Usage: loadModel(arg0)");
Expand All @@ -482,7 +482,7 @@ void install_loadModel(jsi::Runtime &rt, jsi::Object &module) {

auto error = modelInstance->etModule_->load();
if (!modelInstance->etModule_->is_loaded()) {
std::string errorMsg = executorch::runtime::to_string(error);
const std::string errorMsg = executorch::runtime::to_string(error);
throw jsi::JSError(rt, "loadModel: Failed to load model: " + errorMsg);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-executorch/cpp/core/model.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class ModelHostObject : public facebook::jsi::HostObject, public std::enable_sha
std::unique_ptr<executorch::extension::Module> etModule_;
std::mutex mutex_;

ModelHostObject(const std::string &modelPath);
explicit ModelHostObject(const std::string &modelPath);

facebook::jsi::Value get(facebook::jsi::Runtime &rt, const facebook::jsi::PropNameID &name) override;
std::vector<facebook::jsi::PropNameID> getPropertyNames(facebook::jsi::Runtime &rt) override;
Expand Down
Loading