-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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: mut
doesn't reset binding mode
#123535
Match ergonomics 2024: mut
doesn't reset binding mode
#123535
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo one detail and one question
EDIT: oops I was only looking at the first commit
@@ -529,6 +529,8 @@ declare_features! ( | |||
(unstable, more_qualified_paths, "1.54.0", Some(86935)), | |||
/// Allows the `#[must_not_suspend]` attribute. | |||
(unstable, must_not_suspend, "1.57.0", Some(83310)), | |||
/// Make `mut` not reset the binding mode on edition >= 2024. | |||
(unstable, mut_dont_reset_binding_mode_2024, "CURRENT_RUSTC_VERSION", Some(123076)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be incomplete
instead of unstable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mut_preserve_binding_mode_2024
to avoid the double negation? I'm fine either way.
compiler/rustc_hir_typeck/src/pat.rs
Outdated
BindingAnnotation(ByRef::No, Mutability::Not) => def_bm, | ||
_ => ba, | ||
BindingAnnotation(ByRef::No, Mutability::Mut) | ||
if !(pat.span.at_least_rust_2024() | ||
&& self.tcx.features().mut_dont_reset_binding_mode_2024) | ||
&& matches!(def_br, ByRef::Yes(_)) => | ||
{ | ||
// `mut x` resets the binding mode in edition <= 2021. | ||
BindingAnnotation(ByRef::No, Mutability::Mut) | ||
} | ||
BindingAnnotation(ByRef::No, mutbl) => BindingAnnotation(def_br, mutbl), | ||
BindingAnnotation(ByRef::Yes(_), _) => ba, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't preserve old behavior in the case where ba
is BindingAnnotation(ByRef::No, Mutability::Not)
and def_bm.1
is Mutability::Mut
. I guess that can't happen, right? Could we store just a ByRef
in PatInfo
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we store just a
ByRef
inPatInfo
?
I think we can, but it would require changes to calc_default_binding_mode
, so I'll do it in a later PR so as not to conflict with #123512
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I've reviewed all commits 👍
@@ -529,6 +529,8 @@ declare_features! ( | |||
(unstable, more_qualified_paths, "1.54.0", Some(86935)), | |||
/// Allows the `#[must_not_suspend]` attribute. | |||
(unstable, must_not_suspend, "1.57.0", Some(83310)), | |||
/// Make `mut` not reset the binding mode on edition >= 2024. | |||
(unstable, mut_dont_reset_binding_mode_2024, "CURRENT_RUSTC_VERSION", Some(123076)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mut_preserve_binding_mode_2024
to avoid the double negation? I'm fine either way.
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #123991) made this pull request unmergeable. Please resolve the merge conflicts. |
Rustfix remains TODO
The lint is unstable, and the lint group `rust_2024_compatibility` must keep working on stable
Co-authored-by: Guillaume Boisseau <[email protected]>
9f9b051
to
7a32117
Compare
@bors r+ |
…ding_mode_2024, r=Nadrieril Match ergonomics 2024: `mut` doesn't reset binding mode r? `@Nadrieril` cc rust-lang#123076 `@rustbot` label A-edition-2024 A-patterns
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#122632 (fetch submodule before checking llvm stamp) - rust-lang#123355 (Support type '/' to search) - rust-lang#123501 (Stabilize checking of cfgs at compile-time: `--check-cfg` option) - rust-lang#123535 (Match ergonomics 2024: `mut` doesn't reset binding mode) - rust-lang#123711 (drop `changelog-seen`) - rust-lang#123969 (The new solver ignores `DefineOpaqueTypes`, so switch it to `Yes`) - rust-lang#124007 (Miri subtree update) - rust-lang#124017 (Change a diagnostics-path-only `DefineOpaqueTypes` to `Yes`.) - rust-lang#124018 (interpret: pass MemoryKind to before_memory_deallocation) - rust-lang#124024 (interpret: remove outdated comment) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#123535 - Jules-Bertholet:mut_dont_reset_binding_mode_2024, r=Nadrieril Match ergonomics 2024: `mut` doesn't reset binding mode r? ``@Nadrieril`` cc rust-lang#123076 ``@rustbot`` label A-edition-2024 A-patterns
r? @Nadrieril
cc #123076
@rustbot label A-edition-2024 A-patterns