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

Replace a long inline "autoref" comment with method docs #127556

Merged
merged 1 commit into from
Jul 20, 2024
Merged

Replace a long inline "autoref" comment with method docs #127556

merged 1 commit into from
Jul 20, 2024

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Jul 10, 2024 •
edited
Loading

This comment has two problems:

  • It is very long, making the flow of the enclosing method hard to follow.
  • It starts by talking about an autoref flag that hasn't existed since Enable NLL migrate mode on the 2015 edition #59114.
    • This makes it hard to trust that the information in the comment is accurate or relevant, even though much of it still seems to be true.

This PR therefore replaces the long inline comment with a revised doc comment on bind_matched_candidate_for_guard, and some shorter inline comments.

For readers who want more historical context, we also link to the PR that added the old comment, and the PR that removed the autoref flag.

@rustbot
Copy link
Collaborator

rustbot commented Jul 10, 2024

r? @lcnr

rustbot has assigned @lcnr.
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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 10, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jul 10, 2024

Some changes occurred in match lowering

cc @Nadrieril

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Zalathar Zalathar changed the title Replace a long inline "autoref" comment with some historical links Replace a long inline "autoref" comment with method docs Jul 10, 2024
@lcnr
Copy link
Contributor

lcnr commented Jul 10, 2024

r? @Nadrieril

@rustbot rustbot assigned Nadrieril and unassigned lcnr Jul 10, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Zalathar
Replace a long inline "autoref" comment with method docs
83e1efb 
This comment has two problems:

- It is very long, making the flow of the enclosing method hard to follow.
- It starts by talking about an `autoref` flag that hasn't existed since rust-lang#59114.

This PR therefore replaces the long inline comment with a revised doc comment
on `bind_matched_candidate_for_guard`, and some shorter inline comments.

For readers who want more historical context, we also link to the PR that added
the old comment, and the PR that removed the `autoref` flag.
@Nadrieril
Copy link
Member

Looks good!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 19, 2024

📌 Commit 83e1efb has been approved by Nadrieril

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 19, 2024
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 19, 2024
@tgross35
Rollup merge of rust-lang#127556 - Zalathar:autoref, r=Nadrieril
5fc6649 
Replace a long inline "autoref" comment with method docs

This comment has two problems:

- It is very long, making the flow of the enclosing method hard to follow.
- It starts by talking about an `autoref` flag that hasn't existed since rust-lang#59114.
  - This makes it hard to trust that the information in the comment is accurate or relevant, even though much of it still seems to be true.

This PR therefore replaces the long inline comment with a revised doc comment on `bind_matched_candidate_for_guard`, and some shorter inline comments.

For readers who want more historical context, we also link to the PR that added the old comment, and the PR that removed the `autoref` flag.
@tgross35 tgross35 mentioned this pull request Jul 19, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2024
@bors
Auto merge of rust-lang#127991 - tgross35:rollup-ccfammj, r=tgross35
ead9d17 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#123196 (Add Process support for UEFI)
 - rust-lang#127523 (Migrate `dump-ice-to-disk` and `panic-abort-eh_frame` `run-make` tests to rmake)
 - rust-lang#127556 (Replace a long inline "autoref" comment with method docs)
 - rust-lang#127693 (Migrate `crate-hash-rustc-version` to `rmake`)
 - rust-lang#127866 (Conditionally build `wasm-component-ld` )
 - rust-lang#127918 (Safely enforce thread name requirements)
 - rust-lang#127948 (fixes panic error `index out of bounds` in conflicting error)
 - rust-lang#127980 (Avoid ref when using format! in compiler)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 20, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2024
@bors
Auto merge of rust-lang#127998 - matthiaskrgr:rollup-ykp0h5r, r=matth…
c247b86 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#123196 (Add Process support for UEFI)
 - rust-lang#127556 (Replace a long inline "autoref" comment with method docs)
 - rust-lang#127693 (Migrate `crate-hash-rustc-version` to `rmake`)
 - rust-lang#127866 (Conditionally build `wasm-component-ld` )
 - rust-lang#127918 (Safely enforce thread name requirements)
 - rust-lang#127948 (fixes panic error `index out of bounds` in conflicting error)
 - rust-lang#127980 (Avoid ref when using format! in compiler)
 - rust-lang#127984 (Avoid ref when using format! in src)
 - rust-lang#127987 (More accurate suggestion for `-> Box<dyn Trait>` or `-> impl Trait`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2024
@bors
Auto merge of rust-lang#127998 - matthiaskrgr:rollup-ykp0h5r, r=matth…
1afc5fd 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#123196 (Add Process support for UEFI)
 - rust-lang#127556 (Replace a long inline "autoref" comment with method docs)
 - rust-lang#127693 (Migrate `crate-hash-rustc-version` to `rmake`)
 - rust-lang#127866 (Conditionally build `wasm-component-ld` )
 - rust-lang#127918 (Safely enforce thread name requirements)
 - rust-lang#127948 (fixes panic error `index out of bounds` in conflicting error)
 - rust-lang#127980 (Avoid ref when using format! in compiler)
 - rust-lang#127984 (Avoid ref when using format! in src)
 - rust-lang#127987 (More accurate suggestion for `-> Box<dyn Trait>` or `-> impl Trait`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit dfee7ed into rust-lang:master Jul 20, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2024
@rust-timer
Unrolled build for rust-lang#127556
f2e82b1 
Rollup merge of rust-lang#127556 - Zalathar:autoref, r=Nadrieril

Replace a long inline "autoref" comment with method docs

This comment has two problems:

- It is very long, making the flow of the enclosing method hard to follow.
- It starts by talking about an `autoref` flag that hasn't existed since rust-lang#59114.
  - This makes it hard to trust that the information in the comment is accurate or relevant, even though much of it still seems to be true.

This PR therefore replaces the long inline comment with a revised doc comment on `bind_matched_candidate_for_guard`, and some shorter inline comments.

For readers who want more historical context, we also link to the PR that added the old comment, and the PR that removed the `autoref` flag.
@Zalathar Zalathar deleted the autoref branch July 20, 2024 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nadrieril Nadrieril Nadrieril left review comments

Assignees

@Nadrieril Nadrieril

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.81.0
Development

Successfully merging this pull request may close these issues.
6 participants
@Zalathar @rustbot @rust-log-analyzer @lcnr @Nadrieril @bors