Skip to content

Commit c457bd4

Browse files
Add clang-tidy support (#97)
Added clang-tidy checks locally and via CI, resolved all code that made sense to. Ignored the bridge.
1 parent b6d564b commit c457bd4

43 files changed

Lines changed: 475 additions & 204 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.clang-tidy

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
Checks: >
2+
-*,
3+
bugprone-*,
4+
performance-*,
5+
modernize-*,
6+
readability-misleading-indentation,
7+
readability-redundant-smartptr-get,
8+
-bugprone-easily-swappable-parameters,
9+
-modernize-use-trailing-return-type,
10+
-modernize-avoid-c-arrays,
11+
-modernize-use-auto,
12+
-modernize-use-nodiscard,
13+
-modernize-return-braced-init-list,
14+
-performance-enum-size,
15+
-readability-braces-around-statements,
16+
17+
# These warnings have determined to be critical and are as such treated as errors
18+
WarningsAsErrors: >
19+
clang-analyzer-*,
20+
bugprone-use-after-move,
21+
bugprone-dangling-handle,
22+
bugprone-infinite-loop,
23+
bugprone-narrowing-conversions,
24+
bugprone-undefined-memory-manipulation,
25+
bugprone-move-forwarding-reference,
26+
bugprone-incorrect-roundings,
27+
bugprone-sizeof-expression,
28+
bugprone-string-literal-with-embedded-nul,
29+
bugprone-suspicious-memset-usage,
30+
31+
HeaderFilterRegex: '(include/livekit|src|examples)'
32+
33+
FormatStyle: file
34+
35+
CheckOptions:
36+
- key: modernize-use-nullptr.NullMacros
37+
value: 'NULL'

.github/workflows/builds.yml

Lines changed: 80 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ on:
3737

3838
permissions:
3939
contents: read
40+
actions: read
4041
packages: read
4142

4243
env:
@@ -64,23 +65,23 @@ jobs:
6465
include:
6566
- os: ubuntu-latest
6667
name: linux-x64
67-
build_cmd: ./build.sh release-tests && ./build.sh release-examples
68+
build_cmd: ./build.sh release-all
6869
build_dir: build-release
6970
- os: ubuntu-24.04-arm
7071
name: linux-arm64
71-
build_cmd: ./build.sh release-tests && ./build.sh release-examples
72+
build_cmd: ./build.sh release-all
7273
build_dir: build-release
7374
- os: macos-latest
7475
name: macos-arm64
75-
build_cmd: ./build.sh release-tests && ./build.sh release-examples
76+
build_cmd: ./build.sh release-all
7677
build_dir: build-release
7778
- os: macos-latest
7879
name: macos-x64
79-
build_cmd: ./build.sh release-tests && ./build.sh release-examples --macos-arch x86_64
80+
build_cmd: ./build.sh release-all --macos-arch x86_64
8081
build_dir: build-release
8182
- os: windows-latest
8283
name: windows-x64
83-
build_cmd: .\build.cmd release-tests && .\build.cmd release-examples
84+
build_cmd: .\build.cmd release-all
8485
build_dir: build-release
8586

8687
name: Build (${{ matrix.name }})
@@ -545,3 +546,77 @@ jobs:
545546
cmake -S . -B build -DLIVEKIT_LOCAL_SDK_DIR=/opt/livekit-sdk
546547
cmake --build build --parallel
547548
'
549+
550+
clang-tidy:
551+
name: clang-tidy
552+
runs-on: ubuntu-latest
553+
continue-on-error: false
554+
permissions:
555+
contents: read
556+
pull-requests: write
557+
558+
steps:
559+
- name: Checkout (with submodules)
560+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
561+
with:
562+
submodules: recursive
563+
fetch-depth: 1
564+
565+
- name: Install dependencies
566+
run: |
567+
set -eux
568+
sudo apt-get update
569+
sudo apt-get install -y \
570+
build-essential cmake ninja-build pkg-config \
571+
llvm-dev libclang-dev clang clang-tidy \
572+
libssl-dev
573+
574+
- name: Install Rust (stable)
575+
uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9
576+
with:
577+
toolchain: stable
578+
579+
- name: Set Linux build environment
580+
run: |
581+
echo "CXXFLAGS=-Wno-deprecated-declarations" >> $GITHUB_ENV
582+
echo "CFLAGS=-Wno-deprecated-declarations" >> $GITHUB_ENV
583+
LLVM_VERSION=$(llvm-config --version | cut -d. -f1)
584+
echo "LIBCLANG_PATH=/usr/lib/llvm-${LLVM_VERSION}/lib" >> $GITHUB_ENV
585+
586+
- name: CMake configure
587+
run: cmake --preset linux-release
588+
589+
- name: Generate protobuf headers
590+
run: cmake --build build-release --target livekit_proto
591+
592+
- name: Run clang-tidy
593+
uses: cpp-linter/cpp-linter-action@77c390c5ba9c947ebc185a3e49cc754f1558abb5 # v2.18.0
594+
id: linter
595+
env:
596+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
597+
with:
598+
style: ''
599+
tidy-checks: ''
600+
database: build-release
601+
files-changed-only: false
602+
lines-changed-only: true
603+
extensions: 'c,cpp,cc,cxx'
604+
ignore: 'build-*|cpp-example-collection|client-sdk-rust|vcpkg_installed|src/tests|bridge|src/room_event_converter.cpp'
605+
file-annotations: true
606+
thread-comments: update
607+
step-summary: true
608+
tidy-review: false
609+
no-lgtm: true
610+
jobs: 0 # 0 == use all available CPU cores
611+
612+
- name: Check warning thresholds
613+
env:
614+
TIDY_FINDINGS: ${{ steps.linter.outputs.clang-tidy-checks-failed }}
615+
MAX_TIDY_FINDINGS: '0'
616+
run: |
617+
echo "clang-tidy findings: ${TIDY_FINDINGS}"
618+
if [ "${TIDY_FINDINGS}" -gt "${MAX_TIDY_FINDINGS}" ]; then
619+
echo "::warning::clang-tidy found ${TIDY_FINDINGS} issue(s), threshold is ${MAX_TIDY_FINDINGS}"
620+
exit 1
621+
fi
622+
echo "clang-tidy findings within threshold"

AGENTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ All source files must have the LiveKit Apache 2.0 copyright header. Use the curr
157157

158158
### Readability and Performance
159159
Code should be easy to read and understand. If a sacrifice is made for performance or readability, it should be documented.
160+
161+
### Static Analysis
162+
Adhere to clang-tidy checks configured in `.clang-tidy`.
163+
160164
## Dependencies
161165

162166
| Dependency | Scope | Notes |

CMakePresets.json

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,32 @@
207207
"LIVEKIT_BUILD_TESTS": "OFF"
208208
}
209209
},
210+
{
211+
"name": "windows-release-all",
212+
"displayName": "Windows x64 Release with Tests + Examples",
213+
"description": "Build for Windows x64 Release with tests and examples",
214+
"inherits": "windows-base",
215+
"binaryDir": "${sourceDir}/build-release",
216+
"cacheVariables": {
217+
"CMAKE_BUILD_TYPE": "Release",
218+
"LIVEKIT_BUILD_EXAMPLES": "ON",
219+
"LIVEKIT_BUILD_TESTS": "ON",
220+
"VCPKG_MANIFEST_FEATURES": "examples"
221+
}
222+
},
223+
{
224+
"name": "windows-debug-all",
225+
"displayName": "Windows x64 Debug with Tests + Examples",
226+
"description": "Build for Windows x64 Debug with tests and examples",
227+
"inherits": "windows-base",
228+
"binaryDir": "${sourceDir}/build-debug",
229+
"cacheVariables": {
230+
"CMAKE_BUILD_TYPE": "Debug",
231+
"LIVEKIT_BUILD_EXAMPLES": "ON",
232+
"LIVEKIT_BUILD_TESTS": "ON",
233+
"VCPKG_MANIFEST_FEATURES": "examples"
234+
}
235+
},
210236
{
211237
"name": "windows-release-tests",
212238
"displayName": "Windows x64 Release with Tests",
@@ -231,6 +257,30 @@
231257
"LIVEKIT_BUILD_TESTS": "ON"
232258
}
233259
},
260+
{
261+
"name": "linux-release-all",
262+
"displayName": "Linux Release with Tests + Examples",
263+
"description": "Build for Linux Release with tests and examples",
264+
"inherits": "linux-base",
265+
"binaryDir": "${sourceDir}/build-release",
266+
"cacheVariables": {
267+
"CMAKE_BUILD_TYPE": "Release",
268+
"LIVEKIT_BUILD_EXAMPLES": "ON",
269+
"LIVEKIT_BUILD_TESTS": "ON"
270+
}
271+
},
272+
{
273+
"name": "linux-debug-all",
274+
"displayName": "Linux Debug with Tests + Examples",
275+
"description": "Build for Linux Debug with tests and examples",
276+
"inherits": "linux-base",
277+
"binaryDir": "${sourceDir}/build-debug",
278+
"cacheVariables": {
279+
"CMAKE_BUILD_TYPE": "Debug",
280+
"LIVEKIT_BUILD_EXAMPLES": "ON",
281+
"LIVEKIT_BUILD_TESTS": "ON"
282+
}
283+
},
234284
{
235285
"name": "linux-release-tests",
236286
"displayName": "Linux Release with Tests",
@@ -255,6 +305,30 @@
255305
"LIVEKIT_BUILD_TESTS": "ON"
256306
}
257307
},
308+
{
309+
"name": "macos-release-all",
310+
"displayName": "macOS Release with Tests + Examples",
311+
"description": "Build for macOS Release with tests and examples",
312+
"inherits": "macos-base",
313+
"binaryDir": "${sourceDir}/build-release",
314+
"cacheVariables": {
315+
"CMAKE_BUILD_TYPE": "Release",
316+
"LIVEKIT_BUILD_EXAMPLES": "ON",
317+
"LIVEKIT_BUILD_TESTS": "ON"
318+
}
319+
},
320+
{
321+
"name": "macos-debug-all",
322+
"displayName": "macOS Debug with Tests + Examples",
323+
"description": "Build for macOS Debug with tests and examples",
324+
"inherits": "macos-base",
325+
"binaryDir": "${sourceDir}/build-debug",
326+
"cacheVariables": {
327+
"CMAKE_BUILD_TYPE": "Debug",
328+
"LIVEKIT_BUILD_EXAMPLES": "ON",
329+
"LIVEKIT_BUILD_TESTS": "ON"
330+
}
331+
},
258332
{
259333
"name": "macos-release-tests",
260334
"displayName": "macOS Release with Tests",
@@ -333,6 +407,32 @@
333407
"name": "macos-debug-examples",
334408
"configurePreset": "macos-debug-examples"
335409
},
410+
{
411+
"name": "windows-release-all",
412+
"configurePreset": "windows-release-all",
413+
"configuration": "Release"
414+
},
415+
{
416+
"name": "windows-debug-all",
417+
"configurePreset": "windows-debug-all",
418+
"configuration": "Debug"
419+
},
420+
{
421+
"name": "linux-release-all",
422+
"configurePreset": "linux-release-all"
423+
},
424+
{
425+
"name": "linux-debug-all",
426+
"configurePreset": "linux-debug-all"
427+
},
428+
{
429+
"name": "macos-release-all",
430+
"configurePreset": "macos-release-all"
431+
},
432+
{
433+
"name": "macos-debug-all",
434+
"configurePreset": "macos-debug-all"
435+
},
336436
{
337437
"name": "windows-release-tests",
338438
"configurePreset": "windows-release-tests",

README.md

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ git lfs pull
6767
./build.sh debug-examples # Build Debug with examples
6868
./build.sh release-examples # Build Release with examples
6969
./build.sh debug-tests # Build Debug with tests
70+
./build.sh debug-all # Build Debug with tests + examples
7071
./build.sh release-tests # Build Release with tests
72+
./build.sh release-all # Build Release with tests + examples
7173
```
7274
**Windows**
7375
Using build scripts:
@@ -79,7 +81,9 @@ Using build scripts:
7981
.\build.cmd debug-examples # Build Debug with examples
8082
.\build.cmd release-examples # Build Release with examples
8183
.\build.cmd debug-tests # Build Debug with tests
84+
.\build.cmd debug-all # Build Debug with tests + examples
8285
.\build.cmd release-tests # Build Release with tests
86+
.\build.cmd release-all # Build Release with tests + examples
8387
```
8488

8589
### Windows build using cmake/vcpkg
@@ -488,15 +492,47 @@ In some cases, you may need to perform a full clean that deletes all build artif
488492
./build.sh clean-all
489493
```
490494

491-
### Clang format
492-
CPP SDK is using clang C++ format
495+
## Quality Checks
496+
497+
This SDK leverages various tools and checks to ensure the highest quality of the code.
498+
499+
### Clang Tools
500+
501+
- `clang-tidy`: static analysis checks to catch common C++ pitfalls. See [.clang-tidy](./.clang-tidy) for the list of current checks (enforced in CI on PR)
502+
- `clang-format`: (coming soon) code formatting and style consistency
503+
504+
> **Note**: clang-tidy is not currently supported on Windows for this project because the Visual Studio CMake generator does not produce the compile_commands.json database that clang-tidy requires.
505+
506+
To run locally, first install the following:
507+
508+
**macOS:**
509+
510+
```bash
511+
brew install llvm
512+
```
513+
514+
This installs `clang-format`, `clang-tidy`, and `run-clang-tidy`. Homebrew may ask you to add `/opt/homebrew/opt/llvm/bin` to your `PATH`.
515+
516+
**Linux:**
517+
493518
```bash
494-
brew install clang-format
519+
# Ubuntu / Debian:
520+
sudo apt-get install clang-format clang-tidy clang-tools
495521
```
496522

523+
To run:
524+
525+
1. Run a build script command, such that `compile_command.json` is present at the root of the repository
526+
2. Run:
527+
528+
```bash
529+
clang-tidy -p . src/*.cpp
530+
```
531+
532+
### Memory Checks
497533

498-
#### Memory Checks
499534
Run valgrind on various examples or tests to check for memory leaks and other issues.
535+
500536
```bash
501537
valgrind --leak-check=full ./build-debug/bin/livekit_integration_tests
502538
valgrind --leak-check=full ./build-debug/bin/livekit_stress_tests

0 commit comments

Comments
 (0)