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

SemVer: Add section on RPIT capturing #14849

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

SemVer: Add section on RPIT capturing #14849

wants to merge 1 commit into from
+42 −0

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Nov 22, 2024

This adds a section on RPIT capturing, added in Rust 1.82, with a note about how this changes in the 2024 edition.

@rustbot
Copy link
Collaborator

rustbot commented Nov 22, 2024

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 22, 2024
@weihanglo
Copy link
Member

Not sure why.

---- freshness::rebuild_tests_if_lib_changes stdout ----
running `/home/runner/work/cargo/cargo/target/debug/cargo build`
running `/home/runner/work/cargo/cargo/target/debug/cargo test`
running `/home/runner/work/cargo/cargo/target/debug/cargo build -v`
running `/home/runner/work/cargo/cargo/target/debug/cargo test -v`
thread 'freshness::rebuild_tests_if_lib_changes' panicked at tests/testsuite/freshness.rs:633:10:

---- expected: tests/testsuite/freshness.rs:623:27
++++ actual:   stderr
   1    1 | [DIRTY] foo v0.0.1 ([ROOT]/foo): the dependency foo was rebuilt ([TIME_DIFF_AFTER_LAST_BUILD])
   2    2 | [COMPILING] foo v0.0.1 ([ROOT]/foo)
   3    3 | [RUNNING] `rustc --crate-name foo [..]
   4      - [RUNNING] `rustc --crate-name foo [..]
   5    4 | error[E0425]: cannot find function `foo` in crate `foo`
        5 +  --> tests/foo.rs:4:34
   6    6 | ...
   7    7 | [ERROR] could not compile `foo` (test "foo") due to 1 previous error
   8    8 | ...∅

weihanglo
weihanglo approved these changes Nov 22, 2024
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Looks great!

cc @obi1kenobi though I've already noticed obi1kenobi/cargo-semver-checks#815.

obi1kenobi
obi1kenobi reviewed Nov 22, 2024
View reviewed changes
Copy link
Member

@obi1kenobi obi1kenobi left a comment

Choose a reason for hiding this comment

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

Very nice! I look forward to being able to cite this new section in cargo-semver-checks lints :)

Thanks for the tag @weihanglo!

src/doc/src/reference/semver.md

See the [edition guide][rpit-capture-guide] and the [reference][rpit-reference] for more information on RPIT capturing.

It is a minor change to capture fewer generic parameters in an RPIT.
Copy link
Member

@obi1kenobi obi1kenobi Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I believe this might not be true for functions in non-sealed traits. If the trait's function definition changes the RPIT captures, I expect implementations of the trait have to be updated to match.

src/doc/src/reference/semver.md
let iter = updated_crate::f(&a, &b);
drop(b); // Error: cannot move out of `b` because it is borrowed
}
```
Copy link
Member

Choose a reason for hiding this comment

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

A thought: would it be worth adding an example, or even just mentioning in a sentence or two that use<> syntax can also be used to capture type parameters, not just lifetimes? That wasn't immediately obvious to me when I first heard about this new language feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@obi1kenobi obi1kenobi obi1kenobi left review comments

@weihanglo weihanglo weihanglo approved these changes

Assignees

@epage epage

Labels
A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ehuss @rustbot @weihanglo @obi1kenobi @epage