Skip to content

Extract the compile DB in the configuration we develop in - #280

Merged
helly25 merged 1 commit into
mainfrom
compile_db_config_clang
Aug 9, 2026
Merged

Extract the compile DB in the configuration we develop in#280
helly25 merged 1 commit into
mainfrom
compile_db_config_clang

Conversation

@helly25

@helly25 helly25 commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Fixes the root cause that #279 treats symptomatically, and closes the last coverage hole in compile_commands.json.

Depends on helly25/bazel-compile-commands-extractor#30 conceptually (see Dropping the repo alias below), but does not need it merged — the alias is simply removed here.

The root cause

The DB was extracted in the default build configuration and only had its compiler substituted afterwards via --bcce-compiler. Every other flag — standard library, include paths, feature macros — came from whatever toolchain bazel picked by default, which differs per platform. That is what made clang-tidy report differently on macOS and Linux.

Now it extracts with --config=clang-tidy, a new .bazelrc config:

common:clang-tidy --config=clang
common:clang-tidy --cxxopt=-stdlib=libc++
common:clang-tidy --linkopt=-stdlib=libc++

The stdlib stays explicit on purpose: development targets libc++, so the DB should say so rather than inherit the driver default (libc++ on macOS, libstdc++ on Linux).

Consequently --bcce-compiler is now redundant — the extracted commands already name the hermetic toolchain — and is dropped, along with the ~34-line compiler-resolution block that supported it. --bcce-copt=-isysroot stays: macOS still needs the SDK for system C headers.

The missing sources

aquery expands //... with manual targets already removed, so the benchmarks, the differential test and show_compiler were never in the DB at all — they were being linted with flags interpolated from unrelated siblings. //bazelmod:refresh_compile_commands now names them explicitly.

It is a single targets entry: the key is interpolated into deps(...), so it may be a query expression, and a union avoids duplicating flags per target.

targets = {" + ".join(["@//..."] + CLANG_TIDY_MANUAL_TARGETS): "--config=clang-tidy"}

A tag alone cannot do this job — aquery's manual exclusion happens before any attr(tags, ...) filter can see the target, which I confirmed returns nothing extra. So the clang-tidy tag states the intent, and a new clang-tidy-targets-match-tag hook fails if the tag and the explicit list ever disagree. Verified it detects a removed entry and names exactly which target drifted.

The one repository source still absent, mbo/hash/measurements/smhasher3/mbohash.cpp, cannot be included: it is an SMHasher3 plugin, copied into that project by build_smhasher3.sh and compiled by its cmake. There is no BUILD file for it here, and linting it against these flags would only produce bogus errors for the Platform.h / Hashlib.h it includes.

Dropping the repo alias

bazel_dep(..., repo_name = "bazel_compile_commands_extractor") is removed. The extractor's macro emits a raw @hedron_compile_commands//:print_args label, which resolves in the caller's repo context and so fails under a rename. Fixed upstream in helly25/bazel-compile-commands-extractor#30 (with a regression test); removing the alias here is the local unblock and nothing else referenced it.

Test

  • compile_commands.json: 2034 entries, hermetic compiler, -stdlib=libc++ present, -isysroot retained, and every bazel-built source covered (was 6 missing).
  • clang-tidy works with the extracted commands, including on the previously-missing hash_benchmark.cc.
  • bazel test --config=clang //...109/109 pass.
  • pre-commit run -a green, including the new drift check.

The DB was extracted in the DEFAULT build configuration and only had its
compiler substituted afterwards (`--bcce-compiler`). Everything else - the
standard library, include paths, feature macros - therefore came from
whatever toolchain bazel picked by default, which differs per platform.
That is what made clang-tidy report differently on macOS and Linux.

Extract with `--config=clang-tidy` instead, a new .bazelrc config that is
`--config=clang` plus an explicit `-stdlib=libc++`. The stdlib stays
explicit on purpose: development targets libc++, and the DB should say so
rather than inherit the driver default (libc++ on macOS, libstdc++ on
Linux). `--bcce-compiler` is now redundant - the extracted commands
already name the hermetic toolchain - and is dropped along with the
compiler-resolution block that supported it. `--bcce-copt=-isysroot` stays:
macOS still needs the SDK for system C headers.

This also fixes the missing sources. aquery expands `//...` with `manual`
targets already removed, so the benchmarks, the differential test and
show_compiler were never in the DB at all and were being linted with
flags interpolated from unrelated siblings. `//bazelmod:refresh_compile_commands`
now names them explicitly. It is a SINGLE targets entry: the key is
interpolated into `deps(...)`, so it may be a query expression, and a
union needs no per-target flags duplication.

The `clang-tidy` tag on those targets states the intent, and
`clang-tidy-targets-match-tag` fails if the tag and the explicit list ever
disagree - verified to fail on a removed entry and to name exactly which
target drifted. A tag alone cannot do the job, because aquery's exclusion
happens before any `attr(tags, ...)` filter can see the target.

The refresh target lives in //bazelmod because the extractor is a
dev_dependency: the root package defines //:clang-format and //:is_clang,
which library targets reference, so a module consumer would load it and
fail on the missing dep. Nothing outside MODULE.bazel references
//bazelmod. The `repo_name` alias is dropped because the extractor's macro
emits a raw `@hedron_compile_commands//...` label, which resolves in the
CALLER's repo context and so breaks under a rename; that is fixed
upstream in helly25/bazel-compile-commands-extractor#30.

compile_commands.json: 2034 entries, every bazel-built source covered.
bazel test --config=clang //... - 109/109 pass.

Signed-off-by: helly25 <6420169+helly25@users.noreply.github.com>
@helly25
helly25 force-pushed the compile_db_config_clang branch from cf68868 to bcc2d43 Compare August 9, 2026 15:10
@helly25
helly25 enabled auto-merge (squash) August 9, 2026 15:20
@helly25
helly25 requested a review from Fab-Cat August 9, 2026 15:24
@helly25
helly25 merged commit 6c86258 into main Aug 9, 2026
23 checks passed
@helly25
helly25 deleted the compile_db_config_clang branch August 9, 2026 15:49
helly25 added a commit to helly25/xff that referenced this pull request Aug 9, 2026
…-tidy) (#420)

Adopt helly25/mbo#280's approach: extract compile_commands.json in the exact
configuration we develop in, rather than the default build config with a
post-hoc compiler substitution. A new .bazelrc `clang-tidy` config layers the
hermetic clang toolchain (--config=clang) plus an explicit libc++ (std + link),
and compile_commands-update.sh runs the extractor's internal aquery under it
(passed as a runtime flag, which the fork forwards to aquery). So clang-tidy /
clangd parse with the compiler, standard library, include paths and feature
macros the real --config=clang builds use, on every platform.

Two xff-specific deviations from mbo#280:

- Keep --bcce-compiler: xff's toolchain names `cc_wrapper.sh` in the extracted
  command, not a real clang++. clang-tidy reads that leading token to derive the
  driver target + resource dir, and a shell script leaves it with the wrong
  builtins (SDK / libc++ headers fail to parse). Substituting the hermetic
  clang++ binary fixes that; all OTHER flags still come from the config.

- macOS: drop the SDK libc++ redirection. On macOS toolchains_llvm emits
  `-nostdinc++ -cxx-isystem <SDK>/usr/include/c++/v1` (Xcode's libc++) while
  -resource-dir is the hermetic clang's; that mismatch silently degrades
  clang-tidy's analysis (spurious unused-variable / const-correctness findings).
  The hermetic clang++ finds its own libc++ by default, so a gated post-filter
  drops just those two flags; -isysroot stays for the system C headers. Linux
  never emits the SDK -cxx-isystem, so the filter is a no-op there.

Also materialize the generated / virtual-include headers in the SAME config
(bazel build --config=clang-tidy //...) so the forests land where the DB points,
and drop the now-redundant ~15-line default-config header-build note. Verified:
DB names the hermetic clang++ with libc++, and clang-tidy parses the tree clean.
helly25 added a commit that referenced this pull request Aug 9, 2026
#275 made `test` and `item` const in limited_set_benchmark.h, but both
are assigned:

  ./mbo/container/limited_set_benchmark.h:134:11: error: cannot assign to
  variable 'item' with const-qualified type 'const std::size_t'

It reached main because `bazel test //...` skips `manual` targets, so
nothing built the benchmark. Only #280 - which put those targets into the
compile DB - made clang-tidy able to see it. Reverted, and CI now builds
every clang-tidy-tagged manual target so the next one cannot slip through
the same way. All five build.

tools/clang_tidy.sh now skips mbo/hash/measurements/smhasher3/: bazel does
not build it (it is an SMHasher3 plugin compiled by that project's cmake),
so clang-tidy was linting it with flags guessed from unrelated files and
reporting its SMHasher3 includes as missing. That removes 4 spurious
categories at once, including all 10 c-style-cast findings.

Disabled, each with its reason recorded:
  * misc-no-recursion - recursion is the shape of the code it flags
    (Stringify walks nested structures, BigNumberLen recurses once for
    negatives), and it reports every member of a call chain.
  * bugprone-std-namespace-modification - the `namespace std` blocks are
    specialisations of std templates for our own types, which is how
    those extension points work.
  * cppcoreguidelines-avoid-magic-numbers / readability-magic-numbers
    (aliases) - they fire on test expectations and on the numeric tables
    that ARE the subject.

bazel test --config=clang //... - 109/109 pass.

Signed-off-by: helly25 <6420169+helly25@users.noreply.github.com>
helly25 added a commit that referenced this pull request Aug 9, 2026
#275 made `test` and `item` const in limited_set_benchmark.h, but both
are assigned:

  ./mbo/container/limited_set_benchmark.h:134:11: error: cannot assign to
  variable 'item' with const-qualified type 'const std::size_t'

It reached main because `bazel test //...` skips `manual` targets, so
nothing built the benchmark. Only #280 - which put those targets into the
compile DB - made clang-tidy able to see it. Reverted, and CI now builds
every clang-tidy-tagged manual target so the next one cannot slip through
the same way. All five build.

tools/clang_tidy.sh now skips mbo/hash/measurements/smhasher3/: bazel does
not build it (it is an SMHasher3 plugin compiled by that project's cmake),
so clang-tidy was linting it with flags guessed from unrelated files and
reporting its SMHasher3 includes as missing. That removes 4 spurious
categories at once, including all 10 c-style-cast findings.

Disabled, each with its reason recorded:
  * misc-no-recursion - recursion is the shape of the code it flags, and
    it reports every member of a call chain.
  * bugprone-std-namespace-modification - the `namespace std` blocks are
    specialisations of std templates for our own types.
  * cppcoreguidelines-avoid-magic-numbers / readability-magic-numbers
    (aliases) - they fire on test expectations and on the numeric tables
    that ARE the subject.

bazel test --config=clang //... - 109/109 pass.

Signed-off-by: helly25 <6420169+helly25@users.noreply.github.com>
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.

2 participants