Skip to content

nix: Make clang tooling and IWYU find the right standard library headers - #333

Open
ryanofsky wants to merge 4 commits into
bitcoin-core:masterfrom
ryanofsky:pr/iwyumatch
Open

nix: Make clang tooling and IWYU find the right standard library headers#333
ryanofsky wants to merge 4 commits into
bitcoin-core:masterfrom
ryanofsky:pr/iwyumatch

Conversation

@ryanofsky

@ryanofsky ryanofsky commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

On Nix the LLVM analysis tools (clangd, clang-tidy, clang-check, include-what-you-use and others) need two things to work correctly: they need find the standard library headers at all, and they need to find same standard library the build uses (libc++ vs libstdc++). This PR makes the tools resolve the right headers on their own in every context they run in: editors, command line, and CI.

PR was motivated by recent discussion: #296 (review) and includes documentation improvements, new IWYU changes, and an old clang-tools change I've been using for a long time locally.

Specifically the PR:

  • puts the header-aware clang-tools binaries ahead of the raw clang ones on PATH, so clangd/clang-tidy/clang-check find the standard library on their own;
  • binds IWYU to the shell's compiler so it analyzes the same standard library the build uses in any context;
  • provides IWYU only in the non-minimal shells that actually run it; and
  • updates the comment on the build's CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES workaround — which injects these header paths into the compile database when clang-tidy or IWYU are enabled — to note that clang-tidy no longer needs it and IWYU still does.

Details are in commit messages.

ryanofsky and others added 4 commits August 3, 2026 18:29
The workaround was added in 977d721 so clang-tidy could find headers that nix
compiler wrappers inject internally. Its comment has since become inaccurate and
misleading:

- It names clang-tidy as the tool that needs the workaround, but clang-tidy no
  longer does: the nixpkgs clang-tools wrapper was fixed upstream
  (NixOS/nixpkgs#462747). IWYU is the tool that needs it
  now.
- It says the tool "ignores $NIX_CFLAGS_COMPILE." In fact the nixpkgs
  analysis-tool wrappers do read $NIX_CFLAGS_COMPILE and re-add its paths.
- It says the missing headers are capnp (dependency) headers. That is backwards:
  dependency headers are passed via -isystem in $NIX_CFLAGS_COMPILE and are
  found. The headers that go missing are the C++ standard library headers (e.g.
  <cstddef>), because the include-what-you-use wrapper drops the -cxx-isystem
  flags those use.
- It pins the whole mechanism on $NIX_CFLAGS_COMPILE, omitting that standard
  library paths are injected through the compiler wrapper's own flag files, and
  it neither explains why the tools normally cope nor that the workaround only
  compensates for a temporary wrapper bug.

Rewrite the comment to describe the nix header-injection mechanism from first
principles, identify IWYU's dropped -cxx-isystem flags as the specific reason
the workaround is still needed, and note that it is removable once that wrapper
bug is fixed upstream.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Since cbb1e43 the llvm CI job is intended to test libc++ instead of
libstdc++, in the build and in IWYU. That works, but only accidentally: the
nixpkgs include-what-you-use wrapper bakes in the include paths of the
toolchain IWYU was built against (libstdc++ on Linux), and IWYU only sees
libc++ headers because the explicit -isystem flags generated by the
CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES workaround in CMakeLists.txt take
precedence over the wrapper's environment variables. Relying on that is
fragile: IWYU invoked outside the CMake build (or after that workaround is
removed) silently analyzes libstdc++, with a mapping file that only matches
libc++.

Make shell.nix responsible for this instead: rebind the clang recorded in
the IWYU wrapper to the shell's compiler, so IWYU resolves the same standard
library the build uses in any context. Also expand the IWYU_MAPPING_FILE
comment to explain how the mapping file is consumed and kept consistent.

The override changes the IWYU derivation, so shells rebuild it from source
once per channel instead of fetching it from the binary cache.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move include-what-you-use into the non-minimal tool group alongside clang
and clang-tools. Minimal shells, used by the cross-compiling gnu32 job, do
not run analysis tools, and after the previous commit shipping IWYU there
would pull the cross clang closure into the shell and rebuild IWYU per cross
target for no benefit. Cross shells that do provide IWYU get the cross
toolchain's target headers baked in, which is what analyzing a cross build
requires (the embedded clang frontend parses in target mode given a matching
--target flag).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The clang-tools package provides clangd, clang-tidy, clang-check and about
20 other clang-tools-extra programs, wrapped so they can find the standard
library headers on Nix. The clang package provides the same programs too,
but as raw binaries that cannot. Both end up on the PATH, and
nativeBuildInputs order sets PATH priority (the earlier entry wins), so
list clang-tools first to make the working copies win. This lets clangd
(in an editor) and clang-tidy or clang-check (run by hand) resolve
<cstddef> and the rest of the standard library. Without it, the raw
clang-check fails immediately over any source file:

    include/mp/util.h:8:10: fatal error: 'array' file not found

The wrapping is a Nix quirk. On a normal system the standard library lives
in a default location like /usr/include that clang searches automatically,
so these tools work out of the box. Nix has no such default: glibc and
libstdc++ live in isolated store paths, and only the clang compiler wrapper
knows where. It injects the right -isystem paths when it compiles, but
standalone tools like clang-check and clangd run clang's parser directly,
never going through the compiler wrapper, so they need another way to learn
the paths.

That is what the clang-tools wrappers do. Before running the real tool,
each reads the libc-cflags and libcxx-cxxflags files from the clang
compiler wrapper, which hold the -idirafter and -cxx-isystem flags for the
glibc and libstdc++ header directories. The wrapper copies those
directories into the C_INCLUDE_PATH and CPLUS_INCLUDE_PATH environment
variables, which clang reads and adds to its header search path. The raw
binaries do none of this.

Why does the clang package ship these tools at all? It exposes them only
as a side effect: its cc-wrapper setup-hook adds the whole unwrapped-clang
bin/ to the PATH so the compiler driver and adjacent programs are
reachable, and upstream LLVM installs the clang-tools-extra programs into
that same bin/.

Why not fix this in CMake instead? CMakeLists.txt already does, as an
alternative workaround: it adds the compiler's implicit include
directories to the compile database via CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES,
so tools that read compile_commands.json find the standard headers
regardless of package order. But it is only enabled alongside IWYU or
clang-tidy, so it is not always present, and it is more fragile and
nonstandard: it bakes the detected store paths into the compile commands as
explicit -isystem flags that would not normally be there. Ordering
clang-tools first fixes the tools themselves, so they work independently of
the CMake configuration.

This does not affect include-what-you-use, which is a separate package
with no PATH collision and its own wrapper script.

https://web.archive.org/web/20260311024938/https://blog.kotatsu.dev/posts/2024-04-10-nixpkgs-clangd-missing-headers/
NixOS/nixpkgs#76486

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@DrahtBot

DrahtBot commented Aug 6, 2026

Copy link
Copy Markdown

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers
Concept ACK hebasto

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #231 (Add windows support by ryanofsky)
  • #212 (ci: add newdeps job testing newer versions of cmake and capnproto by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Concept ACK.

@ryanofsky

Copy link
Copy Markdown
Collaborator Author

Updated 505c358 -> 03329f6 (pr/iwyumatch.7 -> pr/iwyumatch.8, compare) fixing more problems in CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants