Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
2010d15 to
948912f
Compare
948912f to
aa49a11
Compare
ffc365c to
03c498b
Compare
d1adcf0 to
e91cd5d
Compare
e91cd5d to
d493533
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/batteries/slab_stack.cxx`:
- Around line 109-114: The push() routine currently does pointer arithmetic
diff_type free_bytes = node_size * (m_hi - m_sp) which is UB if init_slab failed
in release() leaving m_ctrl/m_lo/m_sp/m_hi == nullptr; modify push() to first
check that m_hi and m_sp are non-null (e.g., if (m_hi == nullptr || m_sp ==
nullptr) throw std::bad_alloc{} or handle as out-of-space) before performing
m_hi - m_sp, and apply the same null-safety check in the analogous location
around lines 158-163; also update the comment in release() to note that pointers
are cleared and that callers must handle null pointer state so subsequent
pointer arithmetic is guarded.
In `@test/src/stack.cpp`:
- Around line 129-137: Wrap tests that depend on exceptions with conditional
compilation and avoid catching std::bad_alloc for stacks that allocate
dynamically: enclose the entire "slab_stack - throws when full" test in `#if`
LF_COMPILER_EXCEPTIONS ... `#endif`, and in the TEMPLATE_TEST_CASEs that iterate
over geometric_stack, adaptor_stack and slab_stack, remove or guard the
try/catch(std::bad_alloc) blocks with `#if` LF_COMPILER_EXCEPTIONS so only
slab_stack is allowed to trigger and catch bad_alloc; alternatively use
type-specific branches (e.g., if constexpr or trait for fixed_capacity_stack) to
only perform exception-based early-exit logic for slab_stack and use standard
allocation failure checks for geometric_stack/adaptor_stack.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d47dbc6-0c30-4623-a7d3-0840622eb4b7
📒 Files selected for processing (6)
CMakeLists.txtbenchmark/src/libfork_benchmark/fib/libfork.cppsrc/batteries/batteries.cxxsrc/batteries/geometric_stack.cxxsrc/batteries/slab_stack.cxxtest/src/stack.cpp
74d1538 to
4bb14f8
Compare
Summary by CodeRabbit
New Features
slab_stack, a fixed-size, single-slab user-space stack implementation for memory allocation.slab_stackvariants.Bug Fixes
geometric_stackmethod signature.Tests
slab_stackacross multiple scenarios, including allocation limits and checkpoint/acquire/release cycles.