[V4] Restore (optional) task context#78
Conversation
tmp tmp async invoke
|
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 |
60dabfa to
6b09eb6
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/core/concepts.cxx (1)
188-191: Consider value category coverage for noexcept detection.The concept uses
std::declval<Args>()...which produces rvalue references. If a callable has different noexcept specifications for lvalue vs rvalue arguments, this might not reflect all call site scenarios. This is likely acceptable for the library's use cases, but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/concepts.cxx` around lines 188 - 191, The concept noexcept_async_invocable currently uses std::declval<Args>()... which models rvalue arguments only and can miss different noexcept specifications for lvalue call sites; update the noexcept check to use lvalue references (e.g. std::declval<std::add_lvalue_reference_t<Args>>()...) or otherwise test both lvalue and rvalue categories so noexcept(async_invoke<Context, Args...>(std::declval<Fn>(), ...)) reflects the call-site value category; modify the noexcept expression in noexcept_async_invocable (and keep async_invocable/async_invoke names intact) to use the chosen reference form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/core/concepts.cxx`:
- Around line 188-191: The concept noexcept_async_invocable currently uses
std::declval<Args>()... which models rvalue arguments only and can miss
different noexcept specifications for lvalue call sites; update the noexcept
check to use lvalue references (e.g.
std::declval<std::add_lvalue_reference_t<Args>>()...) or otherwise test both
lvalue and rvalue categories so noexcept(async_invoke<Context,
Args...>(std::declval<Fn>(), ...)) reflects the call-site value category; modify
the noexcept expression in noexcept_async_invocable (and keep
async_invocable/async_invoke names intact) to use the chosen reference form.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 558a112f-eb94-4ad1-8048-f4f491601250
📒 Files selected for processing (4)
benchmark/src/libfork_benchmark/fib/lf_parts.cppsrc/core/concepts.cxxsrc/core/promise.cxxsrc/core/schedule.cxx
Summary by CodeRabbit