Skip to content

Commit

Permalink
Let CI check C++ includes
Browse files Browse the repository at this point in the history
The commit adds a CI workflow that uses the included-what-you-use (IWYU)
tool to check for missing or superfluous includes in .cpp files and
their corresponding .h files. This means that some .h files (especially
in the nnue folder) are not checked yet.

The CI setup looks like this:
- We build IWYU from source to include some yet unreleased fixes.
  This IWYU version targets LLVM 17. Thus, we get the latest release
  candidate of LLVM 17 from LLVM's nightly packages.
- The Makefile now has an analyze target that just build the object
  files (without linking)
- The CI uses the analyze target with the IWYU tool as compiler to
  analyze the compiled .cpp file and its corresponding .h file.
- If IWYU suggests a change the build fails (-Xiwyu --error).
- To avoid false positives we use LLVM's libc++ as standard library
- We have a custom mappings file that adds some mappings that are
  missing in IWYU's default mappings

We also had to add one IWYU pragma to prevent a false positive in
movegen.h.

No functional changea
  • Loading branch information
PikaCat-OuO committed Sep 23, 2023
1 parent 016ad97 commit 3ddb27a
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 10 deletions.
54 changes: 46 additions & 8 deletions .github/workflows/codeql.yml → .github/workflows/analyzer.yml
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
name: "CodeQL"
name: "Analyzer"

on:
push:
branches: [ 'master' ]
pull_request:
# The branches below must be a subset of the branches above
branches: [ 'master' ]
schedule:
- cron: '17 18 * * 1'

jobs:
analyze:
name: Analyze
CodeQL:
name: CodeQL
runs-on: ubuntu-latest
permissions:
actions: read
Expand Down Expand Up @@ -51,3 +46,46 @@ jobs:
uses: github/codeql-action/analyze@v2
with:
category: "/language:${{matrix.language}}"

IYUW:
name: Check includes
runs-on: ubuntu-latest
defaults:
run:
working-directory: Pikafish/src
shell: bash
steps:
- name: Checkout Pikafish
uses: actions/checkout@v3
with:
path: Pikafish

- name: Checkout include-what-you-use
uses: actions/checkout@v3
with:
repository: include-what-you-use/include-what-you-use
ref: f25caa280dc3277c4086ec345ad279a2463fea0f
path: include-what-you-use

- name: Download required linux packages
run: |
sudo add-apt-repository 'deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-17 main'
wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
sudo apt update
sudo apt install -y libclang-17-dev clang-17 libc++-17-dev
- name: Set up include-what-you-use
run: |
mkdir build && cd build
cmake -G "Unix Makefiles" -DCMAKE_PREFIX_PATH="/usr/lib/llvm-17" ..
sudo make install
working-directory: include-what-you-use

- name: Check include-what-you-use
run: include-what-you-use --version

- name: Check includes
run: >
make analyze
COMP=clang
CXX=include-what-you-use
CXXFLAGS="-stdlib=libc++ -Xiwyu --comment_style=long -Xiwyu --mapping='${{ github.workspace }}/Pikafish/.github/workflows/libcxx17.imp' -Xiwyu --error"
21 changes: 21 additions & 0 deletions .github/workflows/libcxx17.imp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[
# Mappings for libcxx's internal headers
{ include: [ "<__fwd/fstream.h>", private, "<iosfwd>", public ] },
{ include: [ "<__fwd/ios.h>", private, "<iosfwd>", public ] },
{ include: [ "<__fwd/istream.h>", private, "<iosfwd>", public ] },
{ include: [ "<__fwd/ostream.h>", private, "<iosfwd>", public ] },
{ include: [ "<__fwd/sstream.h>", private, "<iosfwd>", public ] },
{ include: [ "<__fwd/streambuf.h>", private, "<iosfwd>", public ] },
{ include: [ "<__fwd/string_view.h>", private, "<string_view>", public ] },

# Mappings for includes between public headers
{ include: [ "<ios>", public, "<iostream>", public ] },
{ include: [ "<streambuf>", public, "<iostream>", public ] },
{ include: [ "<istream>", public, "<iostream>", public ] },
{ include: [ "<ostream>", public, "<iostream>", public ] },
{ include: [ "<iosfwd>", public, "<iostream>", public ] },

# Missing mappings in include-what-you-use's libcxx.imp
{ include: ["@<__condition_variable/.*>", private, "<condition_variable>", public ] },
{ include: ["@<__mutex/.*>", private, "<mutex>", public ] },
]
6 changes: 5 additions & 1 deletion src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -838,12 +838,16 @@ ifneq ($(SUPPORTED_ARCH), true)
endif


.PHONY: help build profile-build strip install clean net objclean profileclean \
.PHONY: help analyze build profile-build strip install clean net \
objclean profileclean config-sanity \
config-sanity \
icx-profile-use icx-profile-make \
gcc-profile-use gcc-profile-make \
clang-profile-use clang-profile-make FORCE

analyze: net config-sanity objclean
$(MAKE) -k ARCH=$(ARCH) COMP=$(COMP) $(OBJS)

build: net config-sanity
$(MAKE) ARCH=$(ARCH) COMP=$(COMP) all

Expand Down
2 changes: 1 addition & 1 deletion src/movegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#ifndef MOVEGEN_H_INCLUDED
#define MOVEGEN_H_INCLUDED

#include <algorithm>
#include <algorithm> // IWYU pragma: keep
#include <cstddef>

#include "types.h"
Expand Down

0 comments on commit 3ddb27a

Please sign in to comment.