Skip to content

Commit a90df67

Browse files
authored
[RNE Rewrite] chore: set up clangd for C++ sources (#1285)
## Description Sets up a committed [clangd](https://clangd.llvm.org/) config for the core package's C/C++ sources so the rewrite gets code intelligence and a shared, strict warning set, and enforces that set at commit time. First step of #1271; clang-tidy CI is a follow-up (#1286). - `compile_flags.txt` — include roots / `-std=c++20` / defines, with third-party headers (ExecuTorch/torch/JSI) as `-isystem` so their warnings don't pollute our diagnostics. Relative paths anchored to the package dir, so it is portable with no per-machine generation. - `.clangd` — strict warning set layered on top (incl. `-Wconversion`/`-Wsign-conversion`), with clangd's include cleaner disabled (ExecuTorch umbrella headers misreport includes). - `scripts/check-cpp-warnings.sh` + `lefthook.yml` — a `pre-commit` command that compiles staged `cpp/` sources with the same warning set and aborts the commit on any warning, keeping the editor and the commit gate in sync. It skips gracefully (never blocks) when no compiler or the provisioned headers are available; bypass with `git commit --no-verify`. - `.gitignore` — track the shared config while keeping generated `compile_commands.json` local. - `CONTRIBUTING.md` — prerequisites (provisioned `third-party/include` + `yarn install`), editor setup, and the pre-commit behavior. ### Introduces a breaking change? - [ ] Yes - [x] 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) - [x] Other (chores, tests, code style improvements etc.) ### Tested on - [ ] iOS - [ ] Android ### Testing instructions 1. Provision `packages/react-native-executorch/third-party/include` and run `yarn install`. 2. Open a file under `packages/react-native-executorch/cpp` in an editor with the clangd extension (MS C/C++ IntelliSense disabled); confirm the `<executorch/...>`/`<jsi/jsi.h>` includes resolve and the current sources are clean. 3. Confirm the strict flags fire by introducing a deliberate violation, e.g. in `cpp/core/dtype.cpp`: ```cpp int rne_probe(float f) { int casted = (int)f; // expect -Wold-style-cast + -Wconversion int unused = 7; // expect -Wunused-variable return casted; } ``` clangd should flag these on our code (third-party headers stay quiet thanks to `-isystem`). 4. `git add` that change and `git commit` — the pre-commit hook aborts with the warning printed. Verified end-to-end via lefthook: warning → `cpp-warnings ❌` (exit 1); clean → `✔️` (exit 0). Revert afterwards. ### Related issues #1271 ### Checklist - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have updated the documentation accordingly - [x] My changes generate no new warnings
1 parent 7ec5627 commit a90df67

13 files changed

Lines changed: 227 additions & 64 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ jsconfig.json
1515
.cache/
1616
compile_commands.json
1717
compile_flags.txt
18+
# ...but the core package ships a shared, committed clangd config for its C++ sources:
19+
!/packages/react-native-executorch/.clangd
20+
!/packages/react-native-executorch/compile_flags.txt
1821

1922
# Xcode
2023
#

CONTRIBUTING.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,40 @@ Found a model you would like to use in your app but it is not currently supplied
4545
Do you have a neat example use case and want to share it with us? You can just drop us a message on Discord server and/or open a PR to `apps` directory here.
4646
If you found some inconsistencies in our documentation or just something is missing just open a PR with suggested changes (remember to add changes to previous docs versions too, e.g `docs/versioned_docs/version-0.3.x`, `docs/versioned_docs/version-0.4.x`).
4747

48+
# C++ tooling (clangd)
49+
50+
The core package ships a committed [clangd](https://clangd.llvm.org/) setup so the
51+
C/C++ sources under `packages/react-native-executorch/cpp` get code intelligence and
52+
a strict, shared set of compiler warnings:
53+
54+
- `packages/react-native-executorch/compile_flags.txt` — the compilation database:
55+
language standard, preprocessor defines, and include roots (paths are relative to
56+
the package, so the config is portable). ExecuTorch/torch/JSI headers are added as
57+
system includes so their internal warnings don't pollute diagnostics for our code.
58+
- `packages/react-native-executorch/.clangd` — the warning policy layered on top.
59+
60+
For clangd to resolve the `<executorch/...>` and `<jsi/jsi.h>` includes you need the
61+
same prerequisites as a native build:
62+
63+
1. `yarn install` at the repo root (provides the JSI headers under `node_modules`).
64+
2. ExecuTorch headers provisioned under `packages/react-native-executorch/third-party/include`
65+
(see [third-party/README.md](./packages/react-native-executorch/third-party/README.md)).
66+
67+
Without them clangd still lints your own code; the third-party includes simply stay
68+
unresolved until the headers are present.
69+
70+
A `pre-commit` hook (lefthook) compiles staged `cpp/` sources with this same warning set
71+
and aborts the commit if any warning is introduced — the editor and the commit gate stay
72+
in sync. It skips automatically when no compiler or the provisioned headers are available,
73+
so it never blocks contributors who don't build the native code; bypass with
74+
`git commit --no-verify`.
75+
76+
Editor setup: install the official **clangd** extension (e.g. `llvm-vs-code-extensions.vscode-clangd`
77+
for VS Code) and disable the default Microsoft C/C++ IntelliSense engine so the two
78+
don't conflict. clangd discovers `compile_flags.txt`/`.clangd` automatically from the
79+
open file's directory. If you produce a `compile_commands.json` from a native build,
80+
clangd will prefer it — it's gitignored and takes precedence over `compile_flags.txt`.
81+
4882
# Creating a Pull Request
4983

5084
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.

lefthook.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ pre-commit:
1313
format-objc:
1414
glob: '*.{h,hpp,m,mm,c,cpp}'
1515
run: clang-format -i {staged_files} && git add {staged_files}
16+
cpp-warnings:
17+
glob: '*.{h,hpp,c,cpp}'
18+
run: packages/react-native-executorch/scripts/check-cpp-warnings.sh {staged_files}
1619
format-kotlin:
1720
glob: '*.{kt}'
1821
run: ktlint -F {staged_files} && git add {staged_files}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# clangd configuration for the react-native-executorch C/C++ sources.
2+
#
3+
# Include roots, the language standard, and preprocessor defines live in
4+
# `compile_flags.txt` (the compilation database). This file layers the project's
5+
# warning policy on top and configures clangd's diagnostics.
6+
#
7+
# The warning set is intentionally strict to surface bugs early during the
8+
# rewrite. Third-party headers are included as `-isystem` (see compile_flags.txt),
9+
# so these warnings only apply to our own code in `cpp/`.
10+
CompileFlags:
11+
Add:
12+
- -Wall
13+
- -Wextra
14+
- -Wpedantic
15+
- -Wshadow
16+
- -Wnon-virtual-dtor
17+
- -Woverloaded-virtual
18+
- -Wold-style-cast
19+
- -Wcast-align
20+
- -Wunused
21+
- -Wnull-dereference
22+
- -Wimplicit-fallthrough
23+
- -Wextra-semi
24+
- -Wformat=2
25+
- -Wconversion
26+
- -Wsign-conversion
27+
- -Wdouble-promotion
28+
29+
Diagnostics:
30+
# ExecuTorch/torch rely heavily on umbrella headers, which clangd's include
31+
# cleaner misreports as unused/missing. Disable it to avoid false positives.
32+
UnusedIncludes: None
33+
MissingIncludes: None
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
-xc++
2+
-std=c++20
3+
-DC10_USING_CUSTOM_GENERATED_MACROS=1
4+
-DEXECUTORCH_ENABLE_EXECUTION_PROFILING=1
5+
-Icpp
6+
-isystem../../node_modules/react-native/ReactCommon/jsi
7+
-isystemthird-party/include
8+
-isystemthird-party/include/executorch/extension/llm/tokenizers/include
9+
-isystemthird-party/include/executorch/extension/llm/tokenizers/third-party/json/include
10+
-isystemthird-party/include/executorch/extension/llm/tokenizers/third-party/re2
11+
-isystemthird-party/include/executorch/extension/llm/tokenizers/third-party/abseil-cpp

packages/react-native-executorch/cpp/core/model.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
2727

2828
if (nameStr == "getMethodNames") {
2929
auto self = shared_from_this();
30-
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
30+
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value * /*args*/, size_t count) -> jsi::Value {
3131
if (count != 0) {
3232
throw jsi::JSError(rt, "getMethodNames: Usage: getMethodNames()");
3333
}
@@ -61,7 +61,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
6161

6262
if (nameStr == "getMethodMeta") {
6363
auto self = shared_from_this();
64-
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
64+
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
6565
if (count != 1) {
6666
throw jsi::JSError(rt, "getMethodMeta: Usage: getMethodMeta(methodName)");
6767
}
@@ -176,7 +176,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
176176

177177
if (nameStr == "execute") {
178178
auto self = shared_from_this();
179-
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
179+
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
180180
if (count != 3) {
181181
throw jsi::JSError(rt, "execute: Usage: execute(methodName, inputs, outputTensors)");
182182
}
@@ -435,7 +435,7 @@ jsi::Value ModelHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name) {
435435

436436
if (nameStr == "dispose") {
437437
auto self = shared_from_this();
438-
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
438+
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value * /*args*/, size_t count) -> jsi::Value {
439439
if (count != 0) {
440440
throw jsi::JSError(rt, "dispose: Usage: dispose()");
441441
}
@@ -468,7 +468,7 @@ std::vector<facebook::jsi::PropNameID> ModelHostObject::getPropertyNames(jsi::Ru
468468

469469
void install_loadModel(jsi::Runtime &rt, jsi::Object &module) {
470470
auto name = "loadModel";
471-
auto fnBody = [](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
471+
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
472472
if (count != 1) {
473473
throw jsi::JSError(rt, "loadModel: Usage: loadModel(arg0)");
474474
}

packages/react-native-executorch/cpp/core/tensor.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ jsi::Value TensorHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name)
3838

3939
if (nameStr == "copyTo") {
4040
auto self = shared_from_this();
41-
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
41+
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
4242
if (count != 1) {
4343
throw jsi::JSError(rt, "copyTo: Usage: copyTo(dst)");
4444
}
@@ -143,7 +143,7 @@ jsi::Value TensorHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name)
143143

144144
if (nameStr == "getData") {
145145
auto self = shared_from_this();
146-
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
146+
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
147147
if (count != 1) {
148148
throw jsi::JSError(rt, "getData: Usage: getData(array)");
149149
}
@@ -259,7 +259,7 @@ jsi::Value TensorHostObject::get(jsi::Runtime &rt, const jsi::PropNameID &name)
259259

260260
if (nameStr == "dispose") {
261261
auto self = shared_from_this();
262-
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
262+
auto fnBody = [self](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value * /*args*/, size_t count) -> jsi::Value {
263263
if (count != 0) {
264264
throw jsi::JSError(rt, "dispose: Usage: dispose()");
265265
}
@@ -297,7 +297,7 @@ std::vector<facebook::jsi::PropNameID> TensorHostObject::getPropertyNames(jsi::R
297297

298298
void install_createTensor(jsi::Runtime &rt, jsi::Object &module) {
299299
auto name = "createTensor";
300-
auto fnBody = [](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
300+
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
301301
if (count != 2) {
302302
throw jsi::JSError(rt, "createTensor: Usage: createTensor(shape, dtype)");
303303
}

packages/react-native-executorch/cpp/core/utils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace jsi = facebook::jsi;
1010

1111
void install_getExecuTorchRegisteredBackends(jsi::Runtime &rt, jsi::Object &module) {
1212
auto name = "getExecuTorchRegisteredBackends";
13-
auto fnBody = [](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
13+
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value * /*args*/, size_t count) -> jsi::Value {
1414
if (count != 0) {
1515
throw jsi::JSError(rt, "Usage: getExecuTorchRegisteredBackends()");
1616
}

packages/react-native-executorch/cpp/extensions/cv/box_ops.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ std::array<float, 4> decodeToXyxy(
6767

6868
void install_nms(jsi::Runtime &rt, jsi::Object &module) {
6969
auto name = "nms";
70-
auto fnBody = [](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
70+
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
7171
if (count < 3) {
7272
throw jsi::JSError(rt, "Usage: nms(boxes, scores, options)");
7373
}
@@ -143,7 +143,7 @@ void install_nms(jsi::Runtime &rt, jsi::Object &module) {
143143
const float *scoresPtr = reinterpret_cast<const float *>(scores->data_.get());
144144

145145
std::vector<std::pair<std::int32_t, float>> candidates;
146-
candidates.reserve(numAnchors);
146+
candidates.reserve(static_cast<size_t>(numAnchors));
147147

148148
for (size_t idx = 0; std::cmp_less(idx, numAnchors); ++idx) {
149149
float score = scoresPtr[idx];

packages/react-native-executorch/cpp/extensions/cv/image_ops.cpp

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ FitBox computeFit(int32_t srcW, int32_t srcH, int32_t dstW, int32_t dstH, bool i
6565

6666
void install_resize(jsi::Runtime &rt, jsi::Object &module) {
6767
auto name = "resize";
68-
auto fnBody = [](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
68+
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
6969
if (count != 3) {
7070
throw jsi::JSError(rt, "Usage: resize(src, dst, options)");
7171
}
@@ -233,7 +233,7 @@ int codeToColorConversionFlag(const std::string &code) {
233233

234234
void install_cvtColor(jsi::Runtime &rt, jsi::Object &module) {
235235
auto name = "cvtColor";
236-
auto fnBody = [](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
236+
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
237237
if (count != 3) {
238238
throw jsi::JSError(rt, "Usage: cvtColor(src, dst, code)");
239239
}
@@ -319,7 +319,7 @@ void install_cvtColor(jsi::Runtime &rt, jsi::Object &module) {
319319

320320
void install_toChannelsFirst(jsi::Runtime &rt, jsi::Object &module) {
321321
auto name = "toChannelsFirst";
322-
auto fnBody = [](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
322+
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
323323
if (count != 2) {
324324
throw jsi::JSError(rt, "Usage: toChannelsFirst(src, dst)");
325325
}
@@ -391,7 +391,7 @@ void install_toChannelsFirst(jsi::Runtime &rt, jsi::Object &module) {
391391
std::vector<::cv::Mat> channels;
392392
::cv::split(srcMat, channels);
393393

394-
int32_t hw = srcH * srcW;
394+
size_t hw = static_cast<size_t>(srcH) * static_cast<size_t>(srcW);
395395
size_t elemSize = rnexecutorch::core::types::elementSize(src->dtype_);
396396
uint8_t *dstPtr = dst->data_.get();
397397

@@ -407,7 +407,7 @@ void install_toChannelsFirst(jsi::Runtime &rt, jsi::Object &module) {
407407

408408
void install_toChannelsLast(jsi::Runtime &rt, jsi::Object &module) {
409409
auto name = "toChannelsLast";
410-
auto fnBody = [](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
410+
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
411411
if (count != 2) {
412412
throw jsi::JSError(rt, "Usage: toChannelsLast(src, dst)");
413413
}
@@ -475,7 +475,7 @@ void install_toChannelsLast(jsi::Runtime &rt, jsi::Object &module) {
475475
throw jsi::JSError(rt, "toChannelsLast: " + std::string(e.what()));
476476
}
477477

478-
int32_t hw = srcH * srcW;
478+
size_t hw = static_cast<size_t>(srcH) * static_cast<size_t>(srcW);
479479
size_t elemSize = rnexecutorch::core::types::elementSize(src->dtype_);
480480
uint8_t *srcPtr = src->data_.get();
481481

@@ -495,7 +495,7 @@ void install_toChannelsLast(jsi::Runtime &rt, jsi::Object &module) {
495495

496496
void install_normalize(jsi::Runtime &rt, jsi::Object &module) {
497497
auto name = "normalize";
498-
auto fnBody = [](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
498+
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
499499
if (count != 3) {
500500
throw jsi::JSError(rt, "Usage: normalize(src, dst, options)");
501501
}
@@ -541,7 +541,7 @@ void install_normalize(jsi::Runtime &rt, jsi::Object &module) {
541541
}
542542

543543
auto val = opts.getProperty(rt, name);
544-
std::vector<double> result(c);
544+
std::vector<double> result(static_cast<size_t>(c));
545545

546546
if (val.isNumber()) {
547547
std::ranges::fill(result, val.asNumber());
@@ -602,9 +602,10 @@ void install_normalize(jsi::Runtime &rt, jsi::Object &module) {
602602
uint8_t *srcPtr = src->data_.get();
603603
uint8_t *dstPtr = dst->data_.get();
604604

605+
const size_t plane = static_cast<size_t>(h) * static_cast<size_t>(w);
605606
for (size_t ch = 0; std::cmp_less(ch, c); ++ch) {
606-
::cv::Mat srcChannel(h, w, srcDepthType, srcPtr + ch * h * w * srcElemSize);
607-
::cv::Mat dstChannel(h, w, dstDepthType, dstPtr + ch * h * w * dstElemSize);
607+
::cv::Mat srcChannel(h, w, srcDepthType, srcPtr + ch * plane * srcElemSize);
608+
::cv::Mat dstChannel(h, w, dstDepthType, dstPtr + ch * plane * dstElemSize);
608609

609610
srcChannel.convertTo(dstChannel, dstDepthType, alpha[ch], beta[ch]);
610611
}
@@ -617,7 +618,7 @@ void install_normalize(jsi::Runtime &rt, jsi::Object &module) {
617618

618619
void install_applyColormap(jsi::Runtime &rt, jsi::Object &module) {
619620
auto name = "applyColormap";
620-
auto fnBody = [](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) -> jsi::Value {
621+
auto fnBody = [](jsi::Runtime &rt, const jsi::Value & /*thisVal*/, const jsi::Value *args, size_t count) -> jsi::Value {
621622
if (count != 3) {
622623
throw jsi::JSError(rt, "Usage: applyColormap(src, dst, colormap)");
623624
}
@@ -695,7 +696,7 @@ void install_applyColormap(jsi::Runtime &rt, jsi::Object &module) {
695696
std::to_string(numColors) + ")");
696697
}
697698
for (size_t c = 0; c < numRgbaChannels; ++c) {
698-
dstData[i * numRgbaChannels + c] = lut[idx][c];
699+
dstData[i * numRgbaChannels + c] = lut[static_cast<size_t>(idx)][c];
699700
}
700701
}
701702

0 commit comments

Comments
 (0)