Address issues flagged in review

3 files changed, 45 insertions, 75 deletions

diff --git a/crates/ra_hir/src/code_model_impl/function/scope.rs b/crates/ra_hir/src/code_model_impl/function/scope.rs
index 220c71f74..3a7d53a93 100644
--- a/crates/ra_hir/src/code_model_impl/function/scope.rs
+++ b/crates/ra_hir/src/code_model_impl/function/scope.rs
@@ -97,8 +97,6 @@ impl FnScopes {
                 };
                 self.scopes[scope].entries.push(entry)
             }
-            // FIXME: isn't every call to add_binding starting an entirely new
-            // tree walk!?
             p => p.walk_child_pats(|pat| self.add_bindings(body, scope, pat)),
         }
     }
diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs
index c05bbc442..1d0beb148 100644
--- a/crates/ra_hir/src/expr.rs
+++ b/crates/ra_hir/src/expr.rs
@@ -329,8 +329,6 @@ impl Expr {
 pub struct PatId(RawId);
 impl_arena_id!(PatId);
 
-// copied verbatim from librustc::hir
-
 /// 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.
@@ -341,8 +339,6 @@ pub enum BindingAnnotation {
     /// when matching. For example, the `x` in `Some(x)` will have binding
     /// mode `None`; if you do `let Some(x) = &Some(22)`, it will
     /// ultimately be inferred to be by-reference.
-    ///
-    /// Note that implicit reference skipping is not implemented yet (#42640).
     Unannotated,
 
     /// Annotated with `mut x` -- could be either ref or not, similar to `None`.
@@ -375,7 +371,7 @@ pub struct FieldPat {
 /// Close relative to rustc's hir::PatKind
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub enum Pat {
-    Missing, // do we need this?
+    Missing,
     Wild,
     Tuple(Vec<PatId>),
     Struct {
@@ -387,7 +383,6 @@ pub enum Pat {
         start: ExprId,
         end: ExprId,
     },
-    Box(PatId),
     Slice {
         prefix: Vec<PatId>,
         rest: Option<PatId>,
@@ -420,7 +415,7 @@ impl Pat {
             Pat::Tuple(args) | Pat::TupleStruct { args, .. } => {
                 args.iter().map(|pat| *pat).for_each(f);
             }
-            Pat::Ref { pat, .. } | Pat::Box(pat) => f(*pat),
+            Pat::Ref { pat, .. } => f(*pat),
             Pat::Slice {
                 prefix,
                 rest,
diff --git a/crates/ra_hir/src/ty.rs b/crates/ra_hir/src/ty.rs
index 53b7b5836..09b816cc7 100644
--- a/crates/ra_hir/src/ty.rs
+++ b/crates/ra_hir/src/ty.rs
@@ -37,7 +37,7 @@ use crate::{
     db::HirDatabase,
     type_ref::{TypeRef, Mutability},
     name::KnownName,
-    expr::{Body, Expr, BindingAnnotation, MatchArm, Literal, ExprId, Pat, PatId, UnaryOp, BinaryOp, Statement, FieldPat},
+    expr::{Body, Expr, BindingAnnotation, Literal, ExprId, Pat, PatId, UnaryOp, BinaryOp, Statement, FieldPat},
 };
 
 /// The ID of a type variable.
@@ -874,15 +874,8 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
     }
 
     fn resolve_fields(&self, path: Option<&Path>) -> Option<(Ty, Vec<StructField>)> {
-        let def = path
-            .and_then(|path| self.module.resolve_path(self.db, &path).take_types())
-            .map(|def_id| def_id.resolve(self.db));
-
-        let def = if let Some(def) = def {
-            def
-        } else {
-            return None;
-        };
+        let def_id = self.module.resolve_path(self.db, path?).take_types()?;
+        let def = def_id.resolve(self.db);
 
         match def {
             Def::Struct(s) => {
@@ -891,60 +884,47 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
                     .struct_data(s.def_id())
                     .variant_data
                     .fields()
-                    .iter()
-                    .cloned()
-                    .collect();
+                    .to_owned();
                 Some((type_for_struct(self.db, s), fields))
             }
             Def::EnumVariant(ev) => {
-                let fields: Vec<_> = ev.variant_data(self.db).fields().iter().cloned().collect();
+                let fields: Vec<_> = ev.variant_data(self.db).fields().to_owned();
                 Some((type_for_enum_variant(self.db, ev), fields))
             }
             _ => None,
         }
     }
 
-    fn infer_tuple_struct(&mut self, path: Option<&Path>, subpats: &[PatId]) -> Ty {
-        let (ty, fields) = if let Some(x) = self.resolve_fields(path) {
-            x
-        } else {
-            return Ty::Unknown;
-        };
-
-        if fields.len() != subpats.len() {
-            return Ty::Unknown;
-        }
+    fn infer_tuple_struct_pat(&mut self, path: Option<&Path>, subpats: &[PatId]) -> Ty {
+        let (ty, fields) = self
+            .resolve_fields(path)
+            .unwrap_or((Ty::Unknown, Vec::new()));
 
-        for (&subpat, field) in subpats.iter().zip(fields.iter()) {
-            let sub_ty = self.make_ty(&field.type_ref);
-            self.infer_pat(subpat, &Expectation::has_type(sub_ty));
+        for (i, &subpat) in subpats.iter().enumerate() {
+            let expected_ty = fields
+                .get(i)
+                .map_or(Ty::Unknown, |field| self.make_ty(&field.type_ref));
+            self.infer_pat(subpat, &Expectation::has_type(expected_ty));
         }
 
         ty
     }
 
-    fn infer_struct(&mut self, path: Option<&Path>, subpats: &[FieldPat]) -> Ty {
-        let (ty, fields) = if let Some(x) = self.resolve_fields(path) {
-            x
-        } else {
-            return Ty::Unknown;
-        };
+    fn infer_struct_pat(&mut self, path: Option<&Path>, subpats: &[FieldPat]) -> Ty {
+        let (ty, fields) = self
+            .resolve_fields(path)
+            .unwrap_or((Ty::Unknown, Vec::new()));
 
         for subpat in subpats {
             let matching_field = fields.iter().find(|field| field.name == subpat.name);
-
-            if let Some(field) = matching_field {
-                let typeref = &field.type_ref;
-                let sub_ty = self.make_ty(typeref);
-                self.infer_pat(subpat.pat, &Expectation::has_type(sub_ty));
-            }
+            let expected_ty =
+                matching_field.map_or(Ty::Unknown, |field| self.make_ty(&field.type_ref));
+            self.infer_pat(subpat.pat, &Expectation::has_type(expected_ty));
         }
 
         ty
     }
 
-    // TODO: Expectation should probably contain a Cow pointer to Ty?
-    // so that we can make new expectations of subtypes cheaply
     fn infer_pat(&mut self, pat: PatId, expected: &Expectation) -> Ty {
         let body = Arc::clone(&self.body); // avoid borrow checker problem
 
@@ -969,7 +949,10 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
             }
             Pat::Ref { pat, mutability } => {
                 let expectation = match expected.ty {
-                    Ty::Ref(ref sub_ty, exp_mut) if *mutability == exp_mut => {
+                    Ty::Ref(ref sub_ty, exp_mut) => {
+                        if *mutability != exp_mut {
+                            // TODO: emit type error?
+                        }
                         Expectation::has_type((&**sub_ty).clone())
                     }
                     _ => Expectation::none(),
@@ -980,18 +963,16 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
             Pat::TupleStruct {
                 path: ref p,
                 args: ref subpats,
-            } => self.infer_tuple_struct(p.as_ref(), subpats),
+            } => self.infer_tuple_struct_pat(p.as_ref(), subpats),
             Pat::Struct {
                 path: ref p,
                 args: ref fields,
-            } => self.infer_struct(p.as_ref(), fields),
-            Pat::Path(path) => {
-                // is this right?
-                self.module
-                    .resolve_path(self.db, &path)
-                    .take_values()
-                    .map_or(Ty::Unknown, |resolved| self.db.type_for_def(resolved))
-            }
+            } => self.infer_struct_pat(p.as_ref(), fields),
+            Pat::Path(path) => self
+                .module
+                .resolve_path(self.db, &path)
+                .take_values()
+                .map_or(Ty::Unknown, |resolved| self.db.type_for_def(resolved)),
             Pat::Bind {
                 mode,
                 name: _name,
@@ -1000,10 +981,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
                 let subty = if let Some(subpat) = subpat {
                     self.infer_pat(*subpat, expected)
                 } else {
-                    let ty = self.new_type_var();
-                    self.unify(&ty, &expected.ty);
-                    let ty = self.resolve_ty_as_possible(ty);
-                    ty
+                    expected.ty.clone()
                 };
 
                 match mode {
@@ -1075,8 +1053,8 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
                 assert_eq!(args.len(), arg_types.len());
 
                 for (arg_pat, arg_type) in args.iter().zip(arg_types.iter()) {
-                    let expected = if let Some(tyref) = arg_type {
-                        let ty = self.make_ty(tyref);
+                    let expected = if let Some(type_ref) = arg_type {
+                        let ty = self.make_ty(type_ref);
                         Expectation::has_type(ty)
                     } else {
                         Expectation::none()
@@ -1143,21 +1121,20 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
                 ret_ty
             }
             Expr::Match { expr, arms } => {
-                let mut expected = expected.clone();
+                let expected = if expected.ty == Ty::Unknown {
+                    Expectation::has_type(self.new_type_var())
+                } else {
+                    expected.clone()
+                };
                 let input_ty = self.infer_expr(*expr, &Expectation::none());
                 let pat_expectation = Expectation::has_type(input_ty);
 
-                for MatchArm {
-                    pats,
-                    expr: arm_expr,
-                } in arms
-                {
-                    for &pat in pats {
+                for arm in arms {
+                    for &pat in &arm.pats {
                         let _pat_ty = self.infer_pat(pat, &pat_expectation);
                     }
                     // TODO type the guard
-                    let ty = self.infer_expr(*arm_expr, &expected);
-                    expected = Expectation::has_type(ty);
+                    self.infer_expr(arm.expr, &expected);
                 }
 
                 expected.ty