Skip to content

Clang tidy include cleaner #13280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
+28 −2
Draft

Clang tidy include cleaner #13280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
48ec472
Add clang-tidy to pre-commit
fzakaria May 25, 2025
a0527da
Add misc-include-cleaner to .clang-tidy
fzakaria May 25, 2025
1552fb6
Make it only bugprone check
fzakaria May 27, 2025
1460f9f
Fix uninitialized variable in repl.hh
fzakaria May 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .clang-tidy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# We use pointers to aggregates in a couple of places, intentionally.
# void * would look weird.
Checks: '-bugprone-sizeof-expression'
Copy link
Contributor

@xokdvium xokdvium Jun 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can actually be a sequence, since .clang-tidy is in YAML format. So the checks can be composed like so:

Checks:
  - '-bugprone-sizeof-expression'
  - '-blahblah-glob-*'

Checks: '
-bugprone-sizeof-expression
'
10 changes: 10 additions & 0 deletions doc/manual/source/development/building.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,16 @@ To refresh pre-commit hook's config file, do the following:
1. Exit the development shell and start it again by running `nix develop`.
2. If you also use the pre-commit hook, also run `pre-commit-hooks-install` again.

#### Hooks

##### clang-tidy

Portions of the codebase are checked with [clang-tidy](https://clang.llvm.org/extra/clang-tidy/).

> clang-tidy is a clang-based C++ “linter” tool.

The current build system, Meson, by default outputs a `compile_commands.json` file which is used by all Clang tooling (clang-tidy, clangd etc..).

### VSCode

Insert the following json into your `.vscode/settings.json` file to configure `nixfmt`.
Expand Down
14 changes: 14 additions & 0 deletions maintainers/flake-module.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,20 @@
''^tests/functional/lang/eval-fail-set\.nix$''
];
};
clang-tidy = {
enable = true;
# TODO: this requires meson to have been configured
# we could optionally wrap this in a script that runs meson first
# but for now let us keep it simple
#
# clang-tidy doesn't work well running it on multiple files
# TODO: change this to use run-clang-tidy.py
entry = "${pkgs.writeShellScript "clang-tidy-wrapper" ''
for file in "$@"; do
"${pkgs.clang-tools}/bin/clang-tidy" --fix -p ./build "$file"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meson has clang-tidy and clang-tidy-fix targets. Can't we reuse that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea likely will do that but that runs on all targets.
I'm exploring how to add it as a pre-commit as well.
(We might abandon this change for now as we go about fixing warnings)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth noting that I did go over the codebase a bit with clang-tidy as a part of #11839. Thanks for picking up that activity!

Though we'd need to settle on a good clang-tidy config first. A lot of the checks are rather opinionated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to emit profile information to see which checks are really slow, so we can get something fast and iterate but I was hitting: llvm/llvm-project#141639

(Not sure if you faced this)

done
''}";
};
clang-format = {
enable = true;
# https://github.com/cachix/git-hooks.nix/pull/532
Expand Down
2 changes: 1 addition & 1 deletion src/libcmd/include/nix/cmd/repl.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ struct AbstractNixRepl
Bindings * autoArgs;

AbstractNixRepl(ref<EvalState> state)
: state(state)
: state(state), autoArgs(nullptr)
{ }

virtual ~AbstractNixRepl()
Expand Down
Loading