17 Jul 2025 |
fzakaria | Sorry i missed it; my kid does Junior Guards and today was his competition day. | 03:10:51 |
Mic92 | fzakaria: Sergei Zimmerman (xokdvium) https://github.com/Mic92/nix-1/commits/clang-tidy-bonanza/ | 12:13:09 |
Mic92 | Probably gonna split this into smaller prs | 12:13:32 |
Sergei Zimmerman (xokdvium) | Do we want the checks in CI to land after the fixes? Did we actually agree on an appropriate set of checks? | 12:29:31 |
Mic92 | I think we want them in CI. Takes a while to scan. | 13:21:38 |
Sergei Zimmerman (xokdvium) | I've tried cooking up yesterday. Nixpkgs also has clang-tidy-sarif to output sarif traces, that can be published to the PR as annotations via the codeql-action/upload-sarif@v1 action. Maybe that would work well with combination with git-clang-tidy? | 13:22:58 |
Mic92 | So you mean you only want to scan affected files? Good idea | 13:29:35 |
Mic92 | * So you mean you only want to scan changed files? Good idea | 13:29:42 |
Mic92 | Than we can enable even more checks. | 13:30:28 |
Sergei Zimmerman (xokdvium) | Yeah, it should be part of clang-tools-extra alongside clang-tidy, but nixpkgs build doesn't seem to have it for some reason. | 13:30:31 |
Sergei Zimmerman (xokdvium) | https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py | 13:31:00 |
fzakaria | Clang-tidy bonanza!? | 17:41:18 |
fzakaria | sweet -- the IWYU is what i'm really after. | 17:41:28 |
fzakaria | Makes navigating the codebase way easier. | 17:41:34 |
fzakaria | i'm going to try and take on another issue from FetchTree tonight and stream again... (I'm picking some easy ones for now) | 17:42:03 |
Alyssa Ross | IWYU is difficult for portable programs like Nix | 19:28:52 |
Alyssa Ross | Often the header something happens to be defined in for your platform isn't the one you should actually be including to be standards conformant and portable | 19:29:09 |
Alyssa Ross | (If it's possible to exclude libc headers rthat would probably help) | 19:30:31 |
18 Jul 2025 |
fzakaria | aren't we solving that with Nix ? | 02:45:02 |
fzakaria | like... we own all the header up until glibc... there's nothing non-portable | 02:45:16 |
emily | Nix runs on Linux and Darwin (and also to some extent FreeBSD, Windows, …) | 02:45:59 |
emily | so system header portability is still a concern (and has broken the build in the past) | 02:46:26 |
fzakaria | aren't those controlled by IFDEF macros so it's still IWYU..
anyways I don't think 100% is necessary | 02:54:29 |
fzakaria | but something more than today; I open any file and 1/2 of every include is either unused as per clangd or there's no include so it's difficult to follow the hierarchy | 02:54:53 |
fzakaria | like aspirational | 02:55:01 |
fzakaria | :) | 02:55:02 |
fzakaria | I found this bug in Nix -- I sent a failing test case to capture it
https://github.com/NixOS/nix/pull/13456
Dunno if we like having test cases that validate the failure mode -- albeit easy to flip | 02:56:26 |
emily | Alyssa means that including system headers can non-portably lead to the definition of a symbol that is only properly (portably) obtained by including a header that a standard specifies it's found in | 03:23:19 |
emily | because of nested includes in the system headers etc. | 03:23:25 |
emily | at least a naive "redundant include checker" would complain about including both such headers, even if it is the correct thing for portability across platforms | 03:23:59 |