Auto merge of rust-lang#16771 - Veykril:self-param-split, r=Veykril

internal: Don't desugar self param into a pattern

Small experiment to see if this simplifies things

Author: bors

crates/hir-def/src/body.rs

@@ -10,7 +10,6 @@ use std::ops::Index;
 
 use base_db::CrateId;
 use cfg::{CfgExpr, CfgOptions};
-use either::Either;
 use hir_expand::{name::Name, HirFileId, InFile};
 use la_arena::{Arena, ArenaMap};
 use rustc_hash::FxHashMap;
@@ -45,7 +44,8 @@ pub struct Body {
     ///
     /// If this `Body` is for the body of a constant, this will just be
     /// empty.
-    pub params: Vec<PatId>,
+    pub params: Box<[PatId]>,
+    pub self_param: Option<BindingId>,
     /// The `ExprId` of the actual body expression.
     pub body_expr: ExprId,
     /// Block expressions in this body that may contain inner items.
@@ -55,14 +55,15 @@ pub struct Body {
 pub type ExprPtr = AstPtr<ast::Expr>;
 pub type ExprSource = InFile<ExprPtr>;
 
-pub type PatPtr = AstPtr<Either<ast::Pat, ast::SelfParam>>;
+pub type PatPtr = AstPtr<ast::Pat>;
 pub type PatSource = InFile<PatPtr>;
 
 pub type LabelPtr = AstPtr<ast::Label>;
 pub type LabelSource = InFile<LabelPtr>;
 
 pub type FieldPtr = AstPtr<ast::RecordExprField>;
 pub type FieldSource = InFile<FieldPtr>;
+
 pub type PatFieldPtr = AstPtr<ast::RecordPatField>;
 pub type PatFieldSource = InFile<PatFieldPtr>;
 
@@ -88,6 +89,8 @@ pub struct BodySourceMap {
     label_map: FxHashMap<LabelSource, LabelId>,
     label_map_back: ArenaMap<LabelId, LabelSource>,
 
+    self_param: Option<InFile<AstPtr<ast::SelfParam>>>,
+
     /// We don't create explicit nodes for record fields (`S { record_field: 92 }`).
     /// Instead, we use id of expression (`92`) to identify the field.
     field_map_back: FxHashMap<ExprId, FieldSource>,
@@ -215,18 +218,18 @@ impl Body {
     fn shrink_to_fit(&mut self) {
         let Self {
             body_expr: _,
+            params: _,
+            self_param: _,
             block_scopes,
             exprs,
             labels,
-            params,
             pats,
             bindings,
             binding_owners,
         } = self;
         block_scopes.shrink_to_fit();
         exprs.shrink_to_fit();
         labels.shrink_to_fit();
-        params.shrink_to_fit();
         pats.shrink_to_fit();
         bindings.shrink_to_fit();
         binding_owners.shrink_to_fit();
@@ -297,6 +300,7 @@ impl Default for Body {
             params: Default::default(),
             block_scopes: Default::default(),
             binding_owners: Default::default(),
+            self_param: Default::default(),
         }
     }
 }
@@ -354,14 +358,12 @@ impl BodySourceMap {
         self.pat_map_back.get(pat).cloned().ok_or(SyntheticSyntax)
     }
 
-    pub fn node_pat(&self, node: InFile<&ast::Pat>) -> Option<PatId> {
-        let src = node.map(|it| AstPtr::new(it).wrap_left());
-        self.pat_map.get(&src).cloned()
+    pub fn self_param_syntax(&self) -> Option<InFile<AstPtr<ast::SelfParam>>> {
+        self.self_param
     }
 
-    pub fn node_self_param(&self, node: InFile<&ast::SelfParam>) -> Option<PatId> {
-        let src = node.map(|it| AstPtr::new(it).wrap_right());
-        self.pat_map.get(&src).cloned()
+    pub fn node_pat(&self, node: InFile<&ast::Pat>) -> Option<PatId> {
+        self.pat_map.get(&node.map(AstPtr::new)).cloned()
     }
 
     pub fn label_syntax(&self, label: LabelId) -> LabelSource {
@@ -401,6 +403,7 @@ impl BodySourceMap {
 
     fn shrink_to_fit(&mut self) {
         let Self {
+            self_param: _,
             expr_map,
             expr_map_back,
             pat_map,

crates/hir-def/src/body/lower.rs

@@ -4,7 +4,6 @@
 use std::mem;
 
 use base_db::CrateId;
-use either::Either;
 use hir_expand::{
     name::{name, AsName, Name},
     ExpandError, InFile,
@@ -29,7 +28,6 @@ use crate::{
     db::DefDatabase,
     expander::Expander,
     hir::{
-        dummy_expr_id,
         format_args::{
             self, FormatAlignment, FormatArgs, FormatArgsPiece, FormatArgument, FormatArgumentKind,
             FormatArgumentsCollector, FormatCount, FormatDebugHex, FormatOptions,
@@ -66,16 +64,7 @@ pub(super) fn lower(
         def_map: expander.module.def_map(db),
         source_map: BodySourceMap::default(),
         ast_id_map: db.ast_id_map(expander.current_file_id()),
-        body: Body {
-            exprs: Default::default(),
-            pats: Default::default(),
-            bindings: Default::default(),
-            binding_owners: Default::default(),
-            labels: Default::default(),
-            params: Vec::new(),
-            body_expr: dummy_expr_id(),
-            block_scopes: Vec::new(),
-        },
+        body: Body::default(),
         expander,
         current_try_block_label: None,
         is_lowering_assignee_expr: false,
@@ -191,35 +180,35 @@ impl ExprCollector<'_> {
         is_async_fn: bool,
     ) -> (Body, BodySourceMap) {
         if let Some((param_list, mut attr_enabled)) = param_list {
+            let mut params = vec![];
             if let Some(self_param) =
                 param_list.self_param().filter(|_| attr_enabled.next().unwrap_or(false))
             {
                 let is_mutable =
                     self_param.mut_token().is_some() && self_param.amp_token().is_none();
-                let ptr = AstPtr::new(&Either::Right(self_param));
                 let binding_id: la_arena::Idx<Binding> =
                     self.alloc_binding(name![self], BindingAnnotation::new(is_mutable, false));
-                let param_pat = self.alloc_pat(Pat::Bind { id: binding_id, subpat: None }, ptr);
-                self.add_definition_to_binding(binding_id, param_pat);
-                self.body.params.push(param_pat);
+                self.body.self_param = Some(binding_id);
+                self.source_map.self_param = Some(self.expander.in_file(AstPtr::new(&self_param)));
             }
 
             for (param, _) in param_list.params().zip(attr_enabled).filter(|(_, enabled)| *enabled)
             {
                 let param_pat = self.collect_pat_top(param.pat());
-                self.body.params.push(param_pat);
+                params.push(param_pat);
             }
+            self.body.params = params.into_boxed_slice();
         };
         self.body.body_expr = self.with_label_rib(RibKind::Closure, |this| {
             if is_async_fn {
                 match body {
                     Some(e) => {
+                        let syntax_ptr = AstPtr::new(&e);
                         let expr = this.collect_expr(e);
-                        this.alloc_expr_desugared(Expr::Async {
-                            id: None,
-                            statements: Box::new([]),
-                            tail: Some(expr),
-                        })
+                        this.alloc_expr_desugared_with_ptr(
+                            Expr::Async { id: None, statements: Box::new([]), tail: Some(expr) },
+                            syntax_ptr,
+                        )
                     }
                     None => this.missing_expr(),
                 }
@@ -405,7 +394,7 @@ impl ExprCollector<'_> {
             }
             ast::Expr::ParenExpr(e) => {
                 let inner = self.collect_expr_opt(e.expr());
-                // make the paren expr point to the inner expression as well
+                // make the paren expr point to the inner expression as well for IDE resolution
                 let src = self.expander.in_file(syntax_ptr);
                 self.source_map.expr_map.insert(src, inner);
                 inner
@@ -707,6 +696,7 @@ impl ExprCollector<'_> {
             .alloc_label_desugared(Label { name: Name::generate_new_name(self.body.labels.len()) });
         let old_label = self.current_try_block_label.replace(label);
 
+        let ptr = AstPtr::new(&e).upcast();
         let (btail, expr_id) = self.with_labeled_rib(label, |this| {
             let mut btail = None;
             let block = this.collect_block_(e, |id, statements, tail| {
@@ -716,23 +706,21 @@ impl ExprCollector<'_> {
             (btail, block)
         });
 
-        let callee = self.alloc_expr_desugared(Expr::Path(try_from_output));
+        let callee = self.alloc_expr_desugared_with_ptr(Expr::Path(try_from_output), ptr);
         let next_tail = match btail {
-            Some(tail) => self.alloc_expr_desugared(Expr::Call {
-                callee,
-                args: Box::new([tail]),
-                is_assignee_expr: false,
-            }),
+            Some(tail) => self.alloc_expr_desugared_with_ptr(
+                Expr::Call { callee, args: Box::new([tail]), is_assignee_expr: false },
+                ptr,
+            ),
             None => {
-                let unit = self.alloc_expr_desugared(Expr::Tuple {
-                    exprs: Box::new([]),
-                    is_assignee_expr: false,
-                });
-                self.alloc_expr_desugared(Expr::Call {
-                    callee,
-                    args: Box::new([unit]),
-                    is_assignee_expr: false,
-                })
+                let unit = self.alloc_expr_desugared_with_ptr(
+                    Expr::Tuple { exprs: Box::new([]), is_assignee_expr: false },
+                    ptr,
+                );
+                self.alloc_expr_desugared_with_ptr(
+                    Expr::Call { callee, args: Box::new([unit]), is_assignee_expr: false },
+                    ptr,
+                )
             }
         };
         let Expr::Block { tail, .. } = &mut self.body.exprs[expr_id] else {
@@ -1067,16 +1055,12 @@ impl ExprCollector<'_> {
                 None => None,
             },
         );
-        match expansion {
-            Some(tail) => {
-                // Make the macro-call point to its expanded expression so we can query
-                // semantics on syntax pointers to the macro
-                let src = self.expander.in_file(syntax_ptr);
-                self.source_map.expr_map.insert(src, tail);
-                Some(tail)
-            }
-            None => None,
-        }
+        expansion.inspect(|&tail| {
+            // Make the macro-call point to its expanded expression so we can query
+            // semantics on syntax pointers to the macro
+            let src = self.expander.in_file(syntax_ptr);
+            self.source_map.expr_map.insert(src, tail);
+        })
     }
 
     fn collect_stmt(&mut self, statements: &mut Vec<Statement>, s: ast::Stmt) {
@@ -1261,7 +1245,7 @@ impl ExprCollector<'_> {
                     (Some(id), Pat::Bind { id, subpat })
                 };
 
-                let ptr = AstPtr::new(&Either::Left(pat));
+                let ptr = AstPtr::new(&pat);
                 let pat = self.alloc_pat(pattern, ptr);
                 if let Some(binding_id) = binding {
                     self.add_definition_to_binding(binding_id, pat);
@@ -1359,9 +1343,10 @@ impl ExprCollector<'_> {
                     suffix: suffix.into_iter().map(|p| self.collect_pat(p, binding_list)).collect(),
                 }
             }
-            #[rustfmt::skip] // https://github.com/rust-lang/rustfmt/issues/5676
             ast::Pat::LiteralPat(lit) => 'b: {
-                let Some((hir_lit, ast_lit)) = pat_literal_to_hir(lit) else { break 'b Pat::Missing };
+                let Some((hir_lit, ast_lit)) = pat_literal_to_hir(lit) else {
+                    break 'b Pat::Missing;
+                };
                 let expr = Expr::Literal(hir_lit);
                 let expr_ptr = AstPtr::new(&ast::Expr::Literal(ast_lit));
                 let expr_id = self.alloc_expr(expr, expr_ptr);
@@ -1397,7 +1382,7 @@ impl ExprCollector<'_> {
             ast::Pat::MacroPat(mac) => match mac.macro_call() {
                 Some(call) => {
                     let macro_ptr = AstPtr::new(&call);
-                    let src = self.expander.in_file(AstPtr::new(&Either::Left(pat)));
+                    let src = self.expander.in_file(AstPtr::new(&pat));
                     let pat =
                         self.collect_macro_call(call, macro_ptr, true, |this, expanded_pat| {
                             this.collect_pat_opt(expanded_pat, binding_list)
@@ -1426,7 +1411,7 @@ impl ExprCollector<'_> {
                 Pat::Range { start, end }
             }
         };
-        let ptr = AstPtr::new(&Either::Left(pat));
+        let ptr = AstPtr::new(&pat);
         self.alloc_pat(pattern, ptr)
     }
 
@@ -1987,10 +1972,19 @@ impl ExprCollector<'_> {
         self.source_map.expr_map.insert(src, id);
         id
     }
-    // FIXME: desugared exprs don't have ptr, that's wrong and should be fixed somehow.
+    // FIXME: desugared exprs don't have ptr, that's wrong and should be fixed.
+    // Migrate to alloc_expr_desugared_with_ptr and then rename back
     fn alloc_expr_desugared(&mut self, expr: Expr) -> ExprId {
         self.body.exprs.alloc(expr)
     }
+    fn alloc_expr_desugared_with_ptr(&mut self, expr: Expr, ptr: ExprPtr) -> ExprId {
+        let src = self.expander.in_file(ptr);
+        let id = self.body.exprs.alloc(expr);
+        self.source_map.expr_map_back.insert(id, src);
+        // We intentionally don't fill this as it could overwrite a non-desugared entry
+        // self.source_map.expr_map.insert(src, id);
+        id
+    }
     fn missing_expr(&mut self) -> ExprId {
         self.alloc_expr_desugared(Expr::Missing)
     }

crates/hir-def/src/body/pretty.rs

@@ -48,7 +48,16 @@ pub(super) fn print_body_hir(db: &dyn DefDatabase, body: &Body, owner: DefWithBo
     let mut p = Printer { db, body, buf: header, indent_level: 0, needs_indent: false };
     if let DefWithBodyId::FunctionId(it) = owner {
         p.buf.push('(');
-        body.params.iter().zip(db.function_data(it).params.iter()).for_each(|(&param, ty)| {
+        let params = &db.function_data(it).params;
+        let mut params = params.iter();
+        if let Some(self_param) = body.self_param {
+            p.print_binding(self_param);
+            p.buf.push(':');
+            if let Some(ty) = params.next() {
+                p.print_type_ref(ty);
+            }
+        }
+        body.params.iter().zip(params).for_each(|(&param, ty)| {
             p.print_pat(param);
             p.buf.push(':');
             p.print_type_ref(ty);

crates/hir-def/src/body/scope.rs

@@ -96,6 +96,9 @@ impl ExprScopes {
             scope_by_expr: ArenaMap::with_capacity(body.exprs.len()),
         };
         let mut root = scopes.root_scope();
+        if let Some(self_param) = body.self_param {
+            scopes.add_bindings(body, root, self_param);
+        }
         scopes.add_params_bindings(body, root, &body.params);
         compute_expr_scopes(body.body_expr, body, &mut scopes, &mut root);
         scopes

crates/hir-ty/src/infer.rs

@@ -22,7 +22,7 @@ mod pat;
 mod path;
 pub(crate) mod unify;
 
-use std::{convert::identity, ops::Index};
+use std::{convert::identity, iter, ops::Index};
 
 use chalk_ir::{
     cast::Cast, fold::TypeFoldable, interner::HasInterner, DebruijnIndex, Mutability, Safety,
@@ -777,7 +777,15 @@ impl<'a> InferenceContext<'a> {
 
             param_tys.push(va_list_ty)
         }
-        for (ty, pat) in param_tys.into_iter().zip(self.body.params.iter()) {
+        let mut param_tys = param_tys.into_iter().chain(iter::repeat(self.table.new_type_var()));
+        if let Some(self_param) = self.body.self_param {
+            if let Some(ty) = param_tys.next() {
+                let ty = self.insert_type_vars(ty);
+                let ty = self.normalize_associated_types_in(ty);
+                self.write_binding_ty(self_param, ty);
+            }
+        }
+        for (ty, pat) in param_tys.zip(&*self.body.params) {
             let ty = self.insert_type_vars(ty);
             let ty = self.normalize_associated_types_in(ty);
 

crates/hir-ty/src/layout.rs

@@ -371,8 +371,8 @@ pub fn layout_of_ty_query(
         TyKind::Never => cx.layout_of_never_type(),
         TyKind::Dyn(_) | TyKind::Foreign(_) => {
             let mut unit = layout_of_unit(&cx, dl)?;
-            match unit.abi {
-                Abi::Aggregate { ref mut sized } => *sized = false,
+            match &mut unit.abi {
+                Abi::Aggregate { sized } => *sized = false,
                 _ => return Err(LayoutError::Unknown),
             }
             unit

crates/hir-ty/src/mir.rs

@@ -1165,6 +1165,7 @@ impl MirBody {
 pub enum MirSpan {
     ExprId(ExprId),
     PatId(PatId),
+    SelfParam,
     Unknown,
 }