Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid clone in Comments::next #125055

Merged
merged 2 commits into from
May 13, 2024
Merged

Avoid clone in Comments::next #125055

merged 2 commits into from
May 13, 2024

Commits on May 13, 2024

  1. Make handling of Comments more iterator-like. 

    The current way of stepping through each comment in `Comments` is a bit
weird. There is a `Vec<Comments>` and a `current` index, which is fine.
The `Comments::next` method clones the current comment but doesn't
advance `current`; the advancing instead happens in `print_comment`,
which is where each cloned comment is actually finally used (or not, in
some cases, if the comment fails to satisfy a predicate).

This commit makes things more iterator-like:
- `Comments::next` now advances `current` instead of `print_comment`.
- `Comments::peek` is added so you can inspect a comment and check a
  predicate without consuming it.
- This requires splitting `PrintState::comments` into immutable and
  mutable versions. The commit also moves the ref inside the `Option` of
  the return type, to save callers from having to use `as_ref`/`as_mut`.
- It also requires adding `PrintState::peek_comment` alongside the
  existing `PrintState::next_comment`. (The lifetimes in the signature
  of `peek_comment` ended up more complex than I expected.)

We now have a neat separation between consuming (`next`) and
non-consuming (`peek`) uses of each comment. As well as being clearer,
this will facilitate the next commit that avoids unnecessary cloning.
    @nnethercote
    nnethercote committed May 13, 2024
    Configuration menu
    Copy the full SHA
    5e7a80b View commit details
    Browse the repository at this point in the history

  2. Make Comments::next consume a comment. 

    This avoids the need for a clone, fixing a FIXME comment.
    @nnethercote
    nnethercote committed May 13, 2024
    Configuration menu
    Copy the full SHA
    74e1b46 View commit details
    Browse the repository at this point in the history