Skip to content

Addressing code review comments about boost::coroutine2#6977

Draft
pratikmankawde wants to merge 2 commits intodevelopfrom
pratik/fix-coroutine2-review-comments
Draft

Addressing code review comments about boost::coroutine2#6977
pratikmankawde wants to merge 2 commits intodevelopfrom
pratik/fix-coroutine2-review-comments

Conversation

@pratikmankawde
Copy link
Copy Markdown
Contributor

@pratikmankawde pratikmankawde commented Apr 20, 2026

Details to come.

High Level Overview of Change

Context of Change

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
@pratikmankawde pratikmankawde added the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Apr 20, 2026
Comment thread conan/profiles/sanitizers
boost/*:without_context=False
# Boost stacktrace fails to build with some sanitizers
boost/*:without_stacktrace=True
{% elif "thread" in sanitizers %}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TSAN is inactive at the moment, so this is just to keep the profile consistent. This is a dead branch at the moment.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.8%. Comparing base (852fbe9) to head (cd3aa51).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #6977     +/-   ##
=========================================
+ Coverage     81.6%   81.8%   +0.2%     
=========================================
  Files         1010    1010             
  Lines        75992   76090     +98     
  Branches      7605    7530     -75     
=========================================
+ Hits         62002   62229    +227     
+ Misses       13990   13861    -129     
Files with missing lines Coverage Δ
include/xrpl/core/Coro.ipp 100.0% <100.0%> (ø)

... and 11 files with indirect coverage changes

Impacted file tree graph

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the codebase and build configuration to align with current Boost.Coroutine2 APIs and sanitizer-related Boost.Context build requirements.

Changes:

  • Replace deprecated boost::coroutines2::asymmetric_coroutine usage with boost::coroutines2::coroutine in JobQueue::Coro.
  • Centralize coroutine stack size into a named constant for clarity and reuse.
  • Adjust build/config tooling: remove BOOST_COROUTINES2_NO_DEPRECATION_WARNING, remove an ASAN-specific Conan fPIC override, and add Boost.Context ucontext configuration for TSAN in the sanitizer profile.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
include/xrpl/core/Coro.ipp Switch coroutine parameter type to boost::coroutines2::coroutine<void>::push_type and introduce a named stack-size constant.
conanfile.py Removes ASAN-specific fPIC override (but leaves an inert SANITIZERS env check).
conan/profiles/sanitizers Add TSAN-specific Boost.Context ucontext build flags for correct sanitizer annotations.
cmake/XrplInterface.cmake Removes BOOST_COROUTINES2_NO_DEPRECATION_WARNING compile definition.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread conanfile.py Outdated
Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DraftRunCI Normally CI does not run on draft PRs. This opts in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants