Skip to content

7895: replace some futures with poll_fn#57

Open
martin-augment wants to merge 1 commit intomasterfrom
pr-7895-2026-02-09-06-23-43
Open

7895: replace some futures with poll_fn#57
martin-augment wants to merge 1 commit intomasterfrom
pr-7895-2026-02-09-06-23-43

Conversation

@martin-augment
Copy link
Owner

7895: To review by AI

@gemini-code-assist
Copy link

Summary of Changes

Hello @martin-augment, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors several asynchronous functions across the tokio-util and tokio crates to leverage the std::future::poll_fn helper. This change streamlines the implementation of futures by replacing verbose manual Future trait implementations with a more concise and idiomatic approach, ultimately improving code readability and maintainability without altering external behavior.

Highlights

  • Refactoring to poll_fn: Replaced manual Future implementations with std::future::poll_fn in several asynchronous functions, leading to more concise code.
  • Code Simplification: Reduced boilerplate by removing anonymous struct definitions that previously implemented the Future trait directly.
  • Improved Readability: The adoption of poll_fn makes the creation of futures from polling logic more explicit and easier to understand.
Changelog
  • tokio-util/src/io/read_buf.rs
    • Replaced manual Future implementation with std::future::poll_fn in the read_buf function.
    • Removed unused std::future::Future and std::task::{Context, Poll} imports.
  • tokio/src/lib.rs
    • Refactored async_trace_leaf to use std::future::poll_fn instead of a custom Future struct.
    • Updated imports in the trace module to reflect the change.
  • tokio/src/task/yield_now.rs
    • Rewrote the yield_now function to utilize std::future::poll_fn for its asynchronous logic.
    • Removed unnecessary std::future::Future and std::pin::Pin imports.
Activity
  • The pull request description indicates it is 'To review by AI', but no specific review comments or activity have been recorded yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link

coderabbitai bot commented Feb 9, 2026

Walkthrough

This pull request refactors multiple Future implementations across the tokio ecosystem. Three modules replace manual Future structures with simpler polling patterns: read_buf in tokio-util removes a ReadBufFn wrapper using poll_fn, async_trace_leaf in tokio converts to an async function, and yield_now replaces YieldNow struct with poll_fn-based polling using a local boolean variable. The changes consolidate asynchronous polling logic while maintaining the same underlying behavior and preserving public API surfaces.

✨ 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 pr-7895-2026-02-09-06-23-43

No actionable comments were generated in the recent review. 🎉


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.

@augmentcode
Copy link

augmentcode bot commented Feb 9, 2026

🤖 Augment PR Summary

Summary: Replaces several small custom Future implementations with std::future::poll_fn to reduce boilerplate.

Changes:

  • Simplified tokio-util::io::read_buf to await a poll_fn wrapper around poll_read_buf
  • Converted internal tracing helper async_trace_leaf to an async fn that awaits poll_fn(trace_leaf)
  • Reworked task::yield_now to use a poll_fn-based state machine instead of a dedicated Future struct

Technical Notes: Keeps behavior the same while making the poll-based flow more idiomatic and localized.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a great simplification, correctly refactoring several manual Future implementations to use the more concise and idiomatic std::future::poll_fn. This modern approach improves code readability and maintainability. The changes in tokio-util/src/io/read_buf.rs, tokio/src/lib.rs, and tokio/src/task/yield_now.rs are all well-executed, preserving the original logic while making the code cleaner. For instance, async_trace_leaf is now a proper async fn, and the stateful logic in yield_now is neatly handled by a closure. These are excellent changes with no issues found.

@claude
Copy link

claude bot commented Feb 9, 2026

Pull Request Review: Replace some futures with poll_fn

Summary

This PR refactors three hand-written Future implementations to use the standard library's poll_fn helper instead. Overall, this is a clean, well-executed refactoring that improves code maintainability.

Code Quality ✅

Strengths:

  • Reduces boilerplate: Eliminates 58 lines of code while maintaining identical functionality
  • Improves readability: poll_fn is more idiomatic and easier to understand than custom Future implementations
  • Consistent with codebase: This pattern is already widely used in tokio (20+ files use poll_fn)
  • Maintains semantics: All three changes preserve the exact same polling behavior

Specific Changes:

1. tokio-util/src/io/read_buf.rs

  • Replaced ReadBufFn struct with inline poll_fn
  • Clean conversion, behavior preserved
  • The closure correctly captures read and buf by reference

2. tokio/src/lib.rs - async_trace_leaf()

  • Converted from returning impl Future to async fn
  • More idiomatic Rust - async functions are preferred over impl Future returns
  • Simpler and more readable

3. tokio/src/task/yield_now.rs

  • Replaced YieldNow struct with inline poll_fn
  • State (yielded) moved from struct field to closure capture
  • Behavior preserved: still polls twice (once returning Pending, once returning Ready)

Potential Issues 🔍

Minor: Closure Capture Semantics

In tokio-util/src/io/read_buf.rs:48, the closure captures read and buf by reference. While this is correct, it's worth noting that:

  • Pin::new(read) is called on each poll - this is safe because read is &mut R where R: Unpin
  • The closure borrows the function parameters for the duration of the future

Verdict: This is correct, but worth keeping in mind for lifetime reasoning.

Minor: Code Formatting

In tokio/src/task/yield_now.rs:40-53, the poll_fn closure could benefit from slight reformatting for consistency:

poll_fn(|cx| {
    ready!(crate::trace::trace_leaf(cx));
    
    if yielded {
        return Poll::Ready(());
    }
    
    yielded = true;
    context::defer(cx.waker());
    
    Poll::Pending
})
.await

The current formatting is fine, but adding blank lines between logical sections might improve readability.

Performance Considerations ✅

No performance impact expected:

  • poll_fn is a zero-cost abstraction (it's marked #[inline] in std)
  • Generated code should be identical after optimization
  • No additional allocations or indirection introduced

Security Concerns ✅

No security issues identified. The changes are purely structural refactoring.

Test Coverage ⚠️

Observation: The PR doesn't include new tests, but this is expected for a refactoring PR. However:

  • Recommendation: Run the full test suite to ensure no behavioral changes
  • The existing tests should provide adequate coverage since the behavior is unchanged
  • Consider running: cargo test --workspace and cargo test --all-features

Best Practices Assessment ✅

This PR follows Tokio best practices:

  • ✅ Reduces custom Future implementations in favor of combinators
  • ✅ Uses idiomatic Rust patterns
  • ✅ Maintains backward compatibility (no public API changes)
  • ✅ Preserves performance characteristics
  • ✅ Follows the existing codebase patterns

Recommendations

  1. Before merging: Run the full test suite, including:

    • cargo test --workspace
    • cargo test --all-features
    • Any CI checks the project normally runs
  2. Optional improvements:

    • Consider adding a comment explaining why yielded must be false initially in yield_now()
    • Minor formatting adjustments as noted above
  3. PR description: The description "7895: To review by AI" could be more detailed. Consider adding:

    • Motivation: Why replace these futures? (reduced boilerplate, improved readability)
    • Verification: How was behavioral equivalence verified?

Final Verdict

Approve with minor recommendations

This is a solid refactoring that improves code quality without introducing risks. The changes are semantically equivalent to the original code and follow established patterns in the codebase. Once the test suite passes, this is ready to merge.


Review generated by Claude Code - Please verify all recommendations align with project standards

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.

2 participants