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.

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

Tweak a confusing comment in create_match_candidates #126926

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Jun 25, 2024

This comment was accurate at the time it was written, but various later changes reshuffled things in ways that caused the existing comment to become confusing.

I've therefore tried to clarify that these candidates are 1:1 with match arms, while also warning that that isn't the case in general.

@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 Jun 25, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jun 25, 2024

Some changes occurred in match lowering

cc @Nadrieril

@Zalathar
Copy link
Contributor Author

Zalathar commented Jun 25, 2024

The blame for this comment currently points to #57609, but that's misleading because it's actually an older comment that was moved/modified during that change.

The original comment seems to date back to #27893, when matches/mod.rs was created as part of implementing MIR.

@Nadrieril
Copy link
Member

You're quite right, this whole "sub-candidates" thing is very confusing.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 25, 2024

📌 Commit 8016940 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 Jun 25, 2024
@Nadrieril
Copy link
Member

Btw I'm actively thinking about how to simplify the whole thing, especially the handling of or-patterns. Happy to chat if you're trying to detangle it too

compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jun 25, 2024
…ieril

Tweak a confusing comment in `create_match_candidates`

This comment was accurate at the time it was written, but various later changes reshuffled things in ways that caused the existing comment to become confusing.

I've therefore tried to clarify that *these* candidates are 1:1 with match arms, while also warning that that isn't the case in general.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#126326 (Un-unsafe the `StableOrd` trait)
 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126724 (Fix a span in `parse_ty_bare_fn`.)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126916 (Specify target specific linker for `riscv64gc-gnu` job)
 - rust-lang#126926 (Tweak a confusing comment in `create_match_candidates`)

r? `@ghost`
`@rustbot` modify labels: rollup
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 25, 2024
…ieril

Tweak a confusing comment in `create_match_candidates`

This comment was accurate at the time it was written, but various later changes reshuffled things in ways that caused the existing comment to become confusing.

I've therefore tried to clarify that *these* candidates are 1:1 with match arms, while also warning that that isn't the case in general.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…kingjubilee

Rollup of 14 pull requests

Successful merges:

 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126885 (Remove internal `PathBuf::as_mut_vec`)
 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126916 (Specify target specific linker for `riscv64gc-gnu` job)
 - rust-lang#126926 (Tweak a confusing comment in `create_match_candidates`)
 - rust-lang#126927 (core: VaArgSafe is an unsafe trait)
 - rust-lang#126932 (Tweak `FlatPat::new` to avoid a temporarily-invalid state)
 - rust-lang#126941 (Migrate `run-make/llvm-ident` to `rmake.rs`)
 - rust-lang#126946 (Add missing slash in `const_eval_select` doc comment)
 - rust-lang#126947 (Delegation: ast lowering refactor)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#126302 (Detect unused structs which derived Default)
 - rust-lang#126885 (Remove internal `PathBuf::as_mut_vec`)
 - rust-lang#126916 (Specify target specific linker for `riscv64gc-gnu` job)
 - rust-lang#126926 (Tweak a confusing comment in `create_match_candidates`)
 - rust-lang#126927 (core: VaArgSafe is an unsafe trait)
 - rust-lang#126932 (Tweak `FlatPat::new` to avoid a temporarily-invalid state)
 - rust-lang#126946 (Add missing slash in `const_eval_select` doc comment)
 - rust-lang#126947 (Delegation: ast lowering refactor)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6077c0e into rust-lang:master Jun 25, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 25, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
Rollup merge of rust-lang#126926 - Zalathar:candidate-per-arm, r=Nadrieril

Tweak a confusing comment in `create_match_candidates`

This comment was accurate at the time it was written, but various later changes reshuffled things in ways that caused the existing comment to become confusing.

I've therefore tried to clarify that *these* candidates are 1:1 with match arms, while also warning that that isn't the case in general.
@Zalathar Zalathar deleted the candidate-per-arm branch June 25, 2024 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

4 participants