Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #22864 from Veykril/lukaswirth/push-kswvmlyvmvsv
internal: Port `ExprScopes` over to visitor fully
| -rw-r--r-- | crates/hir-def/src/expr_store/scope.rs | 390 | ||||
| -rw-r--r-- | crates/hir-def/src/item_scope.rs | 2 |
2 files changed, 227 insertions, 165 deletions
diff --git a/crates/hir-def/src/expr_store/scope.rs b/crates/hir-def/src/expr_store/scope.rs index d6568b8cfe..ee5396b4cd 100644 --- a/crates/hir-def/src/expr_store/scope.rs +++ b/crates/hir-def/src/expr_store/scope.rs @@ -5,9 +5,9 @@ use la_arena::{Arena, ArenaMap, Idx, IdxRange, RawIdx}; use crate::{ BlockId, DefWithBodyId, ExpressionStoreOwnerId, GenericDefId, VariantId, - expr_store::{Body, ExpressionStore, HygieneId, StoreVisitor, body::Param}, + expr_store::{Body, ExpressionStore, HygieneId, StoreVisitor, StoreVisitorExt, body::Param}, hir::{ - Array, Binding, BindingId, Expr, ExprId, Item, LabelId, Pat, PatId, Statement, + Binding, BindingId, Expr, ExprId, Item, LabelId, Pat, PatId, Statement, generics::GenericParams, }, signatures::VariantFields, @@ -167,7 +167,13 @@ impl ExprScopes { scopes.add_bindings(body, root, self_param, body.binding_hygiene(self_param)); } body.params.iter().for_each(|param| scopes.add_pat_bindings(body, root, param.formal)); - compute_expr_scopes(body.root_expr(), body, &mut scopes, &mut { root }, &mut root); + ExprScopeVisitor { + store: body, + scopes: &mut scopes, + scope: &mut { root }, + const_scope: &mut root, + } + .on_expr(body.root_expr()); scopes } @@ -182,7 +188,13 @@ impl ExprScopes { let root = scopes.root_scope(); for root_expr in roots { let mut scope = scopes.new_scope(root); - compute_expr_scopes(root_expr, store, &mut scopes, &mut { scope }, &mut scope); + ExprScopeVisitor { + store, + scopes: &mut scopes, + scope: &mut { scope }, + const_scope: &mut scope, + } + .on_expr(root_expr); } scopes } @@ -282,174 +294,161 @@ struct ExprScopeVisitor<'a> { const_scope: &'a mut ScopeId, } -impl StoreVisitor for ExprScopeVisitor<'_> { - fn on_expr(&mut self, expr: ExprId) { - compute_expr_scopes(expr, self.store, self.scopes, self.scope, self.const_scope); - } - - fn on_anon_const_expr(&mut self, expr: ExprId) { - let mut scope = *self.const_scope; - compute_expr_scopes(expr, self.store, self.scopes, &mut scope, self.const_scope); - } - - fn on_pat(&mut self, pat: PatId) { - self.store.visit_pat_children(pat, &mut *self); - } +impl ExprScopeVisitor<'_> { + fn visit_block( + &mut self, + expr: ExprId, + id: Option<BlockId>, + statements: &[Statement], + tail: Option<ExprId>, + label: Option<LabelId>, + ) { + let mut scope = self.scopes.new_block_scope(*self.scope, id, label); + let mut const_scope = if id.is_some() { + self.scopes.new_block_scope(*self.const_scope, id, None) + } else { + // We don't need to allocate a new scope, since only items matter to us. + *self.const_scope + }; + // Overwrite the old scope for the block expr, so that every block scope can be found + // via the block itself (important for blocks that only contain items, no expressions). + self.scopes.set_scope(expr, scope); - fn on_type(&mut self, ty: TypeRefId) { - self.store.visit_type_ref_children(ty, &mut *self); + let mut visitor = ExprScopeVisitor { + store: self.store, + scopes: self.scopes, + scope: &mut scope, + const_scope: &mut const_scope, + }; + for stmt in statements { + match stmt { + Statement::Let { pat, initializer, else_branch, type_ref } => { + visitor.on_type_opt(*type_ref); + visitor.on_expr_opt(*initializer); + visitor.on_expr_opt(*else_branch); + *visitor.scope = visitor.scopes.new_scope(*visitor.scope); + visitor.scopes.add_pat_bindings(visitor.store, *visitor.scope, *pat); + } + Statement::Expr { expr, has_semi: _ } => visitor.on_expr(*expr), + Statement::Item(Item::MacroDef(macro_id)) => { + *visitor.scope = + visitor.scopes.new_macro_def_scope(*visitor.scope, macro_id.clone()); + *visitor.const_scope = + visitor.scopes.new_macro_def_scope(*visitor.const_scope, macro_id.clone()); + } + Statement::Item(Item::Other) => (), + } + } + visitor.on_expr_opt(tail); } } -fn compute_type_scopes( - ty: TypeRefId, - store: &ExpressionStore, - scopes: &mut ExprScopes, - const_scope: &mut ScopeId, -) { - let mut scope = *const_scope; - ExprScopeVisitor { store, scopes, scope: &mut scope, const_scope }.on_type(ty); -} - -fn compute_block_scopes( - statements: &[Statement], - tail: Option<ExprId>, - store: &ExpressionStore, - scopes: &mut ExprScopes, - scope: &mut ScopeId, - const_scope: &mut ScopeId, -) { - for stmt in statements { - match stmt { - Statement::Let { pat, initializer, else_branch, type_ref } => { - if let Some(type_ref) = type_ref { - compute_type_scopes(*type_ref, store, scopes, const_scope); +impl StoreVisitor for ExprScopeVisitor<'_> { + fn on_expr(&mut self, expr: ExprId) { + self.scopes.set_scope(expr, *self.scope); + match &self.store[expr] { + Expr::Block { statements, tail, id, label } => { + self.visit_block(expr, *id, statements, *tail, *label); + } + Expr::Const(expr) => self.on_anon_const_expr(*expr), + Expr::Unsafe { id, statements, tail } => { + self.visit_block(expr, *id, statements, *tail, None); + } + Expr::Loop { body, label, source: _ } => { + let mut scope = self.scopes.new_labeled_scope(*self.scope, *label); + ExprScopeVisitor { + store: self.store, + scopes: self.scopes, + scope: &mut scope, + const_scope: self.const_scope, } - if let Some(expr) = initializer { - compute_expr_scopes(*expr, store, scopes, scope, const_scope); + .on_expr(*body); + } + Expr::Closure { args, arg_types, ret_type, body, capture_by: _, closure_kind: _ } => { + arg_types.iter().flatten().for_each(|type_ref| self.on_type(*type_ref)); + self.on_type_opt(*ret_type); + let mut scope = self.scopes.new_scope(*self.scope); + args.iter().for_each(|arg| self.scopes.add_pat_bindings(self.store, scope, *arg)); + ExprScopeVisitor { + store: self.store, + scopes: self.scopes, + scope: &mut scope, + const_scope: self.const_scope, } - if let Some(expr) = else_branch { - compute_expr_scopes(*expr, store, scopes, scope, const_scope); + .on_expr(*body); + } + Expr::Match { expr, arms } => { + self.on_expr(*expr); + for arm in arms.iter() { + let mut scope = self.scopes.new_scope(*self.scope); + self.scopes.add_pat_bindings(self.store, scope, arm.pat); + if let Some(guard) = arm.guard { + scope = self.scopes.new_scope(scope); + ExprScopeVisitor { + store: self.store, + scopes: self.scopes, + scope: &mut scope, + const_scope: self.const_scope, + } + .on_expr(guard); + } + ExprScopeVisitor { + store: self.store, + scopes: self.scopes, + scope: &mut scope, + const_scope: self.const_scope, + } + .on_expr(arm.expr); } - - *scope = scopes.new_scope(*scope); - scopes.add_pat_bindings(store, *scope, *pat); } - Statement::Expr { expr, .. } => { - compute_expr_scopes(*expr, store, scopes, scope, const_scope); + &Expr::If { condition, then_branch, else_branch } => { + let mut then_branch_scope = self.scopes.new_scope(*self.scope); + let mut visitor = ExprScopeVisitor { + store: self.store, + scopes: self.scopes, + scope: &mut then_branch_scope, + const_scope: self.const_scope, + }; + visitor.on_expr(condition); + visitor.on_expr(then_branch); + self.on_expr_opt(else_branch); } - Statement::Item(Item::MacroDef(macro_id)) => { - *scope = scopes.new_macro_def_scope(*scope, macro_id.clone()); - *const_scope = scopes.new_macro_def_scope(*const_scope, macro_id.clone()); + &Expr::Let { pat, expr } => { + self.on_expr(expr); + *self.scope = self.scopes.new_scope(*self.scope); + self.scopes.add_pat_bindings(self.store, *self.scope, pat); } - Statement::Item(Item::Other) => (), + _ => self.store.visit_expr_children(expr, &mut *self), } } - if let Some(expr) = tail { - compute_expr_scopes(expr, store, scopes, scope, const_scope); + + fn on_anon_const_expr(&mut self, expr: ExprId) { + let mut scope = *self.const_scope; + ExprScopeVisitor { + store: self.store, + scopes: self.scopes, + scope: &mut scope, + const_scope: self.const_scope, + } + .on_expr(expr); } -} -fn compute_expr_scopes( - expr: ExprId, - store: &ExpressionStore, - scopes: &mut ExprScopes, - scope: &mut ScopeId, - const_scope: &mut ScopeId, -) { - let compute_expr_scopes = - |scopes: &mut ExprScopes, expr: ExprId, scope: &mut ScopeId, const_scope: &mut ScopeId| { - compute_expr_scopes(expr, store, scopes, scope, const_scope) - }; - let handle_block = |id, - statements, - tail, - label, - scopes: &mut ExprScopes, - scope: &mut ScopeId, - const_scope: &mut ScopeId| { - let mut scope = scopes.new_block_scope(*scope, id, label); - let mut const_scope = if id.is_some() { - scopes.new_block_scope(*const_scope, id, None) - } else { - // We don't need to allocate a new scope, since only items matter to us. - *const_scope - }; - // Overwrite the old scope for the block expr, so that every block scope can be found - // via the block itself (important for blocks that only contain items, no expressions). - scopes.set_scope(expr, scope); - compute_block_scopes(statements, tail, store, scopes, &mut scope, &mut const_scope); - }; + fn on_pat(&mut self, pat: PatId) { + self.store.visit_pat_children(pat, &mut *self); + } - scopes.set_scope(expr, *scope); - match &store[expr] { - Expr::Block { statements, tail, id, label } => { - handle_block(*id, statements, *tail, *label, scopes, scope, const_scope); - } - Expr::Const(id) => { - let mut scope = *const_scope; - compute_expr_scopes(scopes, *id, &mut scope, const_scope); - } - Expr::Array(Array::Repeat { initializer, repeat }) => { - compute_expr_scopes(scopes, *initializer, scope, const_scope); - let mut repeat_scope = *const_scope; - compute_expr_scopes(scopes, *repeat, &mut repeat_scope, const_scope); - } - Expr::Unsafe { id, statements, tail } => { - handle_block(*id, statements, *tail, None, scopes, scope, const_scope); - } - Expr::Loop { body: body_expr, label, source: _ } => { - let mut scope = scopes.new_labeled_scope(*scope, *label); - compute_expr_scopes(scopes, *body_expr, &mut scope, const_scope); - } - Expr::Closure { - args, - arg_types, - ret_type, - body: body_expr, - capture_by: _, - closure_kind: _, - } => { - arg_types - .iter() - .flatten() - .for_each(|type_ref| compute_type_scopes(*type_ref, store, scopes, const_scope)); - if let Some(type_ref) = ret_type { - compute_type_scopes(*type_ref, store, scopes, const_scope); - } - let mut scope = scopes.new_scope(*scope); - args.iter().for_each(|arg| scopes.add_pat_bindings(store, scope, *arg)); - compute_expr_scopes(scopes, *body_expr, &mut scope, const_scope); - } - Expr::Match { expr, arms } => { - compute_expr_scopes(scopes, *expr, scope, const_scope); - for arm in arms.iter() { - let mut scope = scopes.new_scope(*scope); - scopes.add_pat_bindings(store, scope, arm.pat); - if let Some(guard) = arm.guard { - scope = scopes.new_scope(scope); - compute_expr_scopes(scopes, guard, &mut scope, const_scope); - } - compute_expr_scopes(scopes, arm.expr, &mut scope, const_scope); - } - } - &Expr::If { condition, then_branch, else_branch } => { - let mut then_branch_scope = scopes.new_scope(*scope); - compute_expr_scopes(scopes, condition, &mut then_branch_scope, const_scope); - compute_expr_scopes(scopes, then_branch, &mut then_branch_scope, const_scope); - if let Some(else_branch) = else_branch { - compute_expr_scopes(scopes, else_branch, scope, const_scope); - } - } - &Expr::Let { pat, expr } => { - compute_expr_scopes(scopes, expr, scope, const_scope); - *scope = scopes.new_scope(*scope); - scopes.add_pat_bindings(store, *scope, pat); - } - _ => { - store.visit_expr_children(expr, ExprScopeVisitor { store, scopes, scope, const_scope }) - } - }; + fn on_type(&mut self, ty: TypeRefId) { + let mut scope = *self.const_scope; + self.store.visit_type_ref_children( + ty, + ExprScopeVisitor { + store: self.store, + scopes: self.scopes, + scope: &mut scope, + const_scope: self.const_scope, + }, + ); + } } #[cfg(test)] @@ -497,16 +496,27 @@ mod tests { let (file_id, _) = editioned_file_id.unpack(&db); let file_syntax = editioned_file_id.parse(&db).syntax_node(); - let marker: ast::PathExpr = find_node_at_offset(&file_syntax, offset).unwrap(); + let marker: Option<ast::PathExpr> = find_node_at_offset(&file_syntax, offset); let function = find_function(&db, file_id); let scopes = ExprScopes::of(&db, DefWithBodyId::from(function)); - let (_body, source_map) = Body::with_source_map(&db, function.into()); - - let expr_id = source_map - .node_expr(InFile { file_id: editioned_file_id.into(), value: &marker.into() }) - .unwrap() - .as_expr() + let (body, source_map) = Body::with_source_map(&db, function.into()); + + let expr_id = marker + .and_then(|marker| { + source_map + .node_expr(InFile { file_id: editioned_file_id.into(), value: &marker.into() }) + .and_then(|expr| expr.as_expr()) + }) + .or_else(|| { + body.exprs().find_map(|(expr, value)| { + let crate::hir::Expr::Path(path) = value else { return None }; + path.mod_path() + .and_then(|path| path.as_ident()) + .is_some_and(|name| name.as_str() == "marker") + .then_some(expr) + }) + }) .unwrap(); let scope = scopes.scope_for(expr_id); @@ -534,6 +544,58 @@ fn f(param: usize) { } #[test] + fn pattern_type_expr_scope() { + do_check( + r#" +fn f(param: usize) { + let local = 0; + let _: builtin#pattern_type (usize is 0..=$0) = 0; +} +"#, + &["param"], + ); + } + + #[test] + fn closure_pattern_type_expr_scope() { + do_check( + r#" +fn f(param: usize) { + let local = 0; + let _ = |_: builtin#pattern_type (usize is 0..=$0)| {}; +} +"#, + &["param"], + ); + } + + #[test] + fn array_repeat_expr_scope() { + do_check( + r#" +fn f(param: usize) { + let local = 0; + let _ = [(); $0]; +} +"#, + &["param"], + ); + } + + #[test] + fn inline_const_expr_scope() { + do_check( + r#" +fn f(param: usize) { + let local = 0; + let _ = const { $0 }; +} +"#, + &["param"], + ); + } + + #[test] fn test_lambda_scope() { do_check( r" diff --git a/crates/hir-def/src/item_scope.rs b/crates/hir-def/src/item_scope.rs index 14f10651cb..61d578bfda 100644 --- a/crates/hir-def/src/item_scope.rs +++ b/crates/hir-def/src/item_scope.rs @@ -161,7 +161,7 @@ pub struct ItemScope { /// Module scoped macros will be inserted into `items` instead of here. // FIXME: Macro shadowing in one module is not properly handled. Non-item place macros will // be all resolved to the last one defined if shadowing happens. - legacy_macros: FxHashMap<Name, SmallVec<[MacroId; 2]>>, + legacy_macros: FxHashMap<Name, SmallVec<[MacroId; 1]>>, /// The attribute macro invocations in this scope. attr_macros: FxHashMap<AstId<ast::Item>, MacroCallId>, /// The macro invocations in this scope. |