Skip to content

Commit

Permalink
test: migrate clang-tidy to LLVM 18 (#5497)
Browse files Browse the repository at this point in the history
* build: install newer dependencies

* build: only build as plugin not executable

since i don't think anyone's using executable and plugin is sufficient

* build: vendor check_clang_tidy.py

* build: fix headers, errors, warnings

* chore: script to get affected files

* ci: gather affected files

* test: run clang-tidy with plugins

* ci: show most time-consuming job

see: CleverRaven/Cataclysm-DDA#64648

co-authored-by: Binrui Dong <[email protected]>

* test: exclude problemetic checks

bugprone-unchecked-optional-access: llvm/llvm-project#111003
bugprone-chained-comparison: conflicts with catch2
clang-diagnostic-unused-macros: llvm/llvm-project#59572
clang-analyzer-optin: segfaults

* refactor: remove unneeded CATA_CLANG_TIDY logic

building clang-tidy is handled separatedly on build-clang-tidy-plugin.sh

* fix: run a lint as an example

* docs: update how to build clang-tidy plugin

---------

Co-authored-by: nocontribute <>
Co-authored-by: Binrui Dong <[email protected]>
  • Loading branch information
scarf005 and BrettDong authored Oct 5, 2024
1 parent afd8da4 commit 6e37f25
Show file tree
Hide file tree
Showing 68 changed files with 658 additions and 492 deletions.
16 changes: 16 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,22 @@ Checks: >
-readability-redundant-access-specifiers,
-readability-use-anyofallof,
-bugprone-throw-keyword-missing,
-readability-identifier-naming,
-readability-avoid-nested-conditional-operator,
-bugprone-unchecked-optional-access,
-bugprone-chained-comparison,
-bugprone-easily-swappable-parameters,
-bugprone-switch-missing-default-case,
-misc-const-correctness,
-misc-include-cleaner,
-misc-use-anonymous-namespace,
-clang-diagnostic-unused-macros,
-clang-analyzer-optin.*,
# https://github.com/llvm/llvm-project/issues/59572
# https://github.com/llvm/llvm-project/issues/111003

WarningsAsErrors: '*'
HeaderFilterRegex: "(src|(test(?!.*catch.*catch.h))|tools).*"
FormatStyle: none
Expand Down
84 changes: 65 additions & 19 deletions .github/workflows/clang-tidy.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
name: Clang-tidy (clang-12, tiles)
name: Clang-tidy (tiles)

on:
push:
branches: [main]
paths: [ "**.cpp", "**.h", "**.c", "**/CMakeLists.txt", "**/Makefile", "**.hpp", "**.cmake", "build-scripts/**","tools/clang-tidy-plugin/**", ".github/workflows/clang-tidy.yml", "**/.clang-tidy" ]
# push:
# branches: [main]
# paths: [ "**.cpp", "**.h", "**.c", "**/CMakeLists.txt", "**/Makefile", "**.hpp", "**.cmake", "build-scripts/**","tools/clang-tidy-plugin/**", ".github/workflows/clang-tidy.yml", "**/.clang-tidy" ]
pull_request:
branches: [main]
paths: [ "**.cpp", "**.h", "**.c", "**/CMakeLists.txt", "**/Makefile", "**.hpp", "**.cmake", "build-scripts/**", "tools/clang-tidy-plugin/**", ".github/workflows/clang-tidy.yml", "**/.clang-tidy" ]
Expand All @@ -16,7 +16,7 @@ concurrency:
jobs:
skip-duplicates:
continue-on-error: true
runs-on: ubuntu-22.04
runs-on: ubuntu-24.04
# Map a step output to a job output
outputs:
should_skip: ${{ steps.skip_check.outputs.should_skip }}
Expand All @@ -30,28 +30,48 @@ jobs:
needs: skip-duplicates
if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' }}

runs-on: ubuntu-22.04
runs-on: ubuntu-24.04
env:
CMAKE: 1
CLANG: clang++-12
COMPILER: clang++-12
CATA_CLANG_TIDY: plugin
CLANG: /usr/bin/clang++-18
COMPILER: /usr/bin/clang++-18
TILES: 1
SOUND: 1
BUILD_PATH: "build"
AFFECTED_FILES: "affected_files.txt"

steps:
- name: checkout repository
uses: actions/checkout@v4
with:
fetch-depth: 1

- name: install dependencies
run: |
wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
sudo apt-add-repository "deb http://apt.llvm.org/focal/ llvm-toolchain-focal-12 main"
sudo apt-get update
sudo apt-get install libncursesw5-dev clang-12 libclang-12-dev llvm-12-dev llvm-12-tools \
libsdl2-dev libsdl2-ttf-dev libsdl2-image-dev libsdl2-mixer-dev libpulse-dev ccache \
libflac-dev gettext
sudo apt-get install \
cmake gettext ninja-build mold ccache jq \
clang-18 libclang-18-dev llvm-18 llvm-18-dev clang-tidy-18 \
libsdl2-dev libsdl2-ttf-dev libsdl2-image-dev libsdl2-mixer-dev libpulse-dev libflac-dev
- name: ensure clang 18 is installed
run: |
if [ -z "$(command -v clang++-18)" ]; then
echo "clang 18 not found"
exit 1
fi
ls -al /usr/lib/llvm-18/lib
if [ ! -d /usr/lib/llvm-18/lib ]; then
echo "llvm-18 not found"
exit 1
fi
sudo update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-18 100
sudo update-alternatives --install /usr/bin/FileCheck FileCheck /usr/bin/FileCheck-18 100
- name: Setup ccache
uses: Chocobo1/setup-ccache-action@v1
with:
install_ccache: false
update_packager_index: false

- name: add problem matcher
run: |
Expand All @@ -60,8 +80,34 @@ jobs:
echo "::add-matcher::build-scripts/problem-matchers/catch2.json"
echo "::add-matcher::build-scripts/problem-matchers/debugmsg.json"
- name: prepare
run: bash ./build-scripts/requirements.sh
- uses: ammaraskar/gcc-problem-matcher@master

- uses: denoland/setup-deno@v1
with:
deno-version: v1.x

- name: build clang-tidy plugin
run: |
pip install --break-system-packages lit
bash ./build-scripts/build-clang-tidy-plugin.sh
# - name: test clang-tidy plugin
# run: |
# lit -v $BUILD_PATH/tools/clang-tidy-plugin/test
# clang-tidy --version

- name: gather affected files
run: deno task affected-files ${{ github.event.pull_request.number }} --output "$AFFECTED_FILES"

- name: run clang-tidy
run: bash ./build-scripts/build.sh
run: bash ./build-scripts/run-clang-tidy-plugin.sh

- name: show most time consuming checks
if: always()
run: | # the folder may not exist if there is no file to analyze
if [ -d clang-tidy-trace ]
then
jq -n 'reduce(inputs.profile | to_entries[]) as {$key,$value} ({}; .[$key] += $value) | with_entries(select(.key|contains(".wall"))) | to_entries | sort_by(.value) | reverse | .[0:10] | from_entries' clang-tidy-trace/*.json
else
echo "clang-tidy-trace folder not found."
fi
7 changes: 7 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,10 @@ Xcode/
VERSION.txt

changelog.md

tidy-analyze-target

test/Output
test/.lit_test_times.txt
affected_files.txt
clang-tidy-trace
31 changes: 31 additions & 0 deletions build-scripts/build-clang-tidy-plugin.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/usr/bin/env bash

set -exo pipefail

BUILD_TYPE=${BUILD_TYPE:-"RelWithDeb"}

NUM_JOBS=${NUM_JOBS:-$(nproc)}
BUILD_PATH=${BUILD_PATH:-"build"}

echo "Using bash version $BASH_VERSION with $NUM_JOBS jobs"
echo "Creating build files at $BUILD_PATH"

# We might need binaries installed via pip, so ensure that our personal bin dir is on the PATH
export PATH=$HOME/.local/bin:$PATH

cmake \
-B "$BUILD_PATH" \
-G Ninja \
-DBACKTRACE=ON \
-DCMAKE_C_COMPILER=/usr/bin/clang \
-DCMAKE_CXX_COMPILER=/usr/bin/clang++ \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DCMAKE_BUILD_TYPE="$BUILD_TYPE" \
-DTILES="$TILES" \
-DSOUND="$SOUND" \
-DLIBBACKTRACE="${LIBBACKTRACE:-0}" \
-DLINKER=mold \
-DCATA_CLANG_TIDY_PLUGIN=ON

ninja -C "$BUILD_PATH" -j"$NUM_JOBS" CataAnalyzerPlugin
ln -s "$BUILD_PATH/compile_commands.json" compile_commands.json
85 changes: 7 additions & 78 deletions build-scripts/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,6 @@ then
build_type=Debug
fi

cmake_extra_opts=()

if [ "$CATA_CLANG_TIDY" = "plugin" ]
then
cmake_extra_opts+=("-DCATA_CLANG_TIDY_PLUGIN=ON")
# Need to specify the particular LLVM / Clang versions to use, lest it
# use the llvm that comes by default on the Github Actions image.
cmake_extra_opts+=("-DLLVM_DIR=/usr/lib/llvm-12/lib/cmake/llvm")
cmake_extra_opts+=("-DClang_DIR=/usr/lib/llvm-12/lib/cmake/clang")
fi

if echo "$COMPILER" | grep -q "clang"
then
if [ -n "$GITHUB_WORKFLOW" -a -n "$CATA_CLANG_TIDY" ]
then
# This is a hacky workaround for the fact that the custom clang-tidy we are
# using was built for now-defunct Travis CI, so it's not using the correct
# include directories for GitHub workflows.
cmake_extra_opts+=("-DCMAKE_CXX_FLAGS=-isystem /usr/include/clang/12.0.0/include")
fi
fi

mkdir build
cd build
cmake \
Expand All @@ -78,64 +56,15 @@ then
-DTILES=${TILES:-0} \
-DSOUND=${SOUND:-0} \
-DLIBBACKTRACE=${LIBBACKTRACE:-0} \
"${cmake_extra_opts[@]}" \
..
if [ -n "$CATA_CLANG_TIDY" ]
then
if [ "$CATA_CLANG_TIDY" = "plugin" ]
then
make -j$num_jobs CataAnalyzerPlugin
export PATH=$PWD/tools/clang-tidy-plugin/clang-tidy-plugin-support/bin:$PATH
if ! which FileCheck
then
ls -l tools/clang-tidy-plugin/clang-tidy-plugin-support/bin
ls -l /usr/bin
echo "Missing FileCheck"
exit 1
fi
CATA_CLANG_TIDY=clang-tidy
lit -v tools/clang-tidy-plugin/test
fi

"$CATA_CLANG_TIDY" --version

# Show compiler C++ header search path
${COMPILER:-clang++} -v -x c++ /dev/null -c
# And the same for clang-tidy
"$CATA_CLANG_TIDY" ../src/version.cpp -- -v

cd ..
ln -s build/compile_commands.json

# TODO: first analyze all files that changed in this PR
set +x
all_cpp_files="$( \
grep '"file": "' build/compile_commands.json | \
sed "s+.*$PWD/++;s+\",\?$++")"
set -x

function analyze_files_in_random_order
{
if [ -n "$1" ]
then
echo "$1" | shuf | \
xargs -P "$num_jobs" -n 1 ./build-scripts/clang-tidy-wrapper.sh -quiet
else
echo "No files to analyze"
fi
}

echo "Analyzing all files"
analyze_files_in_random_order "$all_cpp_files"
else
# Regular build
make -j$num_jobs translations_compile
make -j$num_jobs
cd ..
# Run regular tests
[ -f "${bin_path}cata_test" ] && run_tests "${bin_path}cata_test"
[ -f "${bin_path}cata_test-tiles" ] && run_tests "${bin_path}cata_test-tiles"
fi
# Regular build
make -j$num_jobs translations_compile
make -j$num_jobs
cd ..
# Run regular tests
[ -f "${bin_path}cata_test" ] && run_tests "${bin_path}cata_test"
[ -f "${bin_path}cata_test-tiles" ] && run_tests "${bin_path}cata_test-tiles"
else
if [ "$OS" == "macos-12" ]
then
Expand Down
16 changes: 8 additions & 8 deletions build-scripts/clang-tidy-wrapper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

set -eu
set -o pipefail
set -x

plugin=build/tools/clang-tidy-plugin/libCataAnalyzerPlugin.so
BUILD_PATH=${BUILD_PATH:-"build"}

set -x
if [ -f "$plugin" ]
then
LD_PRELOAD=$plugin "$CATA_CLANG_TIDY" "$@"
else
"$CATA_CLANG_TIDY" "$@"
fi
PLUGIN=$BUILD_PATH/tools/clang-tidy-plugin/libCataAnalyzerPlugin.so

clang-tidy \
--load="$PLUGIN" \
--enable-check-profile \
--store-check-profile=clang-tidy-trace "$@"
5 changes: 0 additions & 5 deletions build-scripts/requirements.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ if [[ $(bc <<< "$(lsb_release -rs) > 22.04") -eq 1 ]]; then
PIP_FLAGS="--break-system-packages"
fi

if [ -n "$CATA_CLANG_TIDY" ]; then
pip install --user wheel --upgrade $PIP_FLAGS
pip install --user 'lit==0.11.1' 'click==7.1.2' $PIP_FLAGS
fi

if [ -n "$LANGUAGES" ]; then
pip install --user polib luaparser $PIP_FLAGS
fi
Expand Down
24 changes: 24 additions & 0 deletions build-scripts/run-clang-tidy-plugin.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/usr/bin/env bash

set -exo pipefail

BUILD_TYPE=${BUILD_TYPE:-"RelWithDeb"}
NUM_JOBS=${NUM_JOBS:-$(nproc)}
BUILD_PATH=${BUILD_PATH:-"build"}
COMPILER=${COMPILER:-"/usr/bin/clang++"}

echo "Using bash version $BASH_VERSION with $NUM_JOBS jobs"
echo "Using build files at $BUILD_PATH"

AFFECTED_FILES=${AFFECTED_FILES:-"affected_files.txt"}

if ! grep -q '[^[:space:]]' "$AFFECTED_FILES"
then
echo "No files to analyze."
exit 0
else
echo "Analyzing $(wc -l < "$AFFECTED_FILES") files"
# shellcheck disable=SC2002
# cat "$AFFECTED_FILES" | parallel -j"$NUM_JOBS" --no-notice -a ./build-scripts/clang-tidy-wrapper.sh {}
< "$AFFECTED_FILES" xargs -n1 -P"$NUM_JOBS" ./build-scripts/clang-tidy-wrapper.sh -quiet
fi
3 changes: 2 additions & 1 deletion deno.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"migrate-unit": "deno run -A scripts/migrate_legacy_unit.ts --path data/json; deno run -A scripts/migrate_legacy_unit.ts --path data/mods; make style-all-json-parallel",
"dprint": "deno run -A npm:dprint",
// creates a weekly reddit changelog template
"changelog": "deno run -A scripts/changelog/changelog_reddit.ts"
"changelog": "deno run -A scripts/changelog/changelog_reddit.ts",
"affected-files": "deno run -RWEN --allow-run scripts/affected_files.ts"
},
"test": { "include": ["scripts"] },
"lint": { "include": ["scripts"] },
Expand Down
Loading

0 comments on commit 6e37f25

Please sign in to comment.