Auto merge of #101241 - camsteffen:refactor-binding-annotations, r=cj…

…gillot

`BindingAnnotation` refactor

* `ast::BindingMode` is deleted and replaced with `hir::BindingAnnotation` (which is moved to `ast`)
* `BindingAnnotation` is changed from an enum to a tuple struct e.g. `BindingAnnotation(ByRef::No, Mutability::Mut)`
* Associated constants added for convenience `BindingAnnotation::{NONE, REF, MUT, REF_MUT}`

One goal is to make it more clear that `BindingAnnotation` merely represents syntax `ref mut` and not the actual binding mode. This was especially confusing since we had `ast::BindingMode`->`hir::BindingAnnotation`->`thir::BindingMode`.

I wish there were more symmetry between `ByRef` and `Mutability` (variant) naming (maybe `Mutable::Yes`?), and I also don't love how long the name `BindingAnnotation` is, but this seems like the best compromise. Ideas welcome.

compiler/rustc_ast/src/ast.rs

@@ -594,7 +594,7 @@ impl Pat {
             // In a type expression `_` is an inference variable.
             PatKind::Wild => TyKind::Infer,
             // An IDENT pattern with no binding mode would be valid as path to a type. E.g. `u32`.
-            PatKind::Ident(BindingMode::ByValue(Mutability::Not), ident, None) => {
+            PatKind::Ident(BindingAnnotation::NONE, ident, None) => {
                 TyKind::Path(None, Path::from_ident(*ident))
             }
             PatKind::Path(qself, path) => TyKind::Path(qself.clone(), path.clone()),
@@ -681,10 +681,43 @@ pub struct PatField {
     pub is_placeholder: bool,
 }
 
-#[derive(Clone, PartialEq, Encodable, Decodable, Debug, Copy)]
-pub enum BindingMode {
-    ByRef(Mutability),
-    ByValue(Mutability),
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+#[derive(Encodable, Decodable, HashStable_Generic)]
+pub enum ByRef {
+    Yes,
+    No,
+}
+
+impl From<bool> for ByRef {
+    fn from(b: bool) -> ByRef {
+        match b {
+            false => ByRef::No,
+            true => ByRef::Yes,
+        }
+    }
+}
+
+/// Explicit binding annotations given in the HIR for a binding. Note
+/// that this is not the final binding *mode* that we infer after type
+/// inference.
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+#[derive(Encodable, Decodable, HashStable_Generic)]
+pub struct BindingAnnotation(pub ByRef, pub Mutability);
+
+impl BindingAnnotation {
+    pub const NONE: Self = Self(ByRef::No, Mutability::Not);
+    pub const REF: Self = Self(ByRef::Yes, Mutability::Not);
+    pub const MUT: Self = Self(ByRef::No, Mutability::Mut);
+    pub const REF_MUT: Self = Self(ByRef::Yes, Mutability::Mut);
+
+    pub fn prefix_str(self) -> &'static str {
+        match self {
+            Self::NONE => "",
+            Self::REF => "ref ",
+            Self::MUT => "mut ",
+            Self::REF_MUT => "ref mut ",
+        }
+    }
 }
 
 #[derive(Clone, Encodable, Decodable, Debug)]
@@ -713,7 +746,7 @@ pub enum PatKind {
     /// or a unit struct/variant pattern, or a const pattern (in the last two cases the third
     /// field must be `None`). Disambiguation cannot be done with parser alone, so it happens
     /// during name resolution.
-    Ident(BindingMode, Ident, Option<P<Pat>>),
+    Ident(BindingAnnotation, Ident, Option<P<Pat>>),
 
     /// A struct or struct variant pattern (e.g., `Variant {x, y, ..}`).
     /// The `bool` is `true` in the presence of a `..`.
@@ -2228,7 +2261,7 @@ pub type ExplicitSelf = Spanned<SelfKind>;
 impl Param {
     /// Attempts to cast parameter to `ExplicitSelf`.
     pub fn to_self(&self) -> Option<ExplicitSelf> {
-        if let PatKind::Ident(BindingMode::ByValue(mutbl), ident, _) = self.pat.kind {
+        if let PatKind::Ident(BindingAnnotation(ByRef::No, mutbl), ident, _) = self.pat.kind {
             if ident.name == kw::SelfLower {
                 return match self.ty.kind {
                     TyKind::ImplicitSelf => Some(respan(self.pat.span, SelfKind::Value(mutbl))),
@@ -2258,31 +2291,31 @@ impl Param {
     pub fn from_self(attrs: AttrVec, eself: ExplicitSelf, eself_ident: Ident) -> Param {
         let span = eself.span.to(eself_ident.span);
         let infer_ty = P(Ty { id: DUMMY_NODE_ID, kind: TyKind::ImplicitSelf, span, tokens: None });
-        let param = |mutbl, ty| Param {
+        let (mutbl, ty) = match eself.node {
+            SelfKind::Explicit(ty, mutbl) => (mutbl, ty),
+            SelfKind::Value(mutbl) => (mutbl, infer_ty),
+            SelfKind::Region(lt, mutbl) => (
+                Mutability::Not,
+                P(Ty {
+                    id: DUMMY_NODE_ID,
+                    kind: TyKind::Rptr(lt, MutTy { ty: infer_ty, mutbl }),
+                    span,
+                    tokens: None,
+                }),
+            ),
+        };
+        Param {
             attrs,
             pat: P(Pat {
                 id: DUMMY_NODE_ID,
-                kind: PatKind::Ident(BindingMode::ByValue(mutbl), eself_ident, None),
+                kind: PatKind::Ident(BindingAnnotation(ByRef::No, mutbl), eself_ident, None),
                 span,
                 tokens: None,
             }),
             span,
             ty,
             id: DUMMY_NODE_ID,
             is_placeholder: false,
-        };
-        match eself.node {
-            SelfKind::Explicit(ty, mutbl) => param(mutbl, ty),
-            SelfKind::Value(mutbl) => param(mutbl, infer_ty),
-            SelfKind::Region(lt, mutbl) => param(
-                Mutability::Not,
-                P(Ty {
-                    id: DUMMY_NODE_ID,
-                    kind: TyKind::Rptr(lt, MutTy { ty: infer_ty, mutbl }),
-                    span,
-                    tokens: None,
-                }),
-            ),
         }
     }
 }

compiler/rustc_ast_lowering/src/expr.rs

@@ -577,7 +577,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let (pat, task_context_hid) = self.pat_ident_binding_mode(
             span,
             Ident::with_dummy_span(sym::_task_context),
-            hir::BindingAnnotation::Mutable,
+            hir::BindingAnnotation::MUT,
         );
         let param = hir::Param {
             hir_id: self.next_id(),
@@ -671,7 +671,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         // this name to identify what is being awaited by a suspended async functions.
         let awaitee_ident = Ident::with_dummy_span(sym::__awaitee);
         let (awaitee_pat, awaitee_pat_hid) =
-            self.pat_ident_binding_mode(span, awaitee_ident, hir::BindingAnnotation::Mutable);
+            self.pat_ident_binding_mode(span, awaitee_ident, hir::BindingAnnotation::MUT);
 
         let task_context_ident = Ident::with_dummy_span(sym::_task_context);
 
@@ -1433,7 +1433,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         // `mut iter`
         let iter = Ident::with_dummy_span(sym::iter);
         let (iter_pat, iter_pat_nid) =
-            self.pat_ident_binding_mode(head_span, iter, hir::BindingAnnotation::Mutable);
+            self.pat_ident_binding_mode(head_span, iter, hir::BindingAnnotation::MUT);
 
         // `match Iterator::next(&mut iter) { ... }`
         let match_expr = {

compiler/rustc_ast_lowering/src/item.rs

@@ -1083,12 +1083,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 // Check if this is a binding pattern, if so, we can optimize and avoid adding a
                 // `let <pat> = __argN;` statement. In this case, we do not rename the parameter.
                 let (ident, is_simple_parameter) = match parameter.pat.kind {
-                    hir::PatKind::Binding(
-                        hir::BindingAnnotation::Unannotated | hir::BindingAnnotation::Mutable,
-                        _,
-                        ident,
-                        _,
-                    ) => (ident, true),
+                    hir::PatKind::Binding(hir::BindingAnnotation(ByRef::No, _), _, ident, _) => {
+                        (ident, true)
+                    }
                     // For `ref mut` or wildcard arguments, we can't reuse the binding, but
                     // we can keep the same name for the parameter.
                     // This lets rustdoc render it correctly in documentation.
@@ -1153,7 +1150,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     let (move_pat, move_id) = this.pat_ident_binding_mode(
                         desugared_span,
                         ident,
-                        hir::BindingAnnotation::Mutable,
+                        hir::BindingAnnotation::MUT,
                     );
                     let move_expr = this.expr_ident(desugared_span, ident, new_parameter_id);
                     let move_stmt = this.stmt_let_pat(

compiler/rustc_ast_lowering/src/lib.rs

@@ -1692,10 +1692,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             output,
             c_variadic,
             implicit_self: decl.inputs.get(0).map_or(hir::ImplicitSelfKind::None, |arg| {
-                use BindingMode::{ByRef, ByValue};
                 let is_mutable_pat = matches!(
                     arg.pat.kind,
-                    PatKind::Ident(ByValue(Mutability::Mut) | ByRef(Mutability::Mut), ..)
+                    PatKind::Ident(hir::BindingAnnotation(_, Mutability::Mut), ..)
                 );
 
                 match arg.ty.kind {
@@ -2360,11 +2359,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     }
 
     fn pat_ident(&mut self, span: Span, ident: Ident) -> (&'hir hir::Pat<'hir>, hir::HirId) {
-        self.pat_ident_binding_mode(span, ident, hir::BindingAnnotation::Unannotated)
+        self.pat_ident_binding_mode(span, ident, hir::BindingAnnotation::NONE)
     }
 
     fn pat_ident_mut(&mut self, span: Span, ident: Ident) -> (hir::Pat<'hir>, hir::HirId) {
-        self.pat_ident_binding_mode_mut(span, ident, hir::BindingAnnotation::Unannotated)
+        self.pat_ident_binding_mode_mut(span, ident, hir::BindingAnnotation::NONE)
     }
 
     fn pat_ident_binding_mode(

compiler/rustc_ast_lowering/src/pat.rs

@@ -24,7 +24,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             let node = loop {
                 match pattern.kind {
                     PatKind::Wild => break hir::PatKind::Wild,
-                    PatKind::Ident(ref binding_mode, ident, ref sub) => {
+                    PatKind::Ident(binding_mode, ident, ref sub) => {
                         let lower_sub = |this: &mut Self| sub.as_ref().map(|s| this.lower_pat(&*s));
                         break self.lower_pat_ident(pattern, binding_mode, ident, lower_sub);
                     }
@@ -176,9 +176,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let mut prev_rest_span = None;
 
         // Lowers `$bm $ident @ ..` to `$bm $ident @ _`.
-        let lower_rest_sub = |this: &mut Self, pat, bm, ident, sub| {
+        let lower_rest_sub = |this: &mut Self, pat, ann, ident, sub| {
             let lower_sub = |this: &mut Self| Some(this.pat_wild_with_node_id_of(sub));
-            let node = this.lower_pat_ident(pat, bm, ident, lower_sub);
+            let node = this.lower_pat_ident(pat, ann, ident, lower_sub);
             this.pat_with_node_id_of(pat, node)
         };
 
@@ -194,9 +194,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 }
                 // Found a sub-slice pattern `$binding_mode $ident @ ..`.
                 // Record, lower it to `$binding_mode $ident @ _`, and stop here.
-                PatKind::Ident(ref bm, ident, Some(ref sub)) if sub.is_rest() => {
+                PatKind::Ident(ann, ident, Some(ref sub)) if sub.is_rest() => {
                     prev_rest_span = Some(sub.span);
-                    slice = Some(self.arena.alloc(lower_rest_sub(self, pat, bm, ident, sub)));
+                    slice = Some(self.arena.alloc(lower_rest_sub(self, pat, ann, ident, sub)));
                     break;
                 }
                 // It was not a subslice pattern so lower it normally.
@@ -209,9 +209,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             // There was a previous subslice pattern; make sure we don't allow more.
             let rest_span = match pat.kind {
                 PatKind::Rest => Some(pat.span),
-                PatKind::Ident(ref bm, ident, Some(ref sub)) if sub.is_rest() => {
+                PatKind::Ident(ann, ident, Some(ref sub)) if sub.is_rest() => {
                     // #69103: Lower into `binding @ _` as above to avoid ICEs.
-                    after.push(lower_rest_sub(self, pat, bm, ident, sub));
+                    after.push(lower_rest_sub(self, pat, ann, ident, sub));
                     Some(sub.span)
                 }
                 _ => None,
@@ -235,7 +235,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     fn lower_pat_ident(
         &mut self,
         p: &Pat,
-        binding_mode: &BindingMode,
+        annotation: BindingAnnotation,
         ident: Ident,
         lower_sub: impl FnOnce(&mut Self) -> Option<&'hir hir::Pat<'hir>>,
     ) -> hir::PatKind<'hir> {
@@ -248,7 +248,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 };
 
                 hir::PatKind::Binding(
-                    self.lower_binding_mode(binding_mode),
+                    annotation,
                     self.lower_node_id(canonical_id),
                     self.lower_ident(ident),
                     lower_sub(self),
@@ -269,15 +269,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         }
     }
 
-    fn lower_binding_mode(&mut self, b: &BindingMode) -> hir::BindingAnnotation {
-        match *b {
-            BindingMode::ByValue(Mutability::Not) => hir::BindingAnnotation::Unannotated,
-            BindingMode::ByRef(Mutability::Not) => hir::BindingAnnotation::Ref,
-            BindingMode::ByValue(Mutability::Mut) => hir::BindingAnnotation::Mutable,
-            BindingMode::ByRef(Mutability::Mut) => hir::BindingAnnotation::RefMut,
-        }
-    }
-
     fn pat_wild_with_node_id_of(&mut self, p: &Pat) -> &'hir hir::Pat<'hir> {
         self.arena.alloc(self.pat_with_node_id_of(p, hir::PatKind::Wild))
     }

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -281,8 +281,8 @@ impl<'a> AstValidator<'a> {
     fn check_decl_no_pat(decl: &FnDecl, mut report_err: impl FnMut(Span, Option<Ident>, bool)) {
         for Param { pat, .. } in &decl.inputs {
             match pat.kind {
-                PatKind::Ident(BindingMode::ByValue(Mutability::Not), _, None) | PatKind::Wild => {}
-                PatKind::Ident(BindingMode::ByValue(Mutability::Mut), ident, None) => {
+                PatKind::Ident(BindingAnnotation::NONE, _, None) | PatKind::Wild => {}
+                PatKind::Ident(BindingAnnotation::MUT, ident, None) => {
                     report_err(pat.span, Some(ident), true)
                 }
                 _ => report_err(pat.span, None, false),

compiler/rustc_ast_pretty/src/pprust/state.rs

@@ -11,8 +11,8 @@ use rustc_ast::tokenstream::{TokenStream, TokenTree};
 use rustc_ast::util::classify;
 use rustc_ast::util::comments::{gather_comments, Comment, CommentStyle};
 use rustc_ast::util::parser;
-use rustc_ast::{self as ast, BlockCheckMode, PatKind, RangeEnd, RangeSyntax};
-use rustc_ast::{attr, Term};
+use rustc_ast::{self as ast, BlockCheckMode, Mutability, PatKind, RangeEnd, RangeSyntax};
+use rustc_ast::{attr, BindingAnnotation, ByRef, Term};
 use rustc_ast::{GenericArg, MacArgs, MacArgsEq};
 use rustc_ast::{GenericBound, SelfKind, TraitBoundModifier};
 use rustc_ast::{InlineAsmOperand, InlineAsmRegOrRegClass};
@@ -1399,16 +1399,12 @@ impl<'a> State<'a> {
         is that it doesn't matter */
         match pat.kind {
             PatKind::Wild => self.word("_"),
-            PatKind::Ident(binding_mode, ident, ref sub) => {
-                match binding_mode {
-                    ast::BindingMode::ByRef(mutbl) => {
-                        self.word_nbsp("ref");
-                        self.print_mutability(mutbl, false);
-                    }
-                    ast::BindingMode::ByValue(ast::Mutability::Not) => {}
-                    ast::BindingMode::ByValue(ast::Mutability::Mut) => {
-                        self.word_nbsp("mut");
-                    }
+            PatKind::Ident(BindingAnnotation(by_ref, mutbl), ident, ref sub) => {
+                if by_ref == ByRef::Yes {
+                    self.word_nbsp("ref");
+                }
+                if mutbl == Mutability::Mut {
+                    self.word_nbsp("mut");
                 }
                 self.print_ident(ident);
                 if let Some(ref p) = *sub {
@@ -1487,12 +1483,10 @@ impl<'a> State<'a> {
             }
             PatKind::Ref(ref inner, mutbl) => {
                 self.word("&");
-                if mutbl == ast::Mutability::Mut {
+                if mutbl == Mutability::Mut {
                     self.word("mut ");
                 }
-                if let PatKind::Ident(ast::BindingMode::ByValue(ast::Mutability::Mut), ..) =
-                    inner.kind
-                {
+                if let PatKind::Ident(ast::BindingAnnotation::MUT, ..) = inner.kind {
                     self.popen();
                     self.print_pat(inner);
                     self.pclose();

compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

@@ -367,7 +367,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
 
                 if let Some(Node::Pat(pat)) = self.infcx.tcx.hir().find(upvar_hir_id)
                     && let hir::PatKind::Binding(
-                        hir::BindingAnnotation::Unannotated,
+                        hir::BindingAnnotation::NONE,
                         _,
                         upvar_ident,
                         _,