Match Ergonomics 2024: document and reorganize the currently-implemented feature gates #135237
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This aims to reduce the complexity needed in the boolean logic for telling which rules we're using to type patterns. If we still want the functionality this removes, we can re-add it later, after some cleanup to pattern typing.
As far as I can tell, the assignment removed here will never do anything. `pat_info.max_ref_mutbl` starts at `MutblCap::Mut` for the top-level pattern and is only changed if feature gates are enabled that would result in the statement not being executed. Regardless of what new pattern typing rules are adopted, I don't imagine we want to conditionally reset `max_ref_mutbl` based on edition either, since it'd have consequences for subpatterns in other editions.
If Rules 3 or 5 are adopted in any edition, we'll need to track the `MutblCap` in all editions for macro hygiene purposes. Previously, the check for whether to track it was conflated with the checks for whether to apply Rules 3 and 5, so to make it a bit clearer, this always tracks the `MutblCap`. If needed, we could check if Rules 3 or 5 are present in any edition before tracking the `MutblCap`, but since it's not that much more expensive to always track it, I've figured that's simplest. My main concern with removing the checks is that it may not be clear that the `MutblCap` is tracked for those specific purposes. To try and mitigate this, I've made its doc comment a bit more precise regarding the extent of how and why it's used. This leaves the condition untouched on the `cap_to_weakly_not` call needed for Rule 5, since it's only needed for that and it can affect diagnostics.
The goal of this cleanup is to make it more apparent which feature gates correspond to which typing rules, and which typing rules correspond to what code. My intent is for calls to the "which typing rules do we have?" functions to be replaced by comments (and edition checks, as appropriate), but as long as we're experimenting with multiple rulesets, this seemed to me to be the easiest to document and read. There's still some nontrivial control flow, but I've added comments to try and make it clearer. There's some logic that looks like it could be de-duplicated across different ways of matching against inherited references; however, the duplication is intentional. Once we choose which rulesets we want, we can make this more clever, but until then, my priorities are clarity and ease of modification/extension. That said, I think the diagnostics could use some work; factoring out commonalities there (and separating them from the typing logic) would be ideal. I've opted not to include that here both since it'd make this refactor less obvious and since it affects test output. Also, this doesn't get quite as fine-grained as Typing Rust Patterns, so there's some instances where certain rules are conflated. I'd prefer to minimize dead/untested codepaths for rulesets we're not interested in, so as a compromise I've added comments wherever some aspect of the typing rules is assumed from another. I'm not totally happy with it, but I think it's at least better than plain checks against the feature gates and edition.
This only includes previously existing tests (with a couple duplicates removed). I plan on adding more comprarisons where the rules differ once I've updated the pattern typing rules. I also haven't touched the tests for new rules in old editions; I'll see how best to handle that once those rules are updated as well.
rustbot
added
S-waiting-on-review
This also makes it test the "structural" ruleset, in preparation for additional tests where the rulesets disagree.
This was a test for `ref_pat_eat_one_layer_2024` downgrading a `mut ref` default binding mode to `ref` within a shared reference pattern (i.e. Rule 3) on edition 2021 specifically. Since it's near-identical to another existing test (currently in `ref_pat_eat_one_layer_2021.rs` in the "experimental" rfc 3627 subdirectory) and that particular feature gate's typing rulesets are planned to no longer have Rule 3, I'm opting to remove it rather than continue maintaining it separately.
|
I've added bit more test maintenance. The remaining tests I haven't touched at this point are:
|
It was made as part of the match ergonomics 2024 work, it's not really maintained nor pushed forward by anyone. Let's keep it around until we're sure we won't need it. |
Nadrieril
approved these changes
Jan 19, 2025
There was a problem hiding this comment.
Thanks for this! I'm a fan of making the rules more explicit and separate from the feature gates, this is a great legibility improvement. Much appreciate the small commits also.
Very good point on needing to track MutblCap in all editions! Could you add a test for that? We already have a cross-edition test in rfc-3627-match-ergonomics-2024/migration-lint.rs, it would make sense to add a mixed_editions.rs file with a test or two. Feel free to leave that for a later PR.
Code looks good, I only have suggestions regarding comments.
14 tasks
|
@rustbot author |
rustbot
added
S-waiting-on-author
This adds explanation of inherited references and how they relate to the default binding mode. Co-authored-by: Nadrieril <[email protected]>
|
I've added your suggested comments and debug asserts; ty! I'll make a note to write some tests for mixed-edition patterns too. @rustbot ready |
rustbot
added
S-waiting-on-review
|
Let's goo |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 20, 2025
Rollup of 5 pull requests Successful merges: - rust-lang#134276 (fully de-stabilize all custom inner attributes) - rust-lang#135237 (Match Ergonomics 2024: document and reorganize the currently-implemented feature gates) - rust-lang#135310 (Always force non-trimming of path in `unreachable_patterns` lint) - rust-lang#135446 (further improve panic_immediate_abort by removing rtprintpanic! messages) - rust-lang#135491 (Remove dead rustc_allowed_through_unstable_modules for std::os::fd contents) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 20, 2025
Rollup merge of rust-lang#135237 - dianne:match-2024-cleanup, r=Nadrieril Match Ergonomics 2024: document and reorganize the currently-implemented feature gates The hope here is to make it easier to adjust, understand, and test the experimental pattern typing rules implemented in the compiler. This PR doesn't (or at isn't intended to) change any behavior or add any new tests; I'll be handling that later. I've also included some reasoning/commentary on the more involved changes in the commit messages. Relevant tracking issue: rust-lang#123076 r? `@Nadrieril`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The hope here is to make it easier to adjust, understand, and test the experimental pattern typing rules implemented in the compiler. This PR doesn't (or at isn't intended to) change any behavior or add any new tests; I'll be handling that later. I've also included some reasoning/commentary on the more involved changes in the commit messages.
Relevant tracking issue: #123076
r? @Nadrieril