Skip to content

include-what-you-use 0.25 fixes#797

Merged
laurynas-biveinis merged 1 commit into
masterfrom
iwyu-0.25
Nov 4, 2025
Merged

include-what-you-use 0.25 fixes#797
laurynas-biveinis merged 1 commit into
masterfrom
iwyu-0.25

Conversation

@laurynas-biveinis
Copy link
Copy Markdown
Collaborator

@laurynas-biveinis laurynas-biveinis commented Nov 4, 2025

Summary by CodeRabbit

  • New Features

    • Added additional node specializations and test-friendly type aliases to support internal node handling and testing.
    • Introduced compile-time size validations for those test types.
  • Chores

    • Simplified forward declarations and header ordering.
    • Cleaned up include directives and static-analysis pragma annotations across the codebase.

@laurynas-biveinis laurynas-biveinis self-assigned this Nov 4, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 4, 2025

Walkthrough

This PR removes or adjusts many IWYU pragma/include annotations across sources and headers and adds forward declarations and public ART inode class templates, an inode_defs alias, test-type aliases, and size static_asserts in include/unodb/art.hpp.

Changes

Cohort / File(s) Summary
ART header additions
include/unodb/art.hpp
Removed IWYU pragmas and <limits> include; added forward declarations and public template classes inode_4/16/48/256<Key,Value>, alias inode_defs<Key,Value>, test-type aliases (inode_*_test_type) and size static_asserts (with UNODB_DETAIL_AVX2 guard).
ART internals & common
include/unodb/art_common.hpp, src/art_internal.cpp, include/unodb/art_internal.hpp, include/unodb/art_internal_impl.hpp
Dropped/adjusted IWYU pragmas around global.hpp; added/removed some no_include pragmas and reordered/simplified includes.
Core headers (global first-include changes)
include/unodb/heap.hpp, include/unodb/in_fake_critical_section.hpp, include/unodb/olc_art.hpp, include/unodb/portability_builtins.hpp, include/unodb/thread_sync.hpp, include/unodb/qsbr_ptr.hpp
Replaced #include "global.hpp" // IWYU pragma: keep with plain #include "global.hpp" and added minor no_include annotations in some files; no API/behavior changes.
Headers with removed dependencies
include/unodb/mutex_art.hpp, include/unodb/optimistic_lock.hpp
Removed some includes (e.g., assert.hpp, node_type.hpp, <iterator>, <optional>) and adjusted IWYU pragmas.
QSBR & related
include/unodb/qsbr.hpp, include/unodb/qsbr_ptr.hpp, src/qsbr.cpp
Adjusted IWYU no_include/keep pragmas and include ordering for transitive headers (vector/ostream/byte); reordered some includes.
Benchmark sources & headers
benchmark/* (e.g., benchmark/micro_benchmark.cpp, benchmark/micro_benchmark_*.cpp, benchmark/micro_benchmark_*.hpp, benchmark/micro_benchmark_node_utils.hpp, benchmark/micro_benchmark_utils.*)
Replaced/removed explicit includes (<array>, <vector>, <tuple>, etc.), added/adjusted IWYU no_include/keep pragmas, and reordered includes; no logic changes.
Tests
test/* (e.g., test/*.cpp, test/*.hpp, test/db_test_utils.*, test/qsbr_*.cpp, test/test_*.cpp, test/test_utils.hpp, test_heap.*)
Reordered includes, added/removed no_include pragmas (e.g., __cstddef/byte.h, __new/exceptions.h, <string>), moved keep annotations; no API/behavior changes.
Examples & fuzzing
examples/example_*.cpp, fuzz_deepstate/test_art_fuzz_deepstate.cpp
Removed several IWYU no_include/keep pragmas and simplified include patterns; adjusted some include annotations.

Sequence Diagram(s)

(omitted — changes are header/pragma edits and type-forwarding additions; no runtime control-flow change to diagram)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas needing extra attention:
    • include/unodb/art.hpp — verify the correctness of new template forward declarations, inode_defs alias, test-type aliases and conditional static_asserts (guarded by UNODB_DETAIL_AVX2).
    • Files where includes were removed (e.g., mutex_art.hpp, optimistic_lock.hpp) — ensure no implicit transitive dependencies were relied upon.
    • Test files and benchmarks — run test suite / build to catch missing includes exposed by IWYU pragma removals.

Possibly related PRs

Poem

✨ Pragmas trimmed with tidy care,

Headers lean, forward types laid bare.
Inodes named in neat new rows,
Static checks keep size on shows.
Build the tests — let the errors show.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately describes the main focus of the changeset: addressing compatibility issues with include-what-you-use (IWYU) version 0.25.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch iwyu-0.25

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2918f45 and ce8594e.

📒 Files selected for processing (49)
  • art.hpp (2 hunks)
  • art_common.hpp (1 hunks)
  • art_internal.cpp (1 hunks)
  • art_internal.hpp (1 hunks)
  • art_internal_impl.hpp (1 hunks)
  • benchmark/micro_benchmark.cpp (1 hunks)
  • benchmark/micro_benchmark_concurrency.hpp (1 hunks)
  • benchmark/micro_benchmark_key_prefix.cpp (1 hunks)
  • benchmark/micro_benchmark_mutex.cpp (0 hunks)
  • benchmark/micro_benchmark_n16.cpp (0 hunks)
  • benchmark/micro_benchmark_n256.cpp (0 hunks)
  • benchmark/micro_benchmark_n4.cpp (1 hunks)
  • benchmark/micro_benchmark_n48.cpp (0 hunks)
  • benchmark/micro_benchmark_node_utils.hpp (1 hunks)
  • benchmark/micro_benchmark_utils.cpp (1 hunks)
  • benchmark/micro_benchmark_utils.hpp (1 hunks)
  • examples/example_art.cpp (1 hunks)
  • examples/example_art_stats.cpp (1 hunks)
  • examples/example_olc_art.cpp (1 hunks)
  • fuzz_deepstate/test_art_fuzz_deepstate.cpp (1 hunks)
  • heap.hpp (1 hunks)
  • in_fake_critical_section.hpp (1 hunks)
  • mutex_art.hpp (1 hunks)
  • olc_art.hpp (1 hunks)
  • optimistic_lock.hpp (1 hunks)
  • portability_builtins.hpp (1 hunks)
  • qsbr.cpp (2 hunks)
  • qsbr.hpp (2 hunks)
  • qsbr_ptr.hpp (1 hunks)
  • test/db_test_utils.cpp (1 hunks)
  • test/db_test_utils.hpp (2 hunks)
  • test/gtest_utils.hpp (1 hunks)
  • test/qsbr_gtest_utils.cpp (1 hunks)
  • test/qsbr_gtest_utils.hpp (1 hunks)
  • test/qsbr_test_utils.cpp (1 hunks)
  • test/test_art.cpp (1 hunks)
  • test/test_art_concurrency.cpp (2 hunks)
  • test/test_art_iter.cpp (0 hunks)
  • test/test_art_key_view.cpp (1 hunks)
  • test/test_art_oom.cpp (1 hunks)
  • test/test_art_scan.cpp (2 hunks)
  • test/test_key_encode_decode.cpp (2 hunks)
  • test/test_qsbr.cpp (1 hunks)
  • test/test_qsbr_oom.cpp (1 hunks)
  • test/test_qsbr_ptr.cpp (1 hunks)
  • test/test_utils.hpp (1 hunks)
  • test_heap.cpp (2 hunks)
  • test_heap.hpp (1 hunks)
  • thread_sync.hpp (1 hunks)
💤 Files with no reviewable changes (5)
  • benchmark/micro_benchmark_n256.cpp
  • benchmark/micro_benchmark_mutex.cpp
  • test/test_art_iter.cpp
  • benchmark/micro_benchmark_n48.cpp
  • benchmark/micro_benchmark_n16.cpp
🧰 Additional context used
🧬 Code graph analysis (2)
benchmark/micro_benchmark_node_utils.hpp (3)
optimistic_lock.hpp (3)
  • NDEBUG (403-414)
  • NDEBUG (822-826)
  • NDEBUG (829-835)
qsbr.hpp (1)
  • NDEBUG (1065-1081)
qsbr_ptr.hpp (2)
  • NDEBUG (154-163)
  • NDEBUG (176-185)
test_heap.hpp (3)
optimistic_lock.hpp (3)
  • NDEBUG (403-414)
  • NDEBUG (822-826)
  • NDEBUG (829-835)
qsbr.hpp (1)
  • NDEBUG (1065-1081)
qsbr_ptr.hpp (2)
  • NDEBUG (154-163)
  • NDEBUG (176-185)
🔇 Additional comments (49)
in_fake_critical_section.hpp (1)

20-20: Removing IWYU pragma: keep is correct here.

The global.hpp include is genuinely used—line 49 references the UNODB_DETAIL_RELEASE_CONST macro from it. The pragma removal reflects that IWYU 0.25 now properly detects macro dependencies, so the workaround is no longer needed.

thread_sync.hpp (1)

9-9: Include cleanup looks safe.
Dropping the IWYU pragma leaves behavior unchanged.

test_heap.hpp (1)

15-24: IWYU directives look correct.
The new no_include and keep pragmas match the header’s needs.

benchmark/micro_benchmark_n4.cpp (1)

7-13: Vector header guidance is fine.
The added IWYU hints won’t affect benchmarks.

examples/example_art_stats.cpp (1)

7-7: Global include change is harmless.
Removing the keep pragma doesn’t alter semantics.

benchmark/micro_benchmark_utils.cpp (1)

6-11: Pragmas align with usage.
Keeping these includes for IWYU is appropriate.

test/test_qsbr_ptr.cpp (1)

4-4: Plain global include works.
No functional difference here.

test/gtest_utils.hpp (1)

15-15: Header still fine without the pragma.
No impact on the test helpers.

test/test_utils.hpp (1)

13-13: LGTM.

Removing the IWYU pragma is consistent with the broader cleanup in this PR.

mutex_art.hpp (1)

6-6: LGTM.

IWYU pragma removal is consistent with this PR's cleanup pattern.

test/test_art_scan.cpp (2)

7-7: LGTM.

Suppressing internal implementation headers while keeping the public <vector> header is standard IWYU practice.


16-16: LGTM.

The keep pragma ensures IWYU doesn't suggest removing <vector> when it only sees transitive includes from implementation headers.

portability_builtins.hpp (1)

11-11: LGTM.

Consistent IWYU pragma cleanup.

test/qsbr_gtest_utils.cpp (1)

4-9: LGTM.

The pragma adjustments are consistent with the PR's IWYU cleanup objectives.

heap.hpp (1)

9-9: LGTM.

Standard pragma removal consistent with the PR.

examples/example_art.cpp (1)

7-7: LGTM.

Final pragma cleanup consistent with the rest of the PR.

art_internal_impl.hpp (1)

6-6: LGTM - IWYU pragma cleanup

Removing the IWYU pragma: keep annotation while preserving the first-include position of global.hpp is correct. No functional impact.

examples/example_olc_art.cpp (1)

7-7: LGTM - consistent IWYU cleanup

Same pattern as other files in this PR. Removing the pragma while keeping the first-include semantics.

art_internal.hpp (1)

6-8: LGTM - IWYU pragma modernization

Two changes here: removing keep from global.hpp and adding no_include for the internal __cstddef/byte.h header. Both are proper IWYU hygiene.

test/test_qsbr.cpp (2)

9-9: LGTM - updated IWYU exclusion

Changed from excluding <string> to excluding <gtest/gtest.h>. Reasonable adjustment to IWYU configuration.


11-11: LGTM - added cmath include

Added <cmath> include. No issues.

qsbr_ptr.hpp (1)

14-16: LGTM - standard IWYU cleanup

Same pattern: remove keep from global.hpp, add no_include for internal header. Consistent with other files in this PR.

benchmark/micro_benchmark_key_prefix.cpp (2)

6-6: LGTM - exclude internal vector header

Changed to exclude <__vector/vector.h> (internal implementation header). Works together with line 10's keep pragma on the public <vector> header.


10-10: LGTM - keep public vector header

Added keep pragma to ensure <vector> stays. Pairs correctly with the no_include for the internal header at line 6.

art_common.hpp (2)

6-6: LGTM - remove keep pragma

Consistent removal of IWYU pragma: keep from global.hpp.


12-12: LGTM - simplified iosfwd include

Cleaned up the <iosfwd> include by removing IWYU pragma annotations. Simpler is better.

qsbr.cpp (3)

12-12: LGTM - standard pragma removal

Removing keep from global.hpp - consistent with the entire PR.


14-14: LGTM - exclude internal new header

Added no_include for internal __new/exceptions.h. Works with the keep on public <new> at line 24.


24-24: LGTM - keep public new header

Added keep pragma for <new>. Pairs correctly with excluding the internal header at line 14.

test_heap.cpp (1)

17-17: LGTM - Standard IWYU 0.25 adjustments.

The pragma changes are correct: removed unnecessary keep from global.hpp, added no_include for the internal __new/new_handler.h header, and preserved <new> with keep since std::get_new_handler and std::bad_alloc are used.

Also applies to: 19-20, 30-30

test/test_qsbr_oom.cpp (1)

8-9: LGTM - IWYU pragmas are correct.

The no_include pragmas for __new/exceptions.h and gtest/gtest.h are appropriate. There's no direct include of gtest/gtest.h in this file—it's transitively included via utility headers. The no_include pragma correctly tells IWYU not to suggest adding it directly.

The past review comment about contradictory pragmas doesn't apply to the current code.

Also applies to: 13-13

benchmark/micro_benchmark_concurrency.hpp (1)

6-6: LGTM - Consistent IWYU adjustment.

Removed unnecessary keep pragma from global.hpp. The first-include requirement is preserved by ordering.

optimistic_lock.hpp (1)

214-214: LGTM - Standard pragma cleanup.

Removed unnecessary keep pragma from global.hpp, consistent with the project-wide IWYU updates.

test/test_art_concurrency.cpp (1)

4-4: LGTM - IWYU pragma adjustments.

Removed keep pragma from global.hpp and adjusted include ordering. Changes are consistent with the broader IWYU 0.25 migration.

Also applies to: 22-22

qsbr.hpp (1)

25-25: LGTM - Correct IWYU annotations.

Standard adjustments: removed keep from global.hpp, added no_include for internal __vector/vector.h, and preserved <vector> with keep since it's used throughout the file.

Also applies to: 28-28, 46-46

test/test_art_oom.cpp (1)

8-8: LGTM - IWYU pragma and ordering adjustments.

Added no_include for internal __new/exceptions.h header, preserved <new> with keep (needed for std::bad_alloc), and adjusted include ordering. All changes are correct.

Also applies to: 13-15

test/db_test_utils.hpp (1)

6-6: LGTM - Complete IWYU pragma update.

Removed keep from global.hpp, added no_include pragmas for internal headers __cstddef/byte.h and __vector/vector.h, and preserved <vector> with keep. Changes are correct and consistent with the rest of the PR.

Also applies to: 8-8, 10-10, 26-26

test/db_test_utils.cpp (1)

4-6: Include order/IWYU hint — OK

global.hpp first and keeping db_test_utils.hpp for IWYU is fine. No functional impact.

art_internal.cpp (1)

4-4: First-include pragma removal — OK

No effect on compilation or behavior; remaining includes cover usage.

test/test_art_key_view.cpp (1)

6-13: IWYU adjustments and includes — OK

no_include pragmas + keeping align with usage (std::byte, std::ignore). Looks good.

test/qsbr_test_utils.cpp (1)

7-7: IWYU pragma move — OK

Pure annotation shuffle; no runtime impact.

benchmark/micro_benchmark_node_utils.hpp (1)

18-21: Debug-only iostream include — OK

Including only in !NDEBUG matches usage (debug prints). Good.

fuzz_deepstate/test_art_fuzz_deepstate.cpp (1)

21-22: LGTM - appropriate IWYU pragma annotations.

The pragma: keep directives correctly protect these core includes from removal by IWYU.

test/test_art.cpp (3)

6-10: LGTM - correct IWYU pragma usage.

The no_include directives prevent IWYU from suggesting implementation-detail headers and unused standard headers. The pragma: keep on <cstddef> ensures std::byte remains available.


14-14: LGTM - explicit include for std::ignore.

The code uses std::ignore (lines 78, 168), which is defined in <tuple>. This makes the dependency explicit per IWYU principles.


16-16: LGTM - explicit GTest header.

The file uses GTest macros throughout (UNODB_TYPED_TEST, UNODB_ASSERT_*, etc.). Making this dependency explicit is correct.

benchmark/micro_benchmark.cpp (2)

6-8: LGTM - prevents IWYU from suggesting unnecessary headers.

The no_include pragmas correctly block implementation-detail headers and <array> (which is only needed transitively through included headers).


12-12: LGTM - replaced unused <array> with needed <tuple>.

The file uses std::ignore (lines 149, 186) but not std::array directly. The swap is correct.

benchmark/micro_benchmark_utils.hpp (1)

6-12: LGTM - correct IWYU pragma adjustments.

The changes correctly protect <cstddef> (provides std::byte used in lines 51-55), prevent suggesting the implementation-detail <__cstddef/byte.h>, and remove unnecessary pragma from global.hpp.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
benchmark/micro_benchmark_node_utils.hpp (1)

8-18: Restore required standard headers

This translation unit still uses std::ranges::generate, std::ranges::shuffle, std::numeric_limits, std::cerr, and std::string, but the <algorithm>, <limits>, <iostream>, and <string> headers were just removed. That leaves the file relying on transitive inclusions and breaks the build with any compiler that enforces include hygiene.(cppreference.net)

 #include <array>
 #include <cstddef>
 #include <cstdint>
+#include <algorithm>
+#include <iostream>
+#include <limits>
 #include <random>
 #include <tuple>
 #include <vector>
+#include <string>
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c2a15c5 and 213ee35.

📒 Files selected for processing (49)
  • art.hpp (2 hunks)
  • art_common.hpp (1 hunks)
  • art_internal.cpp (1 hunks)
  • art_internal.hpp (1 hunks)
  • art_internal_impl.hpp (1 hunks)
  • benchmark/micro_benchmark.cpp (1 hunks)
  • benchmark/micro_benchmark_concurrency.hpp (1 hunks)
  • benchmark/micro_benchmark_key_prefix.cpp (1 hunks)
  • benchmark/micro_benchmark_mutex.cpp (0 hunks)
  • benchmark/micro_benchmark_n16.cpp (0 hunks)
  • benchmark/micro_benchmark_n256.cpp (0 hunks)
  • benchmark/micro_benchmark_n4.cpp (1 hunks)
  • benchmark/micro_benchmark_n48.cpp (0 hunks)
  • benchmark/micro_benchmark_node_utils.hpp (1 hunks)
  • benchmark/micro_benchmark_utils.cpp (1 hunks)
  • benchmark/micro_benchmark_utils.hpp (1 hunks)
  • examples/example_art.cpp (1 hunks)
  • examples/example_art_stats.cpp (1 hunks)
  • examples/example_olc_art.cpp (1 hunks)
  • fuzz_deepstate/test_art_fuzz_deepstate.cpp (1 hunks)
  • heap.hpp (1 hunks)
  • in_fake_critical_section.hpp (1 hunks)
  • mutex_art.hpp (1 hunks)
  • olc_art.hpp (1 hunks)
  • optimistic_lock.hpp (1 hunks)
  • portability_builtins.hpp (1 hunks)
  • qsbr.cpp (2 hunks)
  • qsbr.hpp (2 hunks)
  • qsbr_ptr.hpp (1 hunks)
  • test/db_test_utils.cpp (1 hunks)
  • test/db_test_utils.hpp (2 hunks)
  • test/gtest_utils.hpp (1 hunks)
  • test/qsbr_gtest_utils.cpp (1 hunks)
  • test/qsbr_gtest_utils.hpp (1 hunks)
  • test/qsbr_test_utils.cpp (1 hunks)
  • test/test_art.cpp (1 hunks)
  • test/test_art_concurrency.cpp (2 hunks)
  • test/test_art_iter.cpp (0 hunks)
  • test/test_art_key_view.cpp (1 hunks)
  • test/test_art_oom.cpp (1 hunks)
  • test/test_art_scan.cpp (2 hunks)
  • test/test_key_encode_decode.cpp (1 hunks)
  • test/test_qsbr.cpp (1 hunks)
  • test/test_qsbr_oom.cpp (1 hunks)
  • test/test_qsbr_ptr.cpp (1 hunks)
  • test/test_utils.hpp (1 hunks)
  • test_heap.cpp (2 hunks)
  • test_heap.hpp (1 hunks)
  • thread_sync.hpp (1 hunks)
💤 Files with no reviewable changes (5)
  • benchmark/micro_benchmark_mutex.cpp
  • test/test_art_iter.cpp
  • benchmark/micro_benchmark_n256.cpp
  • benchmark/micro_benchmark_n48.cpp
  • benchmark/micro_benchmark_n16.cpp
🧰 Additional context used
🧬 Code graph analysis (1)
test_heap.hpp (3)
optimistic_lock.hpp (3)
  • NDEBUG (403-414)
  • NDEBUG (822-826)
  • NDEBUG (829-835)
qsbr.hpp (1)
  • NDEBUG (1065-1081)
qsbr_ptr.hpp (2)
  • NDEBUG (154-163)
  • NDEBUG (176-185)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
🔇 Additional comments (35)
optimistic_lock.hpp (1)

214-214: LGTM - Clean include hygiene update.

The removal of the IWYU pragma and unused includes is correct. All remaining includes are actually used in the implementation.

test/test_key_encode_decode.cpp (3)

4-10: LGTM - Standard IWYU pragma cleanup.

The removal of the keep pragma from global.hpp and the addition of no_include pragmas for internal headers are standard IWYU practices.


15-15: LGTM - cstddef is used throughout.

The IWYU keep pragma is correct since std::byte is used extensively in this file.


25-25: LGTM - vector is used.

The IWYU keep pragma is correct since std::vector is used in the key_factory class (line 669).

thread_sync.hpp (1)

9-9: LGTM: IWYU pragma cleanup.

The removal of the IWYU pragma is consistent with the broader cleanup effort across the codebase.

test/gtest_utils.hpp (1)

15-15: LGTM: Consistent IWYU pragma cleanup.

Aligns with the IWYU 0.25 migration across the codebase.

benchmark/micro_benchmark_utils.cpp (1)

6-6: LGTM: IWYU pragma formatting improvement.

Moving the pragma comments to the same line as the includes improves readability.

Also applies to: 10-10

test_heap.hpp (1)

15-15: LGTM: Appropriate IWYU pragma additions.

The pragmas correctly exclude implementation headers while retaining <new> for std::bad_alloc used at line 75.

Also applies to: 23-23

in_fake_critical_section.hpp (1)

20-20: LGTM: IWYU pragma cleanup.

Consistent with the IWYU 0.25 fixes applied across the codebase.

portability_builtins.hpp (1)

11-11: LGTM: IWYU pragma cleanup.

Aligns with the broader IWYU 0.25 migration.

test/test_qsbr_ptr.cpp (1)

4-4: LGTM: IWYU pragma cleanup.

Removes the pragma from global.hpp while appropriately retaining other IWYU directives.

examples/example_olc_art.cpp (1)

7-7: LGTM: IWYU pragma cleanup.

Simplifies the include by removing the pragma directive, consistent with the IWYU 0.25 fixes across the codebase.

examples/example_art_stats.cpp (1)

7-7: LGTM - IWYU pragma cleanup.

The removal of IWYU directives is consistent with the broader IWYU 0.25 hygiene effort.

benchmark/micro_benchmark_key_prefix.cpp (1)

6-10: LGTM - Vector include pragma adjustment.

The shift from <string> to <__vector/vector.h> in the no_include directive aligns with the repository-wide IWYU cleanup pattern.

qsbr_ptr.hpp (1)

14-16: LGTM - IWYU pragma refinement.

Removing the keep directive and adding the no_include for the internal byte header is a standard IWYU refinement.

heap.hpp (1)

9-9: LGTM - Pragma cleanup.

Simple removal of unnecessary IWYU keep directive.

benchmark/micro_benchmark_n4.cpp (1)

7-12: LGTM - Consistent vector pragma pattern.

Matches the IWYU adjustments in other benchmark files.

art_internal_impl.hpp (1)

6-6: LGTM - Pragma removal.

Consistent with the project-wide IWYU cleanup.

examples/example_art.cpp (1)

7-7: LGTM - IWYU pragma cleanup.

Straightforward removal of unnecessary IWYU directives.

benchmark/micro_benchmark.cpp (1)

6-12: LGTM—IWYU pragma cleanup.

These pragma adjustments align with IWYU 0.25's analysis. No functional impact.

art_internal.hpp (1)

6-8: LGTM—IWYU pragma adjustments.

Removing keep from global.hpp and excluding platform-specific byte header is consistent with the PR's cleanup pattern.

benchmark/micro_benchmark_concurrency.hpp (1)

6-6: LGTM—IWYU pragma cleanup.

Removing keep pragma from global.hpp is consistent with the PR pattern.

art_internal.cpp (1)

4-4: LGTM—IWYU pragma cleanup.

Removing keep pragma from global.hpp matches the consistent pattern across all modified files.

qsbr.cpp (1)

12-24: LGTM—IWYU pragma adjustments.

Removing keep from global.hpp, excluding platform-specific new header, and explicitly keeping <new> are appropriate IWYU directives.

qsbr.hpp (1)

25-46: LGTM—IWYU pragma adjustments.

The pattern of excluding platform-specific vector headers while keeping the standard <vector> include is correct.

test_heap.cpp (1)

19-30: LGTM—IWYU pragma adjustments.

Excluding platform-specific new_handler header while keeping <new> follows the established pattern in this PR.

benchmark/micro_benchmark_utils.hpp (1)

6-12: LGTM—IWYU pragma adjustments.

Excluding platform-specific byte header while keeping standard <cstddef> completes the consistent IWYU cleanup pattern across all files in this PR.

test/test_art_scan.cpp (1)

7-7: LGTM! Standard IWYU hygiene.

The pragmas correctly suppress the internal vector implementation header while ensuring the public <vector> header is retained.

Also applies to: 16-16

test/test_art_oom.cpp (1)

8-8: LGTM! Consistent IWYU pattern.

Suppresses internal <__new/exceptions.h> while keeping the public <new> header. The reordering of gtest after standard library headers is reasonable.

Also applies to: 13-15

test/qsbr_test_utils.cpp (1)

6-7: LGTM! Pragma-only adjustment.

No include changes, just IWYU guidance adjustments.

test/test_qsbr.cpp (1)

9-11: LGTM! Adds missing include.

The <cmath> include was added (not just reordered), which is correct since the file uses std::isnan at line 877.

test/test_art_key_view.cpp (1)

6-7: LGTM! Proper IWYU annotations.

Suppresses internal headers while keeping <cstddef> which provides std::byte used at line 41.

Also applies to: 12-12

test/test_art.cpp (1)

6-10: LGTM! Standard IWYU pattern.

Suppresses internal and unused headers while keeping <cstddef> for std::byte usage.

test/test_art_concurrency.cpp (1)

4-4: LGTM! Include reordering.

Removes the IWYU keep pragma from global.hpp (consistent with other files) and adjusts qsbr.hpp position.

Also applies to: 22-22

test/db_test_utils.cpp (1)

4-6: LGTM! Proper header ordering.

Moves db_test_utils.hpp to be included immediately after global.hpp with an IWYU keep pragma, following the common pattern of including a source file's corresponding header first.

Comment thread fuzz_deepstate/test_art_fuzz_deepstate.cpp Outdated
Comment thread mutex_art.hpp
Comment thread test/test_key_encode_decode.cpp Outdated
@laurynas-biveinis laurynas-biveinis force-pushed the iwyu-0.25 branch 2 times, most recently from c010294 to b2cfd29 Compare November 4, 2025 09:13
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c010294 and b2cfd29.

📒 Files selected for processing (49)
  • art.hpp (2 hunks)
  • art_common.hpp (1 hunks)
  • art_internal.cpp (1 hunks)
  • art_internal.hpp (1 hunks)
  • art_internal_impl.hpp (1 hunks)
  • benchmark/micro_benchmark.cpp (1 hunks)
  • benchmark/micro_benchmark_concurrency.hpp (1 hunks)
  • benchmark/micro_benchmark_key_prefix.cpp (1 hunks)
  • benchmark/micro_benchmark_mutex.cpp (0 hunks)
  • benchmark/micro_benchmark_n16.cpp (0 hunks)
  • benchmark/micro_benchmark_n256.cpp (0 hunks)
  • benchmark/micro_benchmark_n4.cpp (1 hunks)
  • benchmark/micro_benchmark_n48.cpp (0 hunks)
  • benchmark/micro_benchmark_node_utils.hpp (1 hunks)
  • benchmark/micro_benchmark_utils.cpp (1 hunks)
  • benchmark/micro_benchmark_utils.hpp (1 hunks)
  • examples/example_art.cpp (1 hunks)
  • examples/example_art_stats.cpp (1 hunks)
  • examples/example_olc_art.cpp (1 hunks)
  • fuzz_deepstate/test_art_fuzz_deepstate.cpp (1 hunks)
  • heap.hpp (1 hunks)
  • in_fake_critical_section.hpp (1 hunks)
  • mutex_art.hpp (1 hunks)
  • olc_art.hpp (1 hunks)
  • optimistic_lock.hpp (1 hunks)
  • portability_builtins.hpp (1 hunks)
  • qsbr.cpp (2 hunks)
  • qsbr.hpp (2 hunks)
  • qsbr_ptr.hpp (1 hunks)
  • test/db_test_utils.cpp (1 hunks)
  • test/db_test_utils.hpp (2 hunks)
  • test/gtest_utils.hpp (1 hunks)
  • test/qsbr_gtest_utils.cpp (1 hunks)
  • test/qsbr_gtest_utils.hpp (1 hunks)
  • test/qsbr_test_utils.cpp (1 hunks)
  • test/test_art.cpp (1 hunks)
  • test/test_art_concurrency.cpp (2 hunks)
  • test/test_art_iter.cpp (0 hunks)
  • test/test_art_key_view.cpp (1 hunks)
  • test/test_art_oom.cpp (1 hunks)
  • test/test_art_scan.cpp (2 hunks)
  • test/test_key_encode_decode.cpp (2 hunks)
  • test/test_qsbr.cpp (1 hunks)
  • test/test_qsbr_oom.cpp (1 hunks)
  • test/test_qsbr_ptr.cpp (1 hunks)
  • test/test_utils.hpp (1 hunks)
  • test_heap.cpp (2 hunks)
  • test_heap.hpp (1 hunks)
  • thread_sync.hpp (1 hunks)
💤 Files with no reviewable changes (5)
  • benchmark/micro_benchmark_n48.cpp
  • test/test_art_iter.cpp
  • benchmark/micro_benchmark_n256.cpp
  • benchmark/micro_benchmark_n16.cpp
  • benchmark/micro_benchmark_mutex.cpp
🧰 Additional context used
🧬 Code graph analysis (1)
benchmark/micro_benchmark_node_utils.hpp (3)
optimistic_lock.hpp (3)
  • NDEBUG (403-414)
  • NDEBUG (822-826)
  • NDEBUG (829-835)
qsbr.hpp (1)
  • NDEBUG (1065-1081)
qsbr_ptr.hpp (2)
  • NDEBUG (154-163)
  • NDEBUG (176-185)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (40)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 12 Debug with ASan, Debug, ON, clang, 12)
  • GitHub Check: build (clang 11 Debug with UBSan, Debug, ON, clang, 11)
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 12 Debug with ASan, Debug, ON, clang, 12)
  • GitHub Check: build (clang 11 Debug with UBSan, Debug, ON, clang, 11)
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 12 Debug with ASan, Debug, ON, clang, 12)
  • GitHub Check: build (clang 11 Debug with UBSan, Debug, ON, clang, 11)
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 12 Debug with ASan, Debug, ON, clang, 12)
  • GitHub Check: build (clang 11 Debug with UBSan, Debug, ON, clang, 11)
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 12 Debug with ASan, Debug, ON, clang, 12)
  • GitHub Check: build (clang 11 Debug with UBSan, Debug, ON, clang, 11)
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 12 Debug with ASan, Debug, ON, clang, 12)
  • GitHub Check: build (clang 11 Debug with UBSan, Debug, ON, clang, 11)
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 12 Debug with ASan, Debug, ON, clang, 12)
  • GitHub Check: build (clang 11 Debug with UBSan, Debug, ON, clang, 11)
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 12 Debug with ASan, Debug, ON, clang, 12)
  • GitHub Check: build (clang 11 Debug with UBSan, Debug, ON, clang, 11)
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 12 Debug with ASan, Debug, ON, clang, 12)
  • GitHub Check: build (clang 11 Debug with UBSan, Debug, ON, clang, 11)
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build (clang 12 Debug with ASan, Debug, ON, clang, 12)
  • GitHub Check: build (clang 11 Debug with UBSan, Debug, ON, clang, 11)
  • GitHub Check: build (clang 11 Release, Release, clang, 11)
🔇 Additional comments (46)
test/qsbr_test_utils.cpp (1)

6-7: Pragma cleanup looks fine.

The reordering and duplicate removal are reasonable. If IWYU 0.25 actually requires this specific format, then you're good. If not, it's harmless cleanup anyway.

benchmark/micro_benchmark_node_utils.hpp (2)

6-6: LGTM - IWYU pragma removed.

Removal aligns with IWYU 0.25 compatibility.


18-20: Good optimization - iostream only needed for debug builds.

The move is correct since std::cerr (line 455) is only used within NDEBUG blocks.

examples/example_art_stats.cpp (1)

7-7: LGTM - IWYU pragma cleanup.

The removal of IWYU directives is consistent with upgrading to include-what-you-use 0.25.

thread_sync.hpp (1)

9-9: LGTM - IWYU pragma removed.

Correct cleanup for IWYU 0.25 compatibility.

test/test_utils.hpp (1)

13-13: LGTM - IWYU pragma cleanup.

Consistent with the broader IWYU 0.25 migration.

test/gtest_utils.hpp (1)

15-15: LGTM - IWYU pragma removed.

Correct adaptation to IWYU 0.25.

portability_builtins.hpp (1)

11-11: LGTM - IWYU pragma cleanup.

Consistent with IWYU 0.25 requirements.

in_fake_critical_section.hpp (1)

20-20: LGTM - IWYU pragma removed.

Correct cleanup for IWYU 0.25.

art_common.hpp (2)

6-6: LGTM - IWYU pragma cleanup.

Correct removal of IWYU pragma: keep for IWYU 0.25.


12-12: LGTM - IWYU pragma cleanup.

Simplified iosfwd include by removing obsolete IWYU directives.

examples/example_olc_art.cpp (1)

7-7: LGTM - IWYU pragma cleanup.

Correct removal of multiple IWYU directives for IWYU 0.25 compatibility.

heap.hpp (1)

9-9: LGTM - IWYU pragma cleanup

Removal of the keep pragma is consistent with IWYU 0.25's improved include analysis.

benchmark/micro_benchmark_key_prefix.cpp (1)

6-10: LGTM - Proper vector header management

Excluding the implementation detail header <__vector/vector.h> while keeping the public <vector> header is the correct approach.

test/test_art_scan.cpp (1)

7-16: LGTM - Consistent vector header handling

Same pattern as the benchmark file - correctly managing the vector header includes.

mutex_art.hpp (1)

6-6: LGTM - IWYU pragma cleanup

Consistent with the pattern across the PR. The previous review concern about <type_traits> has been addressed (line 10).

optimistic_lock.hpp (1)

214-214: LGTM - IWYU pragma cleanup

Consistent pragma removal. The AI summary indicates <iterator> and <optional> includes were also removed, which is appropriate if they're unused.

qsbr.cpp (1)

12-24: LGTM - Proper exception header management

Correctly excludes the implementation header while keeping the public <new> header needed for std::bad_alloc (line 56).

art_internal.hpp (1)

6-8: LGTM - Proper cstddef header management

Excluding the implementation detail header is correct since the file already includes the public <cstddef> header (line 13) which provides std::byte.

test_heap.hpp (1)

15-23: LGTM - Exception header management

Same pattern as qsbr.cpp - correctly managing the <new> header includes for std::bad_alloc usage (line 75).

test/test_key_encode_decode.cpp (1)

4-28: LGTM! IWYU pragma adjustments are appropriate.

The IWYU pragmas have been correctly updated. The conditional inclusion of <iostream> within the #ifndef NDEBUG block (lines 26-28) properly reflects that std::cerr is only used in debug-only code paths.

art.hpp (3)

6-6: LGTM! Simplified IWYU pragma is appropriate.

Removing the IWYU pragma: keep from the first include aligns with the project-wide IWYU hygiene improvements.


27-53: LGTM! Forward declarations and inode_defs alias are correctly defined.

The forward declarations for all inode specializations and the inode_defs alias correctly bundle the five inode types for use in the ART policy.


718-802: LGTM! Test types and size assertions add valuable compile-time validation.

The test type aliases and static_asserts for inode sizes provide compile-time verification of memory layout stability across platforms. The platform-specific size differences (MSVC padding, AVX2 alignment) are appropriately documented and conditionally checked.

benchmark/micro_benchmark_n4.cpp (1)

7-12: LGTM! IWYU pragmas correctly guide include analysis.

The no_include for the internal <__vector/vector.h> implementation header and the keep for the public <vector> header are appropriate IWYU guidance.

test/test_qsbr_ptr.cpp (1)

4-4: LGTM! Simplified IWYU pragma is consistent with project-wide changes.

Removing the IWYU pragma: keep from the first include aligns with the PR's IWYU hygiene improvements.

benchmark/micro_benchmark_utils.cpp (1)

6-12: LGTM! IWYU keep pragmas prevent unintended header removal.

The added IWYU pragma: keep comments ensure these headers aren't removed by automated tools, even if their usage isn't immediately apparent to static analysis.

test/qsbr_gtest_utils.cpp (1)

4-10: LGTM! IWYU pragma adjustments are consistent with project patterns.

Moving the IWYU pragma: keep from global.hpp to qsbr_gtest_utils.hpp aligns with the project-wide pattern of ensuring header/implementation file pairs keep their corresponding header includes.

fuzz_deepstate/test_art_fuzz_deepstate.cpp (1)

5-22: No action required—transitive includes are validated.

Your transitive includes are working correctly. The grep results confirm that:

  • std::optional comes from art.hpp (includes <optional>)
  • std::numeric_limits comes from art_internal_impl.hpp (includes <limits>)
  • std::stringstream is available through the included headers

The IWYU pragma validates this strategy. The original concern is resolved.

benchmark/micro_benchmark_utils.hpp (1)

6-12: LGTM - IWYU 0.25 hygiene updates.

The changes follow the consistent pattern across this PR: removing the keep pragma from the first include (global.hpp), adding no_include for internal implementation headers, and moving keep annotations inline where needed. No functional impact.

benchmark/micro_benchmark.cpp (1)

6-12: LGTM - IWYU-driven include adjustments.

Pragmas updated for IWYU 0.25, and the shift from <array> to <tuple> is correct since the code uses std::ignore (line 149) which requires <tuple>. The forward declaration removal aligns with cleaner IWYU practices.

test/db_test_utils.cpp (1)

4-6: LGTM - Include reordering with proper pragma placement.

The reordering maintains global.hpp as the first include while properly marking db_test_utils.hpp with pragma: keep for its implementation file. Consistent with IWYU 0.25 hygiene.

benchmark/micro_benchmark_concurrency.hpp (1)

6-6: LGTM - Consistent IWYU pragma cleanup.

Removing the keep pragma from the first include is part of the consistent IWYU 0.25 hygiene pattern applied across the codebase.

qsbr.hpp (1)

25-46: LGTM - Standard IWYU pattern applied.

The changes suppress the internal implementation header <__vector/vector.h> while explicitly keeping the public <vector> header. This is the correct IWYU 0.25 pattern for handling transitive includes.

test/test_qsbr.cpp (1)

9-12: LGTM - Required include added.

Adding <cmath> is correct since std::isnan is used at line 877. The no_include pragma for <gtest/gtest.h> is appropriate as it's transitively available through gtest_utils.hpp at line 16.

test/db_test_utils.hpp (1)

6-26: LGTM - Comprehensive IWYU pragma updates.

All changes follow the established pattern: removing keep from global.hpp, suppressing internal implementation headers (__cstddef/byte.h, __vector/vector.h), and explicitly keeping the public <vector> header. Correctly applied.

test/test_art_concurrency.cpp (1)

4-22: LGTM - Include ordering refinement.

The pragma removal is consistent with the broader pattern. Moving qsbr.hpp to line 22 (after gtest_utils.hpp) aligns with the revised include sequencing described in the PR. No functional impact.

test/test_art_key_view.cpp (1)

6-7: LGTM - IWYU pragma updates are appropriate.

These directives control IWYU 0.25's header analysis without affecting functionality. The no_include pragmas suppress suggestions for internal/unused headers, and the keep pragma on <cstddef> preserves a header likely needed for std::byte.

Also applies to: 12-12

test/test_art.cpp (1)

6-8: LGTM - IWYU pragma updates are consistent.

Header hygiene directives for IWYU 0.25 compatibility. The suppressions prevent unnecessary header suggestions, and the keep ensures <cstddef> isn't removed despite indirect usage.

Also applies to: 10-10

examples/example_art.cpp (1)

7-7: Include cleanup looks good

global.hpp remains first; dropping IWYU keep is fine here.

test_heap.cpp (1)

19-20: Pragmas are appropriate for IWYU 0.25

  • no_include on internal <__new/new_handler.h> avoids spurious adds.
  • keep on prevents removal while using std::get_new_handler/bad_alloc.

Also applies to: 30-30

test/test_art_oom.cpp (1)

8-8: IWYU hints are consistent for OOM tests

no_include for __new/exceptions.h and keep on make sense; gtest include order is fine.

Also applies to: 13-16

qsbr_ptr.hpp (1)

14-17: Good IWYU hygiene for std::byte

Dropping keep on global.hpp and suppressing libc++’s __cstddef/byte.h while keeping is correct.

art_internal_impl.hpp (1)

6-6: LGTM on include pragma removal

global.hpp stays first; no functional impact.

olc_art.hpp (1)

9-9: Include pragma removal is fine

global.hpp remains first; rest untouched.

art_internal.cpp (1)

4-4: LGTM on IWYU cleanup

Plain include is fine; existing no_include/keep pragmas remain correctly placed.

Comment thread test/qsbr_gtest_utils.hpp
Comment thread test/test_qsbr_oom.cpp
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Nov 4, 2025

@laurynas-biveinis laurynas-biveinis merged commit 2dd6d29 into master Nov 4, 2025
213 of 219 checks passed
@laurynas-biveinis laurynas-biveinis deleted the iwyu-0.25 branch November 4, 2025 15:15
@coderabbitai coderabbitai Bot mentioned this pull request Nov 25, 2025
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.

1 participant