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

Match Ergonomics 2024: update old-edition behavior of feature gates #136093

Merged
merged 11 commits into from
Feb 20, 2025

Conversation

dianne
Copy link
Contributor

@dianne dianne commented Jan 26, 2025

This updates the behavior of the feature gates ref_pat_eat_one_layer_2024_structural and ref_pat_eat_one_layer_2024 in Editions 2021 and earlier to correspond to the left and right typing rules compared here, respectively. Compared to the stable_rust rules:

  • they both allow reference patterns to match a lone inherited ref,
  • they both allow & patterns to eat &mut reference types (and lone &mut inherited refs) as if they're shared,
  • they both allow &mut patterns to eat & reference types when there's a &mut inherited reference to also eat,
  • and the left ruleset has RFC 3627's Rule 3: after encountering a shared reference type in the scrutinee, the default binding mode will be treated as by-shared-ref when it would otherwise be by-mutable-ref.

I think there's already tests for all of those typing rules, so I've added revisions to use the existing tests with the new rulesets. Additionally, I've added a few tests to make sure we handle mixed-edition patterns appropriately, and I've added references to the unstable book.

Relevant tracking issue: #123076

r? @ghost

@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 Jan 26, 2025
@dianne dianne force-pushed the match-2024-for-edition-2021 branch 3 times, most recently from d37a4d4 to 29626d0 Compare January 31, 2025 00:05
@dianne dianne marked this pull request as ready for review January 31, 2025 00:06
@dianne
Copy link
Contributor Author

dianne commented Jan 31, 2025

This should now be ready for review, I think!

r? @Nadrieril

@bors
Copy link
Contributor

bors commented Feb 7, 2025

☔ The latest upstream changes (presumably #136697) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Member

@Nadrieril Nadrieril left a comment

Choose a reason for hiding this comment

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

This looks good! I'm sold on the fallback-to-outer (eat both) change. I have a nit about the comments, to make them clearer to readers who don't have all the context in their heads.

There are conflicts in the tests with your other recent PR, once that's fixed I'll merge this!

Comment on lines 2398 to 2414
if fallback_to_outer && inh_mut.is_mut() {
// If we can fall back to matching the inherited reference, the expected
// type is a reference type (of any mutability), and the inherited
// reference is mutable, we'll always be able to match. We handle that
// here to avoid adding fallback-to-outer to the common logic below.
// NB: This way of phrasing the logic will catch more cases than those
// that need to fall back to matching the inherited reference. However,
// as long as `&` patterns can match mutable (inherited) references
// (RFC 3627, Rule 5) this should be sound.
debug_assert!(ref_pat_matches_mut_ref);
self.check_pat(inner, inner_ty, pat_info);
return expected;
Copy link
Member

@Nadrieril Nadrieril Feb 16, 2025

Choose a reason for hiding this comment

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

Hmmmm this feels a bit too clever, I'm not sure if we may accidentally break that with a future feature like deref patterns. What would it look like to do this the boring way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does feel a bit too clever.. I've made it a bit less clever here: b0f337d

That said, I personally don't enjoy any of the fallthrough-type control flow there where some cases use some common logic below and some cases return early. Once there's only a couple of typing rulesets to worry about, my preference would be to find some way to simplify all of that.

Copy link
Member

Choose a reason for hiding this comment

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

agreed

@Nadrieril
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 16, 2025
@dianne dianne force-pushed the match-2024-for-edition-2021 branch from 24a95bf to b0f337d Compare February 17, 2025 02:48
@dianne
Copy link
Contributor Author

dianne commented Feb 17, 2025

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 17, 2025
@Nadrieril
Copy link
Member

let's gooo @bors r+

@bors
Copy link
Contributor

bors commented Feb 17, 2025

📌 Commit b0f337d 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 Feb 17, 2025
@dianne
Copy link
Contributor Author

dianne commented Feb 17, 2025

oh if it's not too late: I think this will conflict with #137161

@Nadrieril
Copy link
Member

which one would you prefer merged first?

@dianne
Copy link
Contributor Author

dianne commented Feb 17, 2025

let's go with #137161, since it fixes a diagnostic issue

@Nadrieril
Copy link
Member

👍 @bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 17, 2025
This also adds `#[cfg]` attributes to tests for bindings' types,
to make it visually clearer which revisions type successfully.
These are superseded by the old-edition revisions on the shared tests.
dianne and others added 9 commits February 18, 2025 17:44
My reasoning: the ruleset implemented by the same feature gate in
Edition 2024 always tries to eat the inherited reference first. For
consistency, it makes sense to me to say across all editions that users
should consider the inherited reference's mutability when wondering if a
`&mut` pattern will type.
@dianne dianne force-pushed the match-2024-for-edition-2021 branch from b0f337d to 0a15bfb Compare February 19, 2025 02:03
@dianne
Copy link
Contributor Author

dianne commented Feb 19, 2025

The conflicting test should now be fixed @rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 19, 2025
@Nadrieril
Copy link
Member

let's gooo @bors r+

@bors
Copy link
Contributor

bors commented Feb 19, 2025

📌 Commit 0a15bfb 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 Feb 19, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 19, 2025
…, r=Nadrieril

Match Ergonomics 2024: update old-edition behavior of feature gates

This updates the behavior of the feature gates `ref_pat_eat_one_layer_2024_structural` and `ref_pat_eat_one_layer_2024` in Editions 2021 and earlier to correspond to the left and right typing rules compared [here](https://nadrieril.github.io/typing-rust-patterns/?opts1=AQEBAQIBAQEBAAAAAAAAAAAAAAAAAAA%3D&style=UserVisible&compare=true&opts2=AQEBAQIBAQABAAAAAQEBAAEBAAABAAA%3D&mode=rules), respectively. Compared to the `stable_rust` rules:
- they both allow reference patterns to match a lone inherited ref,
- they both allow `&` patterns to eat `&mut` reference types (and lone `&mut` inherited refs) as if they're shared,
- they both allow `&mut` patterns to eat `&` reference types when there's a `&mut` inherited reference to also eat,
- and the left ruleset has RFC 3627's Rule 3: after encountering a shared reference type in the scrutinee, the default binding mode will be treated as by-shared-ref when it would otherwise be by-mutable-ref.

I think there's already tests for all of those typing rules, so I've added revisions to use the existing tests with the new rulesets. Additionally, I've added a few tests to make sure we handle mixed-edition patterns appropriately, and I've added references to the unstable book.

Relevant tracking issue: rust-lang#123076

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 19, 2025
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#120580 (Add `MAX_LEN_UTF8` and `MAX_LEN_UTF16` Constants)
 - rust-lang#132268 (Impl TryFrom<Vec<u8>> for String)
 - rust-lang#136093 (Match Ergonomics 2024: update old-edition behavior of feature gates)
 - rust-lang#136344 (Suggest replacing `.` with `::` in more error diagnostics.)
 - rust-lang#136690 (Use more explicit and reliable ptr select in sort impls)
 - rust-lang#136815 (CI: Stop /msys64/bin from being prepended to PATH in msys2 shell)
 - rust-lang#136923 (Lint `#[must_use]` attributes applied to methods in trait impls)
 - rust-lang#137155 (Organize `OsString`/`OsStr` shims)
 - rust-lang#137225 (vectorcall ABI: require SSE2)

Failed merges:

 - rust-lang#136780 (std: move stdio to `sys`)

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

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120580 (Add `MAX_LEN_UTF8` and `MAX_LEN_UTF16` Constants)
 - rust-lang#132268 (Impl TryFrom<Vec<u8>> for String)
 - rust-lang#136093 (Match Ergonomics 2024: update old-edition behavior of feature gates)
 - rust-lang#136344 (Suggest replacing `.` with `::` in more error diagnostics.)
 - rust-lang#136690 (Use more explicit and reliable ptr select in sort impls)
 - rust-lang#136815 (CI: Stop /msys64/bin from being prepended to PATH in msys2 shell)
 - rust-lang#136923 (Lint `#[must_use]` attributes applied to methods in trait impls)
 - rust-lang#137155 (Organize `OsString`/`OsStr` shims)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 659838e into rust-lang:master Feb 20, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Feb 20, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 20, 2025
Rollup merge of rust-lang#136093 - dianne:match-2024-for-edition-2021, r=Nadrieril

Match Ergonomics 2024: update old-edition behavior of feature gates

This updates the behavior of the feature gates `ref_pat_eat_one_layer_2024_structural` and `ref_pat_eat_one_layer_2024` in Editions 2021 and earlier to correspond to the left and right typing rules compared [here](https://nadrieril.github.io/typing-rust-patterns/?opts1=AQEBAQIBAQEBAAAAAAAAAAAAAAAAAAA%3D&style=UserVisible&compare=true&opts2=AQEBAQIBAQABAAAAAQEBAAEBAAABAAA%3D&mode=rules), respectively. Compared to the `stable_rust` rules:
- they both allow reference patterns to match a lone inherited ref,
- they both allow `&` patterns to eat `&mut` reference types (and lone `&mut` inherited refs) as if they're shared,
- they both allow `&mut` patterns to eat `&` reference types when there's a `&mut` inherited reference to also eat,
- and the left ruleset has RFC 3627's Rule 3: after encountering a shared reference type in the scrutinee, the default binding mode will be treated as by-shared-ref when it would otherwise be by-mutable-ref.

I think there's already tests for all of those typing rules, so I've added revisions to use the existing tests with the new rulesets. Additionally, I've added a few tests to make sure we handle mixed-edition patterns appropriately, and I've added references to the unstable book.

Relevant tracking issue: rust-lang#123076

r? ``@ghost``
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120580 (Add `MAX_LEN_UTF8` and `MAX_LEN_UTF16` Constants)
 - rust-lang#132268 (Impl TryFrom<Vec<u8>> for String)
 - rust-lang#136093 (Match Ergonomics 2024: update old-edition behavior of feature gates)
 - rust-lang#136344 (Suggest replacing `.` with `::` in more error diagnostics.)
 - rust-lang#136690 (Use more explicit and reliable ptr select in sort impls)
 - rust-lang#136815 (CI: Stop /msys64/bin from being prepended to PATH in msys2 shell)
 - rust-lang#136923 (Lint `#[must_use]` attributes applied to methods in trait impls)
 - rust-lang#137155 (Organize `OsString`/`OsStr` shims)

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120580 (Add `MAX_LEN_UTF8` and `MAX_LEN_UTF16` Constants)
 - rust-lang#132268 (Impl TryFrom<Vec<u8>> for String)
 - rust-lang#136093 (Match Ergonomics 2024: update old-edition behavior of feature gates)
 - rust-lang#136344 (Suggest replacing `.` with `::` in more error diagnostics.)
 - rust-lang#136690 (Use more explicit and reliable ptr select in sort impls)
 - rust-lang#136815 (CI: Stop /msys64/bin from being prepended to PATH in msys2 shell)
 - rust-lang#136923 (Lint `#[must_use]` attributes applied to methods in trait impls)
 - rust-lang#137155 (Organize `OsString`/`OsStr` shims)

r? `@ghost`
`@rustbot` modify labels: rollup
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