Skip to content

[RNE Rewrite] chore: set up clangd for C++ sources#1285

Merged
msluszniak merged 4 commits into
rne-rewritefrom
@ms/clangd-setup
Jun 26, 2026
Merged

[RNE Rewrite] chore: set up clangd for C++ sources#1285
msluszniak merged 4 commits into
rne-rewritefrom
@ms/clangd-setup

Conversation

@msluszniak

@msluszniak msluszniak commented Jun 25, 2026

Copy link
Copy Markdown
Member

Description

Sets up a committed clangd 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
  • 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

  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:
    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

  • 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

Add a committed clangd config for the core package's C/C++ sources so the
rewrite gets code intelligence and a shared, strict warning set:

- compile_flags.txt: include roots / std / defines (third-party as -isystem
  so library 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 (incl. -Wconversion/-Wsign-conversion) layered
  on top, with clangd's include cleaner disabled (ET umbrella headers)
- .gitignore: track the shared config while keeping generated DBs local
- CONTRIBUTING.md: document prerequisites and editor setup

clang-tidy CI is a follow-up.
@msluszniak msluszniak self-assigned this Jun 25, 2026
@msluszniak msluszniak added refactoring improvement PRs or issues focused on improvements in the current codebase labels Jun 25, 2026
Add a lefthook pre-commit command that compiles staged cpp/ sources with the
project's clangd warning set (compile_flags.txt + the -W flags in .clangd) and
aborts the commit on any warning, keeping the editor and the commit gate in sync.

It skips gracefully when no compiler or the provisioned ExecuTorch/JSI headers
are available, so contributors who don't build native code are never blocked;
bypass with `git commit --no-verify`.
Bring the existing cpp/ sources into compliance with the strict warning
set so clangd (and the pre-commit guard) start clean:

- comment out unused JSI host-function parameters (thisVal/args) using the
  /* name */ anonymous-parameter idiom
- fix int -> size_t sign-conversions: route validated axis indices through a
  size_t axisIdx, make byte-offset sizes (hw, plane) size_t, and cast the few
  remaining count/index sites
- reformat pre-existing clang-format drift in tokenizer.cpp
@msluszniak msluszniak requested a review from barhanc June 26, 2026 09:33
Comment thread packages/react-native-executorch/scripts/check-cpp-warnings.sh Outdated
Comment thread packages/react-native-executorch/scripts/check-cpp-warnings.sh
Address review feedback:
- skip with a notice if compile_flags.txt or .clangd are missing
- read the last line of compile_flags.txt even without a trailing newline
@msluszniak msluszniak merged commit a90df67 into rne-rewrite Jun 26, 2026
2 checks passed
@msluszniak msluszniak deleted the @ms/clangd-setup branch June 26, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement PRs or issues focused on improvements in the current codebase refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants