Repository: rust-analyzer/rust-analyzer
Pull request #2223 (2019-11-12 15:48:28)

Move expression lowering to hir_def


Patch:
diff --git a/crates/ra_db/src/fixture.rs b/crates/ra_db/src/fixture.rs
index ee883b6154..f5dd59f840 100644
--- a/crates/ra_db/src/fixture.rs
+++ b/crates/ra_db/src/fixture.rs
@@ -1,6 +1,5 @@
 //! FIXME: write short doc here
 
-use std::str::FromStr;
 use std::sync::Arc;
 
 use ra_cfg::CfgOptions;
@@ -165,7 +164,7 @@ fn parse_meta(meta: &str) -> ParsedMeta {
         match key {
             "crate" => krate = Some(value.to_string()),
             "deps" => deps = value.split(',').map(|it| it.to_string()).collect(),
-            "edition" => edition = Edition::from_str(&value).unwrap(),
+            "edition" => edition = Edition::from_string(&value),
             "cfg" => {
                 for key in value.split(',') {
                     match split1(key, '=') {
diff --git a/crates/ra_db/src/input.rs b/crates/ra_db/src/input.rs
index c0d95a13fa..60f7dc8815 100644
--- a/crates/ra_db/src/input.rs
+++ b/crates/ra_db/src/input.rs
@@ -13,7 +13,6 @@ use ra_syntax::SmolStr;
 use rustc_hash::FxHashSet;
 
 use crate::{RelativePath, RelativePathBuf};
-use std::str::FromStr;
 
 /// `FileId` is an integer which uniquely identifies a file. File paths are
 /// messy and system-dependent, so most of the code should work directly with
@@ -98,18 +97,12 @@ pub enum Edition {
     Edition2015,
 }
 
-#[derive(Debug)]
-pub struct ParseEditionError {
-    pub msg: String,
-}
-
-impl FromStr for Edition {
-    type Err = ParseEditionError;
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
+impl Edition {
+    //FIXME: replace with FromStr with proper error handling
+    pub fn from_string(s: &str) -> Edition {
         match s {
-            "2015" => Ok(Edition::Edition2015),
-            "2018" => Ok(Edition::Edition2018),
-            _ => Err(ParseEditionError { msg: format!("unknown edition: {}", s) }),
+            "2015" => Edition::Edition2015,
+            "2018" | _ => Edition::Edition2018,
         }
     }
 }
diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs
index abf4ae4023..9ac811232f 100644
--- a/crates/ra_hir/src/db.rs
+++ b/crates/ra_hir/src/db.rs
@@ -113,10 +113,10 @@ pub trait HirDatabase: DefDatabase + AstDatabase {
     #[salsa::invoke(crate::ty::generic_defaults_query)]
     fn generic_defaults(&self, def: GenericDef) -> Substs;
 
-    #[salsa::invoke(Body::body_with_source_map_query)]
+    #[salsa::invoke(crate::expr::body_with_source_map_query)]
     fn body_with_source_map(&self, def: DefWithBody) -> (Arc<Body>, Arc<BodySourceMap>);
 
-    #[salsa::invoke(Body::body_query)]
+    #[salsa::invoke(crate::expr::body_query)]
     fn body(&self, def: DefWithBody) -> Arc<Body>;
 
     #[salsa::invoke(crate::ty::method_resolution::CrateImplBlocks::impls_in_crate_query)]
diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs
index ddf6051118..82955fa558 100644
--- a/crates/ra_hir/src/expr.rs
+++ b/crates/ra_hir/src/expr.rs
@@ -1,112 +1,52 @@
 //! FIXME: write short doc here
 
-pub(crate) mod lower;
 pub(crate) mod scope;
 pub(crate) mod validation;
 
-use std::{ops::Index, sync::Arc};
+use std::sync::Arc;
 
-use ra_arena::{map::ArenaMap, Arena};
 use ra_syntax::{ast, AstPtr};
-use rustc_hash::FxHashMap;
 
-use crate::{db::HirDatabase, DefWithBody, Either, HasSource, Resolver, Source};
+use crate::{db::HirDatabase, DefWithBody, HasSource, Resolver};
 
 pub use self::scope::ExprScopes;
 
-pub use hir_def::expr::{
-    ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp, MatchArm,
-    Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, UnaryOp,
+pub use hir_def::{
+    body::{Body, BodySourceMap, ExprPtr, ExprSource, PatPtr, PatSource},
+    expr::{
+        ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp,
+        MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, UnaryOp,
+    },
 };
 
-/// The body of an item (function, const etc.).
-#[derive(Debug, Eq, PartialEq)]
-pub struct Body {
-    exprs: Arena<ExprId, Expr>,
-    pats: Arena<PatId, Pat>,
-    /// The patterns for the function's parameters. While the parameter types are
-    /// part of the function signature, the patterns are not (they don't change
-    /// the external type of the function).
-    ///
-    /// If this `Body` is for the body of a constant, this will just be
-    /// empty.
-    params: Vec<PatId>,
-    /// The `ExprId` of the actual body expression.
-    body_expr: ExprId,
+pub(crate) fn body_with_source_map_query(
+    db: &impl HirDatabase,
+    def: DefWithBody,
+) -> (Arc<Body>, Arc<BodySourceMap>) {
+    let mut params = None;
+
+    let (file_id, body) = match def {
+        DefWithBody::Function(f) => {
+            let src = f.source(db);
+            params = src.ast.param_list();
+            (src.file_id, src.ast.body().map(ast::Expr::from))
+        }
+        DefWithBody::Const(c) => {
+            let src = c.source(db);
+            (src.file_id, src.ast.body())
+        }
+        DefWithBody::Static(s) => {
+            let src = s.source(db);
+            (src.file_id, src.ast.body())
+        }
+    };
+    let resolver = hir_def::body::MacroResolver::new(db, def.module(db).id);
+    let (body, source_map) = Body::new(db, resolver, file_id, params, body);
+    (Arc::new(body), Arc::new(source_map))
 }
 
-type ExprPtr = Either<AstPtr<ast::Expr>, AstPtr<ast::RecordField>>;
-type ExprSource = Source<ExprPtr>;
-
-type PatPtr = Either<AstPtr<ast::Pat>, AstPtr<ast::SelfParam>>;
-type PatSource = Source<PatPtr>;
-
-/// An item body together with the mapping from syntax nodes to HIR expression
-/// IDs. This is needed to go from e.g. a position in a file to the HIR
-/// expression containing it; but for type inference etc., we want to operate on
-/// a structure that is agnostic to the actual positions of expressions in the
-/// file, so that we don't recompute types whenever some whitespace is typed.
-///
-/// One complication here is that, due to macro expansion, a single `Body` might
-/// be spread across several files. So, for each ExprId and PatId, we record
-/// both the HirFileId and the position inside the file. However, we only store
-/// AST -> ExprId mapping for non-macro files, as it is not clear how to handle
-/// this properly for macros.
-#[derive(Default, Debug, Eq, PartialEq)]
-pub struct BodySourceMap {
-    expr_map: FxHashMap<ExprPtr, ExprId>,
-    expr_map_back: ArenaMap<ExprId, ExprSource>,
-    pat_map: FxHashMap<PatPtr, PatId>,
-    pat_map_back: ArenaMap<PatId, PatSource>,
-    field_map: FxHashMap<(ExprId, usize), AstPtr<ast::RecordField>>,
-}
-
-impl Body {
-    pub(crate) fn body_with_source_map_query(
-        db: &impl HirDatabase,
-        def: DefWithBody,
-    ) -> (Arc<Body>, Arc<BodySourceMap>) {
-        let mut params = None;
-
-        let (file_id, body) = match def {
-            DefWithBody::Function(f) => {
-                let src = f.source(db);
-                params = src.ast.param_list();
-                (src.file_id, src.ast.body().map(ast::Expr::from))
-            }
-            DefWithBody::Const(c) => {
-                let src = c.source(db);
-                (src.file_id, src.ast.body())
-            }
-            DefWithBody::Static(s) => {
-                let src = s.source(db);
-                (src.file_id, src.ast.body())
-            }
-        };
-
-        let (body, source_map) = lower::lower(db, def.resolver(db), file_id, params, body);
-        (Arc::new(body), Arc::new(source_map))
-    }
-
-    pub(crate) fn body_query(db: &impl HirDatabase, def: DefWithBody) -> Arc<Body> {
-        db.body_with_source_map(def).0
-    }
-
-    pub fn params(&self) -> &[PatId] {
-        &self.params
-    }
-
-    pub fn body_expr(&self) -> ExprId {
-        self.body_expr
-    }
-
-    pub fn exprs(&self) -> impl Iterator<Item = (ExprId, &Expr)> {
-        self.exprs.iter()
-    }
-
-    pub fn pats(&self) -> impl Iterator<Item = (PatId, &Pat)> {
-        self.pats.iter()
-    }
+pub(crate) fn body_query(db: &impl HirDatabase, def: DefWithBody) -> Arc<Body> {
+    db.body_with_source_map(def).0
 }
 
 // needs arbitrary_self_types to be a method... or maybe move to the def?
@@ -132,41 +72,3 @@ pub(crate) fn resolver_for_scope(
     }
     r
 }
-
-impl Index<ExprId> for Body {
-    type Output = Expr;
-
-    fn index(&self, expr: ExprId) -> &Expr {
-        &self.exprs[expr]
-    }
-}
-
-impl Index<PatId> for Body {
-    type Output = Pat;
-
-    fn index(&self, pat: PatId) -> &Pat {
-        &self.pats[pat]
-    }
-}
-
-impl BodySourceMap {
-    pub(crate) fn expr_syntax(&self, expr: ExprId) -> Option<ExprSource> {
-        self.expr_map_back.get(expr).copied()
-    }
-
-    pub(crate) fn node_expr(&self, node: &ast::Expr) -> Option<ExprId> {
-        self.expr_map.get(&Either::A(AstPtr::new(node))).cloned()
-    }
-
-    pub(crate) fn pat_syntax(&self, pat: PatId) -> Option<PatSource> {
-        self.pat_map_back.get(pat).copied()
-    }
-
-    pub(crate) fn node_pat(&self, node: &ast::Pat) -> Option<PatId> {
-        self.pat_map.get(&Either::A(AstPtr::new(node))).cloned()
-    }
-
-    pub(crate) fn field_syntax(&self, expr: ExprId, field: usize) -> AstPtr<ast::RecordField> {
-        self.field_map[&(expr, field)]
-    }
-}
diff --git a/crates/ra_hir/src/expr/lower.rs b/crates/ra_hir/src/expr/lower.rs
deleted file mode 100644
index adc68b23c5..0000000000
--- a/crates/ra_hir/src/expr/lower.rs
+++ /dev/null
@@ -1,593 +0,0 @@
-//! FIXME: write short doc here
-
-use hir_def::{
-    builtin_type::{BuiltinFloat, BuiltinInt},
-    path::GenericArgs,
-    type_ref::TypeRef,
-};
-use hir_expand::{
-    hygiene::Hygiene,
-    name::{self, AsName, Name},
-};
-use ra_arena::Arena;
-use ra_syntax::{
-    ast::{
-        self, ArgListOwner, ArrayExprKind, LiteralKind, LoopBodyOwner, NameOwner,
-        TypeAscriptionOwner,
-    },
-    AstNode, AstPtr,
-};
-use test_utils::tested_by;
-
-use crate::{
-    db::HirDatabase, AstId, Either, HirFileId, MacroCallLoc, MacroFileKind, Mutability, Path,
-    Resolver, Source,
-};
-
-use super::{
-    Array, BinaryOp, BindingAnnotation, Body, BodySourceMap, Expr, ExprId, Literal, MatchArm, Pat,
-    PatId, PatPtr, RecordFieldPat, RecordLitField, Statement,
-};
-
-pub(super) fn lower(
-    db: &impl HirDatabase,
-    resolver: Resolver,
-    file_id: HirFileId,
-    params: Option<ast::ParamList>,
-    body: Option<ast::Expr>,
-) -> (Body, BodySourceMap) {
-    ExprCollector {
-        resolver,
-        db,
-        original_file_id: file_id,
-        current_file_id: file_id,
-        source_map: BodySourceMap::default(),
-        body: Body {
-            exprs: Arena::default(),
-            pats: Arena::default(),
-            params: Vec::new(),
-            body_expr: ExprId::dummy(),
-        },
-    }
-    .collect(params, body)
-}
-
-struct ExprCollector<DB> {
-    db: DB,
-    resolver: Resolver,
-    // Expr collector expands macros along the way. original points to the file
-    // we started with, current points to the current macro expansion. source
-    // maps don't support macros yet, so we only record info into source map if
-    // current == original (see #1196)
-    original_file_id: HirFileId,
-    current_file_id: HirFileId,
-
-    body: Body,
-    source_map: BodySourceMap,
-}
-
-impl<'a, DB> ExprCollector<&'a DB>
-where
-    DB: HirDatabase,
-{
-    fn collect(
-        mut self,
-        param_list: Option<ast::ParamList>,
-        body: Option<ast::Expr>,
-    ) -> (Body, BodySourceMap) {
-        if let Some(param_list) = param_list {
-            if let Some(self_param) = param_list.self_param() {
-                let ptr = AstPtr::new(&self_param);
-                let param_pat = self.alloc_pat(
-                    Pat::Bind {
-                        name: name::SELF_PARAM,
-                        mode: BindingAnnotation::Unannotated,
-                        subpat: None,
-                    },
-                    Either::B(ptr),
-                );
-                self.body.params.push(param_pat);
-            }
-
-            for param in param_list.params() {
-                let pat = match param.pat() {
-                    None => continue,
-                    Some(pat) => pat,
-                };
-                let param_pat = self.collect_pat(pat);
-                self.body.params.push(param_pat);
-            }
-        };
-
-        self.body.body_expr = self.collect_expr_opt(body);
-        (self.body, self.source_map)
-    }
-
-    fn alloc_expr(&mut self, expr: Expr, ptr: AstPtr<ast::Expr>) -> ExprId {
-        let ptr = Either::A(ptr);
-        let id = self.body.exprs.alloc(expr);
-        if self.current_file_id == self.original_file_id {
-            self.source_map.expr_map.insert(ptr, id);
-        }
-        self.source_map
-            .expr_map_back
-            .insert(id, Source { file_id: self.current_file_id, ast: ptr });
-        id
-    }
-    // desugared exprs don't have ptr, that's wrong and should be fixed
-    // somehow.
-    fn alloc_expr_desugared(&mut self, expr: Expr) -> ExprId {
-        self.body.exprs.alloc(expr)
-    }
-    fn alloc_expr_field_shorthand(&mut self, expr: Expr, ptr: AstPtr<ast::RecordField>) -> ExprId {
-        let ptr = Either::B(ptr);
-        let id = self.body.exprs.alloc(expr);
-        if self.current_file_id == self.original_file_id {
-            self.source_map.expr_map.insert(ptr, id);
-        }
-        self.source_map
-            .expr_map_back
-            .insert(id, Source { file_id: self.current_file_id, ast: ptr });
-        id
-    }
-    fn alloc_pat(&mut self, pat: Pat, ptr: PatPtr) -> PatId {
-        let id = self.body.pats.alloc(pat);
-        if self.current_file_id == self.original_file_id {
-            self.source_map.pat_map.insert(ptr, id);
-        }
-        self.source_map.pat_map_back.insert(id, Source { file_id: self.current_file_id, ast: ptr });
-        id
-    }
-
-    fn empty_block(&mut self) -> ExprId {
-        let block = Expr::Block { statements: Vec::new(), tail: None };
-        self.body.exprs.alloc(block)
-    }
-
-    fn missing_expr(&mut self) -> ExprId {
-        self.body.exprs.alloc(Expr::Missing)
-    }
-
-    fn missing_pat(&mut self) -> PatId {
-        self.body.pats.alloc(Pat::Missing)
-    }
-
-    fn collect_expr(&mut self, expr: ast::Expr) -> ExprId {
-        let syntax_ptr = AstPtr::new(&expr);
-        match expr {
-            ast::Expr::IfExpr(e) => {
-                let then_branch = self.collect_block_opt(e.then_branch());
-
-                let else_branch = e.else_branch().map(|b| match b {
-                    ast::ElseBranch::Block(it) => self.collect_block(it),
-                    ast::ElseBranch::IfExpr(elif) => {
-                        let expr: ast::Expr = ast::Expr::cast(elif.syntax().clone()).unwrap();
-                        self.collect_expr(expr)
-                    }
-                });
-
-                let condition = match e.condition() {
-                    None => self.missing_expr(),
-                    Some(condition) => match condition.pat() {
-                        None => self.collect_expr_opt(condition.expr()),
-                        // if let -- desugar to match
-                        Some(pat) => {
-                            let pat = self.collect_pat(pat);
-                            let match_expr = self.collect_expr_opt(condition.expr());
-                            let placeholder_pat = self.missing_pat();
-                            let arms = vec![
-                                MatchArm { pats: vec![pat], expr: then_branch, guard: None },
-                                MatchArm {
-                                    pats: vec![placeholder_pat],
-                                    expr: else_branch.unwrap_or_else(|| self.empty_block()),
-                                    guard: None,
-                                },
-                            ];
-                            return self
-                                .alloc_expr(Expr::Match { expr: match_expr, arms }, syntax_ptr);
-                        }
-                    },
-                };
-
-                self.alloc_expr(Expr::If { condition, then_branch, else_branch }, syntax_ptr)
-            }
-            ast::Expr::TryBlockExpr(e) => {
-                let body = self.collect_block_opt(e.body());
-                self.alloc_expr(Expr::TryBlock { body }, syntax_ptr)
-            }
-            ast::Expr::BlockExpr(e) => self.collect_block(e),
-            ast::Expr::LoopExpr(e) => {
-                let body = self.collect_block_opt(e.loop_body());
-                self.alloc_expr(Expr::Loop { body }, syntax_ptr)
-            }
-            ast::Expr::WhileExpr(e) => {
-                let body = self.collect_block_opt(e.loop_body());
-
-                let condition = match e.condition() {
-                    None => self.missing_expr(),
-                    Some(condition) => match condition.pat() {
-                        None => self.collect_expr_opt(condition.expr()),
-                        // if let -- desugar to match
-                        Some(pat) => {
-                            tested_by!(infer_while_let);
-                            let pat = self.collect_pat(pat);
-                            let match_expr = self.collect_expr_opt(condition.expr());
-                            let placeholder_pat = self.missing_pat();
-                            let break_ = self.alloc_expr_desugared(Expr::Break { expr: None });
-                            let arms = vec![
-                                MatchArm { pats: vec![pat], expr: body, guard: None },
-                                MatchArm { pats: vec![placeholder_pat], expr: break_, guard: None },
-                            ];
-                            let match_expr =
-                                self.alloc_expr_desugared(Expr::Match { expr: match_expr, arms });
-                            return self.alloc_expr(Expr::Loop { body: match_expr }, syntax_ptr);
-                        }
-                    },
-                };
-
-                self.alloc_expr(Expr::While { condition, body }, syntax_ptr)
-            }
-            ast::Expr::ForExpr(e) => {
-                let iterable = self.collect_expr_opt(e.iterable());
-                let pat = self.collect_pat_opt(e.pat());
-                let body = self.collect_block_opt(e.loop_body());
-                self.alloc_expr(Expr::For { iterable, pat, body }, syntax_ptr)
-            }
-            ast::Expr::CallExpr(e) => {
-                let callee = self.collect_expr_opt(e.expr());
-                let args = if let Some(arg_list) = e.arg_list() {
-                    arg_list.args().map(|e| self.collect_expr(e)).collect()
-                } else {
-                    Vec::new()
-                };
-                self.alloc_expr(Expr::Call { callee, args }, syntax_ptr)
-            }
-            ast::Expr::MethodCallExpr(e) => {
-                let receiver = self.collect_expr_opt(e.expr());
-                let args = if let Some(arg_list) = e.arg_list() {
-                    arg_list.args().map(|e| self.collect_expr(e)).collect()
-                } else {
-                    Vec::new()
-                };
-                let method_name = e.name_ref().map(|nr| nr.as_name()).unwrap_or_else(Name::missing);
-                let generic_args = e.type_arg_list().and_then(GenericArgs::from_ast);
-                self.alloc_expr(
-                    Expr::MethodCall { receiver, method_name, args, generic_args },
-                    syntax_ptr,
-                )
-            }
-            ast::Expr::MatchExpr(e) => {
-                let expr = self.collect_expr_opt(e.expr());
-                let arms = if let Some(match_arm_list) = e.match_arm_list() {
-                    match_arm_list
-                        .arms()
-                        .map(|arm| MatchArm {
-                            pats: arm.pats().map(|p| self.collect_pat(p)).collect(),
-                            expr: self.collect_expr_opt(arm.expr()),
-                            guard: arm
-                                .guard()
-                                .and_then(|guard| guard.expr())
-                                .map(|e| self.collect_expr(e)),
-                        })
-                        .collect()
-                } else {
-                    Vec::new()
-                };
-                self.alloc_expr(Expr::Match { expr, arms }, syntax_ptr)
-            }
-            ast::Expr::PathExpr(e) => {
-                let path = e
-                    .path()
-                    .and_then(|path| self.parse_path(path))
-                    .map(Expr::Path)
-                    .unwrap_or(Expr::Missing);
-                self.alloc_expr(path, syntax_ptr)
-            }
-            ast::Expr::ContinueExpr(_e) => {
-                // FIXME: labels
-                self.alloc_expr(Expr::Continue, syntax_ptr)
-            }
-            ast::Expr::BreakExpr(e) => {
-                let expr = e.expr().map(|e| self.collect_expr(e));
-                self.alloc_expr(Expr::Break { expr }, syntax_ptr)
-            }
-            ast::Expr::ParenExpr(e) => {
-                let inner = self.collect_expr_opt(e.expr());
-                // make the paren expr point to the inner expression as well
-                self.source_map.expr_map.insert(Either::A(syntax_ptr), inner);
-                inner
-            }
-            ast::Expr::ReturnExpr(e) => {
-                let expr = e.expr().map(|e| self.collect_expr(e));
-                self.alloc_expr(Expr::Return { expr }, syntax_ptr)
-            }
-            ast::Expr::RecordLit(e) => {
-                let path = e.path().and_then(|path| self.parse_path(path));
-                let mut field_ptrs = Vec::new();
-                let record_lit = if let Some(nfl) = e.record_field_list() {
-                    let fields = nfl
-                        .fields()
-                        .inspect(|field| field_ptrs.push(AstPtr::new(field)))
-                        .map(|field| RecordLitField {
-                            name: field
-                                .name_ref()
-                                .map(|nr| nr.as_name())
-                                .unwrap_or_else(Name::missing),
-                            expr: if let Some(e) = field.expr() {
-                                self.collect_expr(e)
-                            } else if let Some(nr) = field.name_ref() {
-                                // field shorthand
-                                self.alloc_expr_field_shorthand(
-                                    Expr::Path(Path::from_name_ref(&nr)),
-                                    AstPtr::new(&field),
-                                )
-                            } else {
-                                self.missing_expr()
-                            },
-                        })
-                        .collect();
-                    let spread = nfl.spread().map(|s| self.collect_expr(s));
-                    Expr::RecordLit { path, fields, spread }
-                } else {
-                    Expr::RecordLit { path, fields: Vec::new(), spread: None }
-                };
-
-                let res = self.alloc_expr(record_lit, syntax_ptr);
-                for (i, ptr) in field_ptrs.into_iter().enumerate() {
-                    self.source_map.field_map.insert((res, i), ptr);
-                }
-                res
-            }
-            ast::Expr::FieldExpr(e) => {
-                let expr = self.collect_expr_opt(e.expr());
-                let name = match e.field_access() {
-                    Some(kind) => kind.as_name(),
-                    _ => Name::missing(),
-                };
-                self.alloc_expr(Expr::Field { expr, name }, syntax_ptr)
-            }
-            ast::Expr::AwaitExpr(e) => {
-                let expr = self.collect_expr_opt(e.expr());
-                self.alloc_expr(Expr::Await { expr }, syntax_ptr)
-            }
-            ast::Expr::TryExpr(e) => {
-                let expr = self.collect_expr_opt(e.expr());
-                self.alloc_expr(Expr::Try { expr }, syntax_ptr)
-            }
-            ast::Expr::CastExpr(e) => {
-                let expr = self.collect_expr_opt(e.expr());
-                let type_ref = TypeRef::from_ast_opt(e.type_ref());
-                self.alloc_expr(Expr::Cast { expr, type_ref }, syntax_ptr)
-            }
-            ast::Expr::RefExpr(e) => {
-                let expr = self.collect_expr_opt(e.expr());
-                let mutability = Mutability::from_mutable(e.is_mut());
-                self.alloc_expr(Expr::Ref { expr, mutability }, syntax_ptr)
-            }
-            ast::Expr::PrefixExpr(e) => {
-                let expr = self.collect_expr_opt(e.expr());
-                if let Some(op) = e.op_kind() {
-                    self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr)
-                } else {
-                    self.alloc_expr(Expr::Missing, syntax_ptr)
-                }
-            }
-            ast::Expr::LambdaExpr(e) => {
-                let mut args = Vec::new();
-                let mut arg_types = Vec::new();
-                if let Some(pl) = e.param_list() {
-                    for param in pl.params() {
-                        let pat = self.collect_pat_opt(param.pat());
-                        let type_ref = param.ascribed_type().map(TypeRef::from_ast);
-                        args.push(pat);
-                        arg_types.push(type_ref);
-                    }
-                }
-                let body = self.collect_expr_opt(e.body());
-                self.alloc_expr(Expr::Lambda { args, arg_types, body }, syntax_ptr)
-            }
-            ast::Expr::BinExpr(e) => {
-                let lhs = self.collect_expr_opt(e.lhs());
-                let rhs = self.collect_expr_opt(e.rhs());
-                let op = e.op_kind().map(BinaryOp::from);
-                self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr)
-            }
-            ast::Expr::TupleExpr(e) => {
-                let exprs = e.exprs().map(|expr| self.collect_expr(expr)).collect();
-                self.alloc_expr(Expr::Tuple { exprs }, syntax_ptr)
-            }
-            ast::Expr::BoxExpr(e) => {
-                let expr = self.collect_expr_opt(e.expr());
-                self.alloc_expr(Expr::Box { expr }, syntax_ptr)
-            }
-
-            ast::Expr::ArrayExpr(e) => {
-                let kind = e.kind();
-
-                match kind {
-                    ArrayExprKind::ElementList(e) => {
-                        let exprs = e.map(|expr| self.collect_expr(expr)).collect();
-                        self.alloc_expr(Expr::Array(Array::ElementList(exprs)), syntax_ptr)
-                    }
-                    ArrayExprKind::Repeat { initializer, repeat } => {
-                        let initializer = self.collect_expr_opt(initializer);
-                        let repeat = self.collect_expr_opt(repeat);
-                        self.alloc_expr(
-                            Expr::Array(Array::Repeat { initializer, repeat }),
-                            syntax_ptr,
-                        )
-                    }
-                }
-            }
-
-            ast::Expr::Literal(e) => {
-                let lit = match e.kind() {
-                    LiteralKind::IntNumber { suffix } => {
-                        let known_name = suffix.and_then(|it| BuiltinInt::from_suffix(&it));
-
-                        Literal::Int(Default::default(), known_name)
-                    }
-                    LiteralKind::FloatNumber { suffix } => {
-                        let known_name = suffix.and_then(|it| BuiltinFloat::from_suffix(&it));
-
-                        Literal::Float(Default::default(), known_name)
-                    }
-                    LiteralKind::ByteString => Literal::ByteString(Default::default()),
-                    LiteralKind::String => Literal::String(Default::default()),
-                    LiteralKind::Byte => Literal::Int(Default::default(), Some(BuiltinInt::U8)),
-                    LiteralKind::Bool => Literal::Bool(Default::default()),
-                    LiteralKind::Char => Literal::Char(Default::default()),
-                };
-                self.alloc_expr(Expr::Literal(lit), syntax_ptr)
-            }
-            ast::Expr::IndexExpr(e) => {
-                let base = self.collect_expr_opt(e.base());
-                let index = self.collect_expr_opt(e.index());
-                self.alloc_expr(Expr::Index { base, index }, syntax_ptr)
-            }
-
-            // FIXME implement HIR for these:
-            ast::Expr::Label(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
-            ast::Expr::RangeExpr(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
-            ast::Expr::MacroCall(e) => {
-                let ast_id = AstId::new(
-                    self.current_file_id,
-                    self.db.ast_id_map(self.current_file_id).ast_id(&e),
-                );
-
-                if let Some(path) = e.path().and_then(|path| self.parse_path(path)) {
-                    if let Some(def) = self.resolver.resolve_path_as_macro(self.db, &path) {
-                        let call_id = self.db.intern_macro(MacroCallLoc { def: def.id, ast_id });
-                        let file_id = call_id.as_file(MacroFileKind::Expr);
-                        if let Some(node) = self.db.parse_or_expand(file_id) {
-                            if let Some(expr) = ast::Expr::cast(node) {
-                                log::debug!("macro expansion {:#?}", expr.syntax());
-                                let old_file_id =
-                                    std::mem::replace(&mut self.current_file_id, file_id);
-                                let id = self.collect_expr(expr);
-                                self.current_file_id = old_file_id;
-                                return id;
-                            }
-                        }
-                    }
-                }
-                // FIXME: Instead of just dropping the error from expansion
-                // report it
-                self.alloc_expr(Expr::Missing, syntax_ptr)
-            }
-        }
-    }
-
-    fn collect_expr_opt(&mut self, expr: Option<ast::Expr>) -> ExprId {
-        if let Some(expr) = expr {
-            self.collect_expr(expr)
-        } else {
-            self.missing_expr()
-        }
-    }
-
-    fn collect_block(&mut self, expr: ast::BlockExpr) -> ExprId {
-        let syntax_node_ptr = AstPtr::new(&expr.clone().into());
-        let block = match expr.block() {
-            Some(block) => block,
-            None => return self.alloc_expr(Expr::Missing, syntax_node_ptr),
-        };
-        let statements = block
-            .statements()
-            .map(|s| match s {
-                ast::Stmt::LetStmt(stmt) => {
-                    let pat = self.collect_pat_opt(stmt.pat());
-                    let type_ref = stmt.ascribed_type().map(TypeRef::from_ast);
-                    let initializer = stmt.initializer().map(|e| self.collect_expr(e));
-                    Statement::Let { pat, type_ref, initializer }
-                }
-                ast::Stmt::ExprStmt(stmt) => Statement::Expr(self.collect_expr_opt(stmt.expr())),
-            })
-            .collect();
-        let tail = block.expr().map(|e| self.collect_expr(e));
-        self.alloc_expr(Expr::Block { statements, tail }, syntax_node_ptr)
-    }
-
-    fn collect_block_opt(&mut self, expr: Option<ast::BlockExpr>) -> ExprId {
-        if let Some(block) = expr {
-            self.collect_block(block)
-        } else {
-            self.missing_expr()
-        }
-    }
-
-    fn collect_pat(&mut self, pat: ast::Pat) -> PatId {
-        let pattern = match &pat {
-            ast::Pat::BindPat(bp) => {
-                let name = bp.name().map(|nr| nr.as_name()).unwrap_or_else(Name::missing);
-                let annotation = BindingAnnotation::new(bp.is_mutable(), bp.is_ref());
-                let subpat = bp.pat().map(|subpat| self.collect_pat(subpat));
-                Pat::Bind { name, mode: annotation, subpat }
-            }
-            ast::Pat::TupleStructPat(p) => {
-                let path = p.path().and_then(|path| self.parse_path(path));
-                let args = p.args().map(|p| self.collect_pat(p)).collect();
-                Pat::TupleStruct { path, args }
-            }
-            ast::Pat::RefPat(p) => {
-                let pat = self.collect_pat_opt(p.pat());
-                let mutability = Mutability::from_mutable(p.is_mut());
-                Pat::Ref { pat, mutability }
-            }
-            ast::Pat::PathPat(p) => {
-                let path = p.path().and_then(|path| self.parse_path(path));
-                path.map(Pat::Path).unwrap_or(Pat::Missing)
-            }
-            ast::Pat::TuplePat(p) => {
-                let args = p.args().map(|p| self.collect_pat(p)).collect();
-                Pat::Tuple(args)
-            }
-            ast::Pat::PlaceholderPat(_) => Pat::Wild,
-            ast::Pat::RecordPat(p) => {
-                let path = p.path().and_then(|path| self.parse_path(path));
-                let record_field_pat_list =
-                    p.record_field_pat_list().expect("every struct should have a field list");
-                let mut fields: Vec<_> = record_field_pat_list
-                    .bind_pats()
-                    .filter_map(|bind_pat| {
-                        let ast_pat =
-                            ast::Pat::cast(bind_pat.syntax().clone()).expect("bind pat is a pat");
-                        let pat = self.collect_pat(ast_pat);
-                        let name = bind_pat.name()?.as_name();
-                        Some(RecordFieldPat { name, pat })
-                    })
-                    .collect();
-                let iter = record_field_pat_list.record_field_pats().filter_map(|f| {
-                    let ast_pat = f.pat()?;
-                    let pat = self.collect_pat(ast_pat);
-                    let name = f.name()?.as_name();
-                    Some(RecordFieldPat { name, pat })
-                });
-                fields.extend(iter);
-
-                Pat::Record { path, args: fields }
-            }
-
-            // FIXME: implement
-            ast::Pat::DotDotPat(_) => Pat::Missing,
-            ast::Pat::BoxPat(_) => Pat::Missing,
-            ast::Pat::LiteralPat(_) => Pat::Missing,
-            ast::Pat::SlicePat(_) | ast::Pat::RangePat(_) => Pat::Missing,
-        };
-        let ptr = AstPtr::new(&pat);
-        self.alloc_pat(pattern, Either::A(ptr))
-    }
-
-    fn collect_pat_opt(&mut self, pat: Option<ast::Pat>) -> PatId {
-        if let Some(pat) = pat {
-            self.collect_pat(pat)
-        } else {
-            self.missing_pat()
-        }
-    }
-
-    fn parse_path(&mut self, path: ast::Path) -> Option<Path> {
-        let hygiene = Hygiene::new(self.db, self.current_file_id);
-        Path::from_src(path, &hygiene)
-    }
-}
diff --git a/crates/ra_hir/src/marks.rs b/crates/ra_hir/src/marks.rs
index 0d4fa5b673..0f754eb9c7 100644
--- a/crates/ra_hir/src/marks.rs
+++ b/crates/ra_hir/src/marks.rs
@@ -5,6 +5,5 @@ test_utils::marks!(
     type_var_cycles_resolve_as_possible
     type_var_resolves_to_int_var
     match_ergonomics_ref
-    infer_while_let
     coerce_merge_fail_fallback
 );
diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs
index 7447904ea6..ac8f8261b8 100644
--- a/crates/ra_hir_def/src/body.rs
+++ b/crates/ra_hir_def/src/body.rs
@@ -1,2 +1,144 @@
 //! FIXME: write short doc here
 mod lower;
+
+use std::{ops::Index, sync::Arc};
+
+use hir_expand::{either::Either, HirFileId, MacroDefId, Source};
+use ra_arena::{map::ArenaMap, Arena};
+use ra_syntax::{ast, AstPtr};
+use rustc_hash::FxHashMap;
+
+use crate::{
+    db::DefDatabase2,
+    expr::{Expr, ExprId, Pat, PatId},
+    nameres::CrateDefMap,
+    path::Path,
+    ModuleId,
+};
+
+pub struct MacroResolver {
+    crate_def_map: Arc<CrateDefMap>,
+    module: ModuleId,
+}
+
+impl MacroResolver {
+    pub fn new(db: &impl DefDatabase2, module: ModuleId) -> MacroResolver {
+        MacroResolver { crate_def_map: db.crate_def_map(module.krate), module }
+    }
+
+    pub(crate) fn resolve_path_as_macro(
+        &self,
+        db: &impl DefDatabase2,
+        path: &Path,
+    ) -> Option<MacroDefId> {
+        self.crate_def_map.resolve_path(db, self.module.module_id, path).0.get_macros()
+    }
+}
+
+/// The body of an item (function, const etc.).
+#[derive(Debug, Eq, PartialEq)]
+pub struct Body {
+    exprs: Arena<ExprId, Expr>,
+    pats: Arena<PatId, Pat>,
+    /// The patterns for the function's parameters. While the parameter types are
+    /// part of the function signature, the patterns are not (they don't change
+    /// the external type of the function).
+    ///
+    /// If this `Body` is for the body of a constant, this will just be
+    /// empty.
+    params: Vec<PatId>,
+    /// The `ExprId` of the actual body expression.
+    body_expr: ExprId,
+}
+
+pub type ExprPtr = Either<AstPtr<ast::Expr>, AstPtr<ast::RecordField>>;
+pub type ExprSource = Source<ExprPtr>;
+
+pub type PatPtr = Either<AstPtr<ast::Pat>, AstPtr<ast::SelfParam>>;
+pub type PatSource = Source<PatPtr>;
+
+/// An item body together with the mapping from syntax nodes to HIR expression
+/// IDs. This is needed to go from e.g. a position in a file to the HIR
+/// expression containing it; but for type inference etc., we want to operate on
+/// a structure that is agnostic to the actual positions of expressions in the
+/// file, so that we don't recompute types whenever some whitespace is typed.
+///
+/// One complication here is that, due to macro expansion, a single `Body` might
+/// be spread across several files. So, for each ExprId and PatId, we record
+/// both the HirFileId and the position inside the file. However, we only store
+/// AST -> ExprId mapping for non-macro files, as it is not clear how to handle
+/// this properly for macros.
+#[derive(Default, Debug, Eq, PartialEq)]
+pub struct BodySourceMap {
+    expr_map: FxHashMap<ExprPtr, ExprId>,
+    expr_map_back: ArenaMap<ExprId, ExprSource>,
+    pat_map: FxHashMap<PatPtr, PatId>,
+    pat_map_back: ArenaMap<PatId, PatSource>,
+    field_map: FxHashMap<(ExprId, usize), AstPtr<ast::RecordField>>,
+}
+
+impl Body {
+    pub fn new(
+        db: &impl DefDatabase2,
+        resolver: MacroResolver,
+        file_id: HirFileId,
+        params: Option<ast::ParamList>,
+        body: Option<ast::Expr>,
+    ) -> (Body, BodySourceMap) {
+        lower::lower(db, resolver, file_id, params, body)
+    }
+
+    pub fn params(&self) -> &[PatId] {
+        &self.params
+    }
+
+    pub fn body_expr(&self) -> ExprId {
+        self.body_expr
+    }
+
+    pub fn exprs(&self) -> impl Iterator<Item = (ExprId, &Expr)> {
+        self.exprs.iter()
+    }
+
+    pub fn pats(&self) -> impl Iterator<Item = (PatId, &Pat)> {
+        self.pats.iter()
+    }
+}
+
+impl Index<ExprId> for Body {
+    type Output = Expr;
+
+    fn index(&self, expr: ExprId) -> &Expr {
+        &self.exprs[expr]
+    }
+}
+
+impl Index<PatId> for Body {
+    type Output = Pat;
+
+    fn index(&self, pat: PatId) -> &Pat {
+        &self.pats[pat]
+    }
+}
+
+impl BodySourceMap {
+    pub fn expr_syntax(&self, expr: ExprId) -> Option<ExprSource> {
+        self.expr_map_back.get(expr).copied()
+    }
+
+    pub fn node_expr(&self, node: &ast::Expr) -> Option<ExprId> {
+        self.expr_map.get(&Either::A(AstPtr::new(node))).cloned()
+    }
+
+    pub fn pat_syntax(&self, pat: PatId) -> Option<PatSource> {
+        self.pat_map_back.get(pat).copied()
+    }
+
+    pub fn node_pat(&self, node: &ast::Pat) -> Option<PatId> {
+        self.pat_map.get(&Either::A(AstPtr::new(node))).cloned()
+    }
+
+    pub fn field_syntax(&self, expr: ExprId, field: usize) -> AstPtr<ast::RecordField> {
+        self.field_map[&(expr, field)]
+    }
+}
diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index 1a144b1f91..2aa863c9ec 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -1,8 +1,590 @@
 //! FIXME: write short doc here
 
-use ra_syntax::ast;
+use hir_expand::{
+    either::Either,
+    hygiene::Hygiene,
+    name::{self, AsName, Name},
+    AstId, HirFileId, MacroCallLoc, MacroFileKind, Source,
+};
+use ra_arena::Arena;
+use ra_syntax::{
+    ast::{
+        self, ArgListOwner, ArrayExprKind, LiteralKind, LoopBodyOwner, NameOwner,
+        TypeAscriptionOwner,
+    },
+    AstNode, AstPtr,
+};
 
-use crate::expr::{ArithOp, BinaryOp, CmpOp, LogicOp, Ordering};
+use crate::{
+    body::{Body, BodySourceMap, MacroResolver, PatPtr},
+    builtin_type::{BuiltinFloat, BuiltinInt},
+    db::DefDatabase2,
+    expr::{
+        ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp,
+        MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement,
+    },
+    path::GenericArgs,
+    path::Path,
+    type_ref::{Mutability, TypeRef},
+};
+
+pub(super) fn lower(
+    db: &impl DefDatabase2,
+    resolver: MacroResolver,
+    file_id: HirFileId,
+    params: Option<ast::ParamList>,
+    body: Option<ast::Expr>,
+) -> (Body, BodySourceMap) {
+    ExprCollector {
+        resolver,
+        db,
+        original_file_id: file_id,
+        current_file_id: file_id,
+        source_map: BodySourceMap::default(),
+        body: Body {
+            exprs: Arena::default(),
+            pats: Arena::default(),
+            params: Vec::new(),
+            body_expr: ExprId::dummy(),
+        },
+    }
+    .collect(params, body)
+}
+
+struct ExprCollector<DB> {
+    db: DB,
+    resolver: MacroResolver,
+    original_file_id: HirFileId,
+    current_file_id: HirFileId,
+
+    body: Body,
+    source_map: BodySourceMap,
+}
+
+impl<'a, DB> ExprCollector<&'a DB>
+where
+    DB: DefDatabase2,
+{
+    fn collect(
+        mut self,
+        param_list: Option<ast::ParamList>,
+        body: Option<ast::Expr>,
+    ) -> (Body, BodySourceMap) {
+        if let Some(param_list) = param_list {
+            if let Some(self_param) = param_list.self_param() {
+                let ptr = AstPtr::new(&self_param);
+                let param_pat = self.alloc_pat(
+                    Pat::Bind {
+                        name: name::SELF_PARAM,
+                        mode: BindingAnnotation::Unannotated,
+                        subpat: None,
+                    },
+                    Either::B(ptr),
+                );
+                self.body.params.push(param_pat);
+            }
+
+            for param in param_list.params() {
+                let pat = match param.pat() {
+                    None => continue,
+                    Some(pat) => pat,
+                };
+                let param_pat = self.collect_pat(pat);
+                self.body.params.push(param_pat);
+            }
+        };
+
+        self.body.body_expr = self.collect_expr_opt(body);
+        (self.body, self.source_map)
+    }
+
+    fn alloc_expr(&mut self, expr: Expr, ptr: AstPtr<ast::Expr>) -> ExprId {
+        let ptr = Either::A(ptr);
+        let id = self.body.exprs.alloc(expr);
+        if self.current_file_id == self.original_file_id {
+            self.source_map.expr_map.insert(ptr, id);
+        }
+        self.source_map
+            .expr_map_back
+            .insert(id, Source { file_id: self.current_file_id, ast: ptr });
+        id
+    }
+    // desugared exprs don't have ptr, that's wrong and should be fixed
+    // somehow.
+    fn alloc_expr_desugared(&mut self, expr: Expr) -> ExprId {
+        self.body.exprs.alloc(expr)
+    }
+    fn alloc_expr_field_shorthand(&mut self, expr: Expr, ptr: AstPtr<ast::RecordField>) -> ExprId {
+        let ptr = Either::B(ptr);
+        let id = self.body.exprs.alloc(expr);
+        if self.current_file_id == self.original_file_id {
+            self.source_map.expr_map.insert(ptr, id);
+        }
+        self.source_map
+            .expr_map_back
+            .insert(id, Source { file_id: self.current_file_id, ast: ptr });
+        id
+    }
+    fn alloc_pat(&mut self, pat: Pat, ptr: PatPtr) -> PatId {
+        let id = self.body.pats.alloc(pat);
+        if self.current_file_id == self.original_file_id {
+            self.source_map.pat_map.insert(ptr, id);
+        }
+        self.source_map.pat_map_back.insert(id, Source { file_id: self.current_file_id, ast: ptr });
+        id
+    }
+
+    fn empty_block(&mut self) -> ExprId {
+        let block = Expr::Block { statements: Vec::new(), tail: None };
+        self.body.exprs.alloc(block)
+    }
+
+    fn missing_expr(&mut self) -> ExprId {
+        self.body.exprs.alloc(Expr::Missing)
+    }
+
+    fn missing_pat(&mut self) -> PatId {
+        self.body.pats.alloc(Pat::Missing)
+    }
+
+    fn collect_expr(&mut self, expr: ast::Expr) -> ExprId {
+        let syntax_ptr = AstPtr::new(&expr);
+        match expr {
+            ast::Expr::IfExpr(e) => {
+                let then_branch = self.collect_block_opt(e.then_branch());
+
+                let else_branch = e.else_branch().map(|b| match b {
+                    ast::ElseBranch::Block(it) => self.collect_block(it),
+                    ast::ElseBranch::IfExpr(elif) => {
+                        let expr: ast::Expr = ast::Expr::cast(elif.syntax().clone()).unwrap();
+                        self.collect_expr(expr)
+                    }
+                });
+
+                let condition = match e.condition() {
+                    None => self.missing_expr(),
+                    Some(condition) => match condition.pat() {
+                        None => self.collect_expr_opt(condition.expr()),
+                        // if let -- desugar to match
+                        Some(pat) => {
+                            let pat = self.collect_pat(pat);
+                            let match_expr = self.collect_expr_opt(condition.expr());
+                            let placeholder_pat = self.missing_pat();
+                            let arms = vec![
+                                MatchArm { pats: vec![pat], expr: then_branch, guard: None },
+                                MatchArm {
+                                    pats: vec![placeholder_pat],
+                                    expr: else_branch.unwrap_or_else(|| self.empty_block()),
+                                    guard: None,
+                                },
+                            ];
+                            return self
+                                .alloc_expr(Expr::Match { expr: match_expr, arms }, syntax_ptr);
+                        }
+                    },
+                };
+
+                self.alloc_expr(Expr::If { condition, then_branch, else_branch }, syntax_ptr)
+            }
+            ast::Expr::TryBlockExpr(e) => {
+                let body = self.collect_block_opt(e.body());
+                self.alloc_expr(Expr::TryBlock { body }, syntax_ptr)
+            }
+            ast::Expr::BlockExpr(e) => self.collect_block(e),
+            ast::Expr::LoopExpr(e) => {
+                let body = self.collect_block_opt(e.loop_body());
+                self.alloc_expr(Expr::Loop { body }, syntax_ptr)
+            }
+            ast::Expr::WhileExpr(e) => {
+                let body = self.collect_block_opt(e.loop_body());
+
+                let condition = match e.condition() {
+                    None => self.missing_expr(),
+                    Some(condition) => match condition.pat() {
+                        None => self.collect_expr_opt(condition.expr()),
+                        // if let -- desugar to match
+                        Some(pat) => {
+                            let pat = self.collect_pat(pat);
+                            let match_expr = self.collect_expr_opt(condition.expr());
+                            let placeholder_pat = self.missing_pat();
+                            let break_ = self.alloc_expr_desugared(Expr::Break { expr: None });
+                            let arms = vec![
+                                MatchArm { pats: vec![pat], expr: body, guard: None },
+                                MatchArm { pats: vec![placeholder_pat], expr: break_, guard: None },
+                            ];
+                            let match_expr =
+                                self.alloc_expr_desugared(Expr::Match { expr: match_expr, arms });
+                            return self.alloc_expr(Expr::Loop { body: match_expr }, syntax_ptr);
+                        }
+                    },
+                };
+
+                self.alloc_expr(Expr::While { condition, body }, syntax_ptr)
+            }
+            ast::Expr::ForExpr(e) => {
+                let iterable = self.collect_expr_opt(e.iterable());
+                let pat = self.collect_pat_opt(e.pat());
+                let body = self.collect_block_opt(e.loop_body());
+                self.alloc_expr(Expr::For { iterable, pat, body }, syntax_ptr)
+            }
+            ast::Expr::CallExpr(e) => {
+                let callee = self.collect_expr_opt(e.expr());
+                let args = if let Some(arg_list) = e.arg_list() {
+                    arg_list.args().map(|e| self.collect_expr(e)).collect()
+                } else {
+                    Vec::new()
+                };
+                self.alloc_expr(Expr::Call { callee, args }, syntax_ptr)
+            }
+            ast::Expr::MethodCallExpr(e) => {
+                let receiver = self.collect_expr_opt(e.expr());
+                let args = if let Some(arg_list) = e.arg_list() {
+                    arg_list.args().map(|e| self.collect_expr(e)).collect()
+                } else {
+                    Vec::new()
+                };
+                let method_name = e.name_ref().map(|nr| nr.as_name()).unwrap_or_else(Name::missing);
+                let generic_args = e.type_arg_list().and_then(GenericArgs::from_ast);
+                self.alloc_expr(
+                    Expr::MethodCall { receiver, method_name, args, generic_args },
+                    syntax_ptr,
+                )
+            }
+            ast::Expr::MatchExpr(e) => {
+                let expr = self.collect_expr_opt(e.expr());
+                let arms = if let Some(match_arm_list) = e.match_arm_list() {
+                    match_arm_list
+                        .arms()
+                        .map(|arm| MatchArm {
+                            pats: arm.pats().map(|p| self.collect_pat(p)).collect(),
+                            expr: self.collect_expr_opt(arm.expr()),
+                            guard: arm
+                                .guard()
+                                .and_then(|guard| guard.expr())
+                                .map(|e| self.collect_expr(e)),
+                        })
+                        .collect()
+                } else {
+                    Vec::new()
+                };
+                self.alloc_expr(Expr::Match { expr, arms }, syntax_ptr)
+            }
+            ast::Expr::PathExpr(e) => {
+                let path = e
+                    .path()
+                    .and_then(|path| self.parse_path(path))
+                    .map(Expr::Path)
+                    .unwrap_or(Expr::Missing);
+                self.alloc_expr(path, syntax_ptr)
+            }
+            ast::Expr::ContinueExpr(_e) => {
+                // FIXME: labels
+                self.alloc_expr(Expr::Continue, syntax_ptr)
+            }
+            ast::Expr::BreakExpr(e) => {
+                let expr = e.expr().map(|e| self.collect_expr(e));
+                self.alloc_expr(Expr::Break { expr }, syntax_ptr)
+            }
+            ast::Expr::ParenExpr(e) => {
+                let inner = self.collect_expr_opt(e.expr());
+                // make the paren expr point to the inner expression as well
+                self.source_map.expr_map.insert(Either::A(syntax_ptr), inner);
+                inner
+            }
+            ast::Expr::ReturnExpr(e) => {
+                let expr = e.expr().map(|e| self.collect_expr(e));
+                self.alloc_expr(Expr::Return { expr }, syntax_ptr)
+            }
+            ast::Expr::RecordLit(e) => {
+                let path = e.path().and_then(|path| self.parse_path(path));
+                let mut field_ptrs = Vec::new();
+                let record_lit = if let Some(nfl) = e.record_field_list() {
+                    let fields = nfl
+                        .fields()
+                        .inspect(|field| field_ptrs.push(AstPtr::new(field)))
+                        .map(|field| RecordLitField {
+                            name: field
+                                .name_ref()
+                                .map(|nr| nr.as_name())
+                                .unwrap_or_else(Name::missing),
+                            expr: if let Some(e) = field.expr() {
+                                self.collect_expr(e)
+                            } else if let Some(nr) = field.name_ref() {
+                                // field shorthand
+                                self.alloc_expr_field_shorthand(
+                                    Expr::Path(Path::from_name_ref(&nr)),
+                                    AstPtr::new(&field),
+                                )
+                            } else {
+                                self.missing_expr()
+                            },
+                        })
+                        .collect();
+                    let spread = nfl.spread().map(|s| self.collect_expr(s));
+                    Expr::RecordLit { path, fields, spread }
+                } else {
+                    Expr::RecordLit { path, fields: Vec::new(), spread: None }
+                };
+
+                let res = self.alloc_expr(record_lit, syntax_ptr);
+                for (i, ptr) in field_ptrs.into_iter().enumerate() {
+                    self.source_map.field_map.insert((res, i), ptr);
+                }
+                res
+            }
+            ast::Expr::FieldExpr(e) => {
+                let expr = self.collect_expr_opt(e.expr());
+                let name = match e.field_access() {
+                    Some(kind) => kind.as_name(),
+                    _ => Name::missing(),
+                };
+                self.alloc_expr(Expr::Field { expr, name }, syntax_ptr)
+            }
+            ast::Expr::AwaitExpr(e) => {
+                let expr = self.collect_expr_opt(e.expr());
+                self.alloc_expr(Expr::Await { expr }, syntax_ptr)
+            }
+            ast::Expr::TryExpr(e) => {
+                let expr = self.collect_expr_opt(e.expr());
+                self.alloc_expr(Expr::Try { expr }, syntax_ptr)
+            }
+            ast::Expr::CastExpr(e) => {
+                let expr = self.collect_expr_opt(e.expr());
+                let type_ref = TypeRef::from_ast_opt(e.type_ref());
+                self.alloc_expr(Expr::Cast { expr, type_ref }, syntax_ptr)
+            }
+            ast::Expr::RefExpr(e) => {
+                let expr = self.collect_expr_opt(e.expr());
+                let mutability = Mutability::from_mutable(e.is_mut());
+                self.alloc_expr(Expr::Ref { expr, mutability }, syntax_ptr)
+            }
+            ast::Expr::PrefixExpr(e) => {
+                let expr = self.collect_expr_opt(e.expr());
+                if let Some(op) = e.op_kind() {
+                    self.alloc_expr(Expr::UnaryOp { expr, op }, syntax_ptr)
+                } else {
+                    self.alloc_expr(Expr::Missing, syntax_ptr)
+                }
+            }
+            ast::Expr::LambdaExpr(e) => {
+                let mut args = Vec::new();
+                let mut arg_types = Vec::new();
+                if let Some(pl) = e.param_list() {
+                    for param in pl.params() {
+                        let pat = self.collect_pat_opt(param.pat());
+                        let type_ref = param.ascribed_type().map(TypeRef::from_ast);
+                        args.push(pat);
+                        arg_types.push(type_ref);
+                    }
+                }
+                let body = self.collect_expr_opt(e.body());
+                self.alloc_expr(Expr::Lambda { args, arg_types, body }, syntax_ptr)
+            }
+            ast::Expr::BinExpr(e) => {
+                let lhs = self.collect_expr_opt(e.lhs());
+                let rhs = self.collect_expr_opt(e.rhs());
+                let op = e.op_kind().map(BinaryOp::from);
+                self.alloc_expr(Expr::BinaryOp { lhs, rhs, op }, syntax_ptr)
+            }
+            ast::Expr::TupleExpr(e) => {
+                let exprs = e.exprs().map(|expr| self.collect_expr(expr)).collect();
+                self.alloc_expr(Expr::Tuple { exprs }, syntax_ptr)
+            }
+            ast::Expr::BoxExpr(e) => {
+                let expr = self.collect_expr_opt(e.expr());
+                self.alloc_expr(Expr::Box { expr }, syntax_ptr)
+            }
+
+            ast::Expr::ArrayExpr(e) => {
+                let kind = e.kind();
+
+                match kind {
+                    ArrayExprKind::ElementList(e) => {
+                        let exprs = e.map(|expr| self.collect_expr(expr)).collect();
+                        self.alloc_expr(Expr::Array(Array::ElementList(exprs)), syntax_ptr)
+                    }
+                    ArrayExprKind::Repeat { initializer, repeat } => {
+                        let initializer = self.collect_expr_opt(initializer);
+                        let repeat = self.collect_expr_opt(repeat);
+                        self.alloc_expr(
+                            Expr::Array(Array::Repeat { initializer, repeat }),
+                            syntax_ptr,
+                        )
+                    }
+                }
+            }
+
+            ast::Expr::Literal(e) => {
+                let lit = match e.kind() {
+                    LiteralKind::IntNumber { suffix } => {
+                        let known_name = suffix.and_then(|it| BuiltinInt::from_suffix(&it));
+
+                        Literal::Int(Default::default(), known_name)
+                    }
+                    LiteralKind::FloatNumber { suffix } => {
+                        let known_name = suffix.and_then(|it| BuiltinFloat::from_suffix(&it));
+
+                        Literal::Float(Default::default(), known_name)
+                    }
+                    LiteralKind::ByteString => Literal::ByteString(Default::default()),
+                    LiteralKind::String => Literal::String(Default::default()),
+                    LiteralKind::Byte => Literal::Int(Default::default(), Some(BuiltinInt::U8)),
+                    LiteralKind::Bool => Literal::Bool(Default::default()),
+                    LiteralKind::Char => Literal::Char(Default::default()),
+                };
+                self.alloc_expr(Expr::Literal(lit), syntax_ptr)
+            }
+            ast::Expr::IndexExpr(e) => {
+                let base = self.collect_expr_opt(e.base());
+                let index = self.collect_expr_opt(e.index());
+                self.alloc_expr(Expr::Index { base, index }, syntax_ptr)
+            }
+
+            // FIXME implement HIR for these:
+            ast::Expr::Label(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
+            ast::Expr::RangeExpr(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
+            ast::Expr::MacroCall(e) => {
+                let ast_id = AstId::new(
+                    self.current_file_id,
+                    self.db.ast_id_map(self.current_file_id).ast_id(&e),
+                );
+
+                if let Some(path) = e.path().and_then(|path| self.parse_path(path)) {
+                    if let Some(def) = self.resolver.resolve_path_as_macro(self.db, &path) {
+                        let call_id = self.db.intern_macro(MacroCallLoc { def, ast_id });
+                        let file_id = call_id.as_file(MacroFileKind::Expr);
+                        if let Some(node) = self.db.parse_or_expand(file_id) {
+                            if let Some(expr) = ast::Expr::cast(node) {
+                                log::debug!("macro expansion {:#?}", expr.syntax());
+                                let old_file_id =
+                                    std::mem::replace(&mut self.current_file_id, file_id);
+                                let id = self.collect_expr(expr);
+                                self.current_file_id = old_file_id;
+                                return id;
+                            }
+                        }
+                    }
+                }
+                // FIXME: Instead of just dropping the error from expansion
+                // report it
+                self.alloc_expr(Expr::Missing, syntax_ptr)
+            }
+        }
+    }
+
+    fn collect_expr_opt(&mut self, expr: Option<ast::Expr>) -> ExprId {
+        if let Some(expr) = expr {
+            self.collect_expr(expr)
+        } else {
+            self.missing_expr()
+        }
+    }
+
+    fn collect_block(&mut self, expr: ast::BlockExpr) -> ExprId {
+        let syntax_node_ptr = AstPtr::new(&expr.clone().into());
+        let block = match expr.block() {
+            Some(block) => block,
+            None => return self.alloc_expr(Expr::Missing, syntax_node_ptr),
+        };
+        let statements = block
+            .statements()
+            .map(|s| match s {
+                ast::Stmt::LetStmt(stmt) => {
+                    let pat = self.collect_pat_opt(stmt.pat());
+                    let type_ref = stmt.ascribed_type().map(TypeRef::from_ast);
+                    let initializer = stmt.initializer().map(|e| self.collect_expr(e));
+                    Statement::Let { pat, type_ref, initializer }
+                }
+                ast::Stmt::ExprStmt(stmt) => Statement::Expr(self.collect_expr_opt(stmt.expr())),
+            })
+            .collect();
+        let tail = block.expr().map(|e| self.collect_expr(e));
+        self.alloc_expr(Expr::Block { statements, tail }, syntax_node_ptr)
+    }
+
+    fn collect_block_opt(&mut self, expr: Option<ast::BlockExpr>) -> ExprId {
+        if let Some(block) = expr {
+            self.collect_block(block)
+        } else {
+            self.missing_expr()
+        }
+    }
+
+    fn collect_pat(&mut self, pat: ast::Pat) -> PatId {
+        let pattern = match &pat {
+            ast::Pat::BindPat(bp) => {
+                let name = bp.name().map(|nr| nr.as_name()).unwrap_or_else(Name::missing);
+                let annotation = BindingAnnotation::new(bp.is_mutable(), bp.is_ref());
+                let subpat = bp.pat().map(|subpat| self.collect_pat(subpat));
+                Pat::Bind { name, mode: annotation, subpat }
+            }
+            ast::Pat::TupleStructPat(p) => {
+                let path = p.path().and_then(|path| self.parse_path(path));
+                let args = p.args().map(|p| self.collect_pat(p)).collect();
+                Pat::TupleStruct { path, args }
+            }
+            ast::Pat::RefPat(p) => {
+                let pat = self.collect_pat_opt(p.pat());
+                let mutability = Mutability::from_mutable(p.is_mut());
+                Pat::Ref { pat, mutability }
+            }
+            ast::Pat::PathPat(p) => {
+                let path = p.path().and_then(|path| self.parse_path(path));
+                path.map(Pat::Path).unwrap_or(Pat::Missing)
+            }
+            ast::Pat::TuplePat(p) => {
+                let args = p.args().map(|p| self.collect_pat(p)).collect();
+                Pat::Tuple(args)
+            }
+            ast::Pat::PlaceholderPat(_) => Pat::Wild,
+            ast::Pat::RecordPat(p) => {
+                let path = p.path().and_then(|path| self.parse_path(path));
+                let record_field_pat_list =
+                    p.record_field_pat_list().expect("every struct should have a field list");
+                let mut fields: Vec<_> = record_field_pat_list
+                    .bind_pats()
+                    .filter_map(|bind_pat| {
+                        let ast_pat =
+                            ast::Pat::cast(bind_pat.syntax().clone()).expect("bind pat is a pat");
+                        let pat = self.collect_pat(ast_pat);
+                        let name = bind_pat.name()?.as_name();
+                        Some(RecordFieldPat { name, pat })
+                    })
+                    .collect();
+                let iter = record_field_pat_list.record_field_pats().filter_map(|f| {
+                    let ast_pat = f.pat()?;
+                    let pat = self.collect_pat(ast_pat);
+                    let name = f.name()?.as_name();
+                    Some(RecordFieldPat { name, pat })
+                });
+                fields.extend(iter);
+
+                Pat::Record { path, args: fields }
+            }
+
+            // FIXME: implement
+            ast::Pat::DotDotPat(_) => Pat::Missing,
+            ast::Pat::BoxPat(_) => Pat::Missing,
+            ast::Pat::LiteralPat(_) => Pat::Missing,
+            ast::Pat::SlicePat(_) | ast::Pat::RangePat(_) => Pat::Missing,
+        };
+        let ptr = AstPtr::new(&pat);
+        self.alloc_pat(pattern, Either::A(ptr))
+    }
+
+    fn collect_pat_opt(&mut self, pat: Option<ast::Pat>) -> PatId {
+        if let Some(pat) = pat {
+            self.collect_pat(pat)
+        } else {
+            self.missing_pat()
+        }
+    }
+
+    fn parse_path(&mut self, path: ast::Path) -> Option<Path> {
+        let hygiene = Hygiene::new(self.db, self.current_file_id);
+        Path::from_src(path, &hygiene)
+    }
+}
 
 impl From<ast::BinOp> for BinaryOp {
     fn from(ast_op: ast::BinOp) -> Self {
diff --git a/crates/ra_hir_def/src/expr.rs b/crates/ra_hir_def/src/expr.rs
index 12eb0da68b..04c1d8f69b 100644
--- a/crates/ra_hir_def/src/expr.rs
+++ b/crates/ra_hir_def/src/expr.rs
@@ -9,6 +9,8 @@
 //! 3. Unresolved. Paths are stored as sequences of names, and not as defs the
 //!    names refer to.
 //! 4. Desugared. There's no `if let`.
+//!
+//! See also a neighboring `body` module.
 
 use hir_expand::name::Name;
 use ra_arena::{impl_arena_id, RawId};
diff --git a/crates/ra_project_model/src/cargo_workspace.rs b/crates/ra_project_model/src/cargo_workspace.rs
index cf88911b75..28dadea9d7 100644
--- a/crates/ra_project_model/src/cargo_workspace.rs
+++ b/crates/ra_project_model/src/cargo_workspace.rs
@@ -1,7 +1,6 @@
 //! FIXME: write short doc here
 
 use std::path::{Path, PathBuf};
-use std::str::FromStr;
 
 use cargo_metadata::{CargoOpt, MetadataCommand};
 use ra_arena::{impl_arena_id, Arena, RawId};
@@ -141,21 +140,18 @@ impl CargoWorkspace {
         let ws_members = &meta.workspace_members;
 
         for meta_pkg in meta.packages {
-            let cargo_metadata::Package { id, edition, name, manifest_path, .. } = meta_pkg;
-            let is_member = ws_members.contains(&id);
-            let edition = Edition::from_str(&edition)
-                .map_err(|e| (format!("metadata for package {} failed: {}", &name, e.msg)))?;
+            let is_member = ws_members.contains(&meta_pkg.id);
             let pkg = packages.alloc(PackageData {
-                name,
-                manifest: manifest_path,
+                name: meta_pkg.name,
+                manifest: meta_pkg.manifest_path.clone(),
                 targets: Vec::new(),
                 is_member,
-                edition,
+                edition: Edition::from_string(&meta_pkg.edition),
                 dependencies: Vec::new(),
                 features: Vec::new(),
             });
             let pkg_data = &mut packages[pkg];
-            pkg_by_id.insert(id, pkg);
+            pkg_by_id.insert(meta_pkg.id.clone(), pkg);
             for meta_tgt in meta_pkg.targets {
                 let tgt = targets.alloc(TargetData {
                     pkg,


Test patch:
diff --git a/crates/ra_hir/src/ty/tests.rs b/crates/ra_hir/src/ty/tests.rs
index 896bf2924b..8863c36087 100644
--- a/crates/ra_hir/src/ty/tests.rs
+++ b/crates/ra_hir/src/ty/tests.rs
@@ -222,7 +222,6 @@ mod collections {
 
 #[test]
 fn infer_while_let() {
-    covers!(infer_while_let);
     let (db, pos) = TestDB::with_position(
         r#"
 //- /main.rs
@@ -4825,7 +4824,7 @@ fn main() {
         @r###"
         ![0; 1) '6': i32
         [64; 88) '{     ...!(); }': ()
-        [74; 75) 'x': i32        
+        [74; 75) 'x': i32
     "###
     );
 }