Skip to content

Add constexpr to basic_inode_4::key_union constructor#812

Merged
laurynas-biveinis merged 1 commit into
masterfrom
inode4-constexpr-union
Nov 26, 2025
Merged

Add constexpr to basic_inode_4::key_union constructor#812
laurynas-biveinis merged 1 commit into
masterfrom
inode4-constexpr-union

Conversation

@laurynas-biveinis
Copy link
Copy Markdown
Collaborator

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

Enables constexpr construction of the key_union type within basic_inode_4.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Summary by CodeRabbit

Release Notes

  • No user-visible changes
    • Internal compilation optimizations applied.

✏️ Tip: You can customize this high-level summary in your review settings.

Enables constexpr construction of the key_union type within basic_inode_4.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@laurynas-biveinis laurynas-biveinis self-assigned this Nov 25, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 25, 2025

Walkthrough

A single union constructor in basic_inode_4::key_union was changed from non-constexpr to constexpr, enabling compile-time initialization without altering runtime semantics.

Changes

Cohort / File(s) Summary
Union constructor constexprification
art_internal_impl.hpp
Changed default constructor of key_union from non-constexpr to constexpr, allowing constant initialization at compile time while maintaining identical runtime behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify the constexpr-qualified constructor is actually constant-evaluable (check member types and initialization logic)
  • Confirm no hidden side effects or runtime-only dependencies exist in the union's default initialization path

Poem

A constructor now constexpr,
Compile-time magic, compile-time quests,
Same runtime dance, new compile-time gains,
One small const that refines the chains ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 accurately describes the main change: adding constexpr to a specific constructor in basic_inode_4::key_union.
✨ 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 inode4-constexpr-union

📜 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 5eb438e and 351b86d.

📒 Files selected for processing (1)
  • art_internal_impl.hpp (1 hunks)
⏰ 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). (10)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
art_internal_impl.hpp (1)

1672-1674: constexpr on key_union ctor looks fine but needs toolchain verification

Making this default ctor constexpr is semantically neutral at runtime (the body is still empty and the union remains uninitialized until you explicitly write to it) and achieves the stated goal of allowing compile‑time construction of key_union.

The only real risk is build compatibility: for a constexpr union constructor to be accepted everywhere, key_union (and thus critical_section_policy<…>) must satisfy literal‑type rules on all your supported standards and compilers, especially MSVC, where you already gate some constexpr usage via UNODB_DETAIL_CONSTEXPR_NOT_MSVC. If any configuration rejects this, you may need to wrap this in that macro or drop constexpr there.

Please confirm this compiles cleanly across your supported toolchain matrix (GCC/Clang/MSVC, C++ standard levels you target).


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.

@sonarqubecloud
Copy link
Copy Markdown

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.07%. Comparing base (5eb438e) to head (351b86d).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #812      +/-   ##
==========================================
- Coverage   98.35%   98.07%   -0.28%     
==========================================
  Files          35       35              
  Lines        6493     6493              
==========================================
- Hits         6386     6368      -18     
- Misses        107      125      +18     
Flag Coverage Δ
Debug 98.07% <100.00%> (-0.14%) ⬇️
Release 97.30% <100.00%> (-0.48%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@laurynas-biveinis laurynas-biveinis merged commit 6caea31 into master Nov 26, 2025
229 of 231 checks passed
@laurynas-biveinis laurynas-biveinis deleted the inode4-constexpr-union branch November 26, 2025 03:42
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