Skip to content

Pin the trivial default construction of unodb::db's inode child slots - #918

Merged
laurynas-biveinis merged 1 commit into
masterfrom
pin-trivial-inode-child-slots
Oct 7, 2026
Merged

laurynas-biveinis merged 1 commit into
masterfrom
pin-trivial-inode-child-slots

Conversation

@laurynas-biveinis

@laurynas-biveinis laurynas-biveinis commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

(LLM agent)

basic_node_ptr's default constructor leaves tagged_ptr uninitialized so that unodb::db's inode_4/16/256 constructors, which default-initialize children, cost no zero-fill. Nothing checked that. A default member initializer on tagged_ptr, or on in_fake_critical_section's value, would keep both constructors defaulted and pass every existing alias static_assert, yet make each of those constructions zero-fill its slots.

This adds a static_assert that in_fake_critical_section<node_ptr>, unodb::db's slot type, is trivially default-constructible. That holds only while node_ptr's constructor is trivial too, so the one assert fires on either initializer. unodb::olc_db needs no counterpart: C++20 std::atomic value-initializes its slots anyway.

Fixes #915.

Validation at this commit:

  • ./check.sh passed.
  • Full debug build (maintainer mode, IWYU, cpplint, cppcheck) passed with no warnings, and all 17 ctest suites passed.
  • The assert fires when {} is added to either basic_node_ptr::tagged_ptr or in_fake_critical_section's T value.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added a compile-time check to verify that inode children can be default-initialized without zero-filling.

basic_node_ptr's default constructor leaves tagged_ptr uninitialized so
that unodb::db's inode_4/16/256 constructors, which default-initialize
children, cost no zero-fill. Nothing checked that: a default member
initializer on tagged_ptr, or on in_fake_critical_section's value, keeps
both constructors defaulted and passes every alias static_assert, yet
makes each of those constructions zero-fill its slots.

Assert that in_fake_critical_section<node_ptr>, unodb::db's slot type, is
trivially default-constructible. That holds only while node_ptr's
constructor is trivial too, so one assert covers both initializers; it
fires on either. unodb::olc_db needs no counterpart: C++20 std::atomic
value-initializes its slots anyway.

Fixes #915.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: unodb-dev/unodb/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 5aa890f3-6f70-4268-9ada-9d8563cc67b3
📥 Commits

Reviewing files that changed from the base of the PR and between cfe347c and 3713182.

📒 Files selected for processing (1)
  • art.hpp

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The change adds a compile-time assertion that in_fake_critical_section<node_ptr> is trivially default-constructible. The assertion includes a diagnostic about default-initializing inode children without zero-fill.

Changes

Inode child slots

Layer / File(s) Summary
Default-construction assertion
art.hpp
Compilation fails with the supplied diagnostic if in_fake_critical_section<node_ptr> is not trivially default-constructible.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Suggested reviewers: thompsonbry

Merge Risk: ⚪ Minimal · up to 37131

The change adds a compile-time guard for the intended inode child-slot construction contract without changing runtime behavior. No actionable merge-blocking risk is evident.

Architecture Summary

Architecture risk: 🔵 Low · up to 37131

The change affects 1 system.

Changed systems: art.hpp

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — art.hpp (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in art.hpp: Adds a compile-time requirement that in_fake_critical_section<node_ptr> be trivially default-constructible; compilation fails with the supplied diagnostic if the requirement is not met.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the change: enforcing trivial default construction for unodb::db inode child slots.
Linked Issues check ✅ Passed Issue #915 requires a compile-time check that unodb::db’s inode child-slot type is trivially default-constructible. The PR adds that assertion for in_fake_critical_section<node_ptr> in art.hpp, co…
Out of Scope Changes check ✅ Passed The PR adds only the assertion in art.hpp. This change directly implements issue #915. The summary shows no unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A type is checked at compile time,
Its default form must stay in line.
If trivial construction fades away,
The diagnostic has its say.
No runtime path has changed today.

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

@codecov

codecov Bot commented Oct 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@laurynas-biveinis
laurynas-biveinis merged commit 89f5279 into master Oct 7, 2026
236 checks passed
@laurynas-biveinis
laurynas-biveinis deleted the pin-trivial-inode-child-slots branch October 7, 2026 03:42

This branch was successfully deployed

3 active deployments
claude — 3713182a Deployed Oct 6, 2026 by laurynas-biveinis via claude-review #433
coverage — 3713182a Deployed Oct 6, 2026 by laurynas-biveinis via build (Debug, Debug) #557
sonarcloud — 3713182a Deployed Oct 6, 2026 by laurynas-biveinis via SonarCloud #2661
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.

No static_assert pins the trivial default construction of unodb::db's inode child slots

1 participant