[V4] Schedule and work handles

[ConorWilliams]

Summary by CodeRabbit

• New Features

  • Added new scheduling and handle management modules to core library exports.

• Improvements

  • Updated minimum CMake version to 4.3.
  • Enhanced hardened build configuration with additional compiler safety flags.
  • Refined context and allocator interfaces across the library.

• Documentation

  • Normalized and corrected documentation formatting and troubleshooting content.

• Tests

  • Updated test context implementations to align with revised interfaces.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a7bdf931-033a-4c9a-b4ce-ab6a00756a35

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch v4-handles

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (4)

CMakePresets.json (1)

35-35: Document the rationale for -Wno-c2y-extensions.

Adding warning suppressions to the hardened build should be documented (e.g., in a comment or commit message) explaining which code triggers this warning and why suppression is appropriate. This helps future maintainers understand the tradeoff.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@CMakePresets.json` at line 35, Document why the -Wno-c2y-extensions flag is
being added to CMAKE_CXX_FLAGS: update the comment near the CMAKE_CXX_FLAGS
entry (referencing CMAKE_CXX_FLAGS and the -Wno-c2y-extensions token) or the
related commit message to state which source file(s) or pattern trigger the c2y
extension warnings, why those warnings are benign or impractical to fix in the
short term, and any planned remediation or risk acceptance; include the minimal
repro (file or construct) and the rationale so future maintainers can assess and
revert the suppression if needed.

src/core/handles.cxx (1)

27-39: Consider unifying frame_handle and await_handle implementation.

Both handle types have identical structure (pointer wrapper with lock-guarded construction, defaulted equality, explicit bool). If they are semantically distinct but structurally identical, this is fine. However, if their behavior may diverge in the future (as hinted by the TODOs), keeping them separate is reasonable.

The Cppcheck "syntax error" at line 27 is a false positive—Cppcheck 2.20.0 has limited C++20 module support.

Also applies to: 47-59

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/handles.cxx` around lines 27 - 39, The two nearly identical types
frame_handle and await_handle should be unified into a single generic handle
template to avoid duplication while preserving semantic distinction: create a
template like handle<T, Tag> (or a base template handle<T> with an empty Tag
parameter) that implements the lock-guarded constructor (lock, pointer to
frame_type<T>/await_type<T>), defaulted operator==, and explicit operator bool,
then typedef or using-alias frame_handle<T> and await_handle<T> to the generic
handle with distinct tag types; update references to frame_handle and
await_handle to use the new alias while keeping the same constructors and member
semantics.

src/core/schedule.cxx (1)

24-24: Stale TODO comment.

The TODO mentions "void specialization of block" but block<void> is already implemented at lines 30-31. Consider removing this comment.

Proposed fix

-// TODO: void specialization of block
-
 template <typename T>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/schedule.cxx` at line 24, The TODO comment "// TODO: void
specialization of block" is stale because block<void> is already implemented
(see the block<void> specialization in schedule.cxx); remove that TODO comment
to avoid confusion and keep code comments accurate, ensuring no other references
to a void-specialization task remain in the file or nearby comments.

src/core/promise.cxx (1)

394-396: TODO noted: Context type constraint needed.

The comment correctly identifies that child tasks should be constrained to have the same context type as the parent. This prevents subtle bugs from mismatched context types. Consider adding this constraint in a follow-up.

Would you like me to draft the constraint for transform() and schedule() that enforces matching context types?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/promise.cxx` around lines 394 - 396, Add a compile-time constraint
so child tasks must use the same context type as their parent: update the
template declarations for transform() and schedule() (the functions referenced
by the TODO) to require that the child's context type is identical to the
parent's context type (using a C++20 concept or std::enable_if/std::is_same_v)
and place the check both where the TODO lives and in schedule(...) to prevent
mismatched instantiations; ensure the constraint targets the child task/context
template parameter (e.g., ChildContext) and compares it to the parent context
type (ParentContext) so any compile attempt with differing context types fails
with a clear message.

🤖 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/core/schedule.cxx`:
- Around line 41-42: The schedule function is marked noexcept but performs
operations that can throw (allocation with new block<result_type>{} and
invocation via std::invoke of fn/args...), which would call std::terminate;
remove the noexcept from the schedule declaration (export template
<worker_context Context, typename... Args, async_invocable<Args...> Fn>
constexpr auto schedule(...)) until you implement proper exception handling, or
alternatively wrap the allocation and task construction/invocation (new
block<result_type>{} and the code around std::invoke(...)) in a try/catch that
cleans up the allocated block and rethrows the exception; choose one approach
and apply it consistently so schedule no longer causes unexpected termination.

---

Nitpick comments:
In `@CMakePresets.json`:
- Line 35: Document why the -Wno-c2y-extensions flag is being added to
CMAKE_CXX_FLAGS: update the comment near the CMAKE_CXX_FLAGS entry (referencing
CMAKE_CXX_FLAGS and the -Wno-c2y-extensions token) or the related commit message
to state which source file(s) or pattern trigger the c2y extension warnings, why
those warnings are benign or impractical to fix in the short term, and any
planned remediation or risk acceptance; include the minimal repro (file or
construct) and the rationale so future maintainers can assess and revert the
suppression if needed.

In `@src/core/handles.cxx`:
- Around line 27-39: The two nearly identical types frame_handle and
await_handle should be unified into a single generic handle template to avoid
duplication while preserving semantic distinction: create a template like
handle<T, Tag> (or a base template handle<T> with an empty Tag parameter) that
implements the lock-guarded constructor (lock, pointer to
frame_type<T>/await_type<T>), defaulted operator==, and explicit operator bool,
then typedef or using-alias frame_handle<T> and await_handle<T> to the generic
handle with distinct tag types; update references to frame_handle and
await_handle to use the new alias while keeping the same constructors and member
semantics.

In `@src/core/promise.cxx`:
- Around line 394-396: Add a compile-time constraint so child tasks must use the
same context type as their parent: update the template declarations for
transform() and schedule() (the functions referenced by the TODO) to require
that the child's context type is identical to the parent's context type (using a
C++20 concept or std::enable_if/std::is_same_v) and place the check both where
the TODO lives and in schedule(...) to prevent mismatched instantiations; ensure
the constraint targets the child task/context template parameter (e.g.,
ChildContext) and compares it to the parent context type (ParentContext) so any
compile attempt with differing context types fails with a clear message.

In `@src/core/schedule.cxx`:
- Line 24: The TODO comment "// TODO: void specialization of block" is stale
because block<void> is already implemented (see the block<void> specialization
in schedule.cxx); remove that TODO comment to avoid confusion and keep code
comments accurate, ensuring no other references to a void-specialization task
remain in the file or nearby comments.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b7c542d8-d42a-466f-bdf0-108c0b45a466

📥 Commits

Reviewing files that changed from the base of the PR and between 9f23b15 and 60685c2.

📒 Files selected for processing (15)

• AGENTS.md
• CMakeLists.txt
• CMakePresets.json
• benchmark/src/libfork_benchmark/fib/lf_parts.cpp
• include/libfork/__impl/assume.hpp
• src/core/concepts.cxx
• src/core/constants.cxx
• src/core/context.cxx
• src/core/core.cxx
• src/core/frame.cxx
• src/core/handles.cxx
• src/core/ops.cxx
• src/core/promise.cxx
• src/core/schedule.cxx
• test/src/promise.cpp

💤 Files with no reviewable changes (1)

• src/core/constants.cxx

[coderabbitai]

⚠️ Potential issue | 🟠 Major

noexcept specification may cause std::terminate on allocation or invocation failure.

The function is marked noexcept, but:

• Line 47: new block<result_type>{} can throw std::bad_alloc
• Line 54: std::invoke(...) can throw if fn or args... throw during task construction

The TODO at line 49 acknowledges this ("clean up block if exception"). Until exception handling is implemented, consider either removing noexcept or documenting that allocation/invocation failures are considered fatal.

🧰 Tools

🪛 Cppcheck (2.20.0)

[error] 41-41: syntax error

(syntaxError)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/schedule.cxx` around lines 41 - 42, The schedule function is marked
noexcept but performs operations that can throw (allocation with new
block<result_type>{} and invocation via std::invoke of fn/args...), which would
call std::terminate; remove the noexcept from the schedule declaration (export
template <worker_context Context, typename... Args, async_invocable<Args...> Fn>
constexpr auto schedule(...)) until you implement proper exception handling, or
alternatively wrap the allocation and task construction/invocation (new
block<result_type>{} and the code around std::invoke(...)) in a try/catch that
cleans up the allocated block and rethrows the exception; choose one approach
and apply it consistently so schedule no longer causes unexpected termination.