diff --git a/crates/hir/src/hir_def/declaration.rs b/crates/hir/src/hir_def/declaration.rs index 99b832f6..d39845c1 100644 --- a/crates/hir/src/hir_def/declaration.rs +++ b/crates/hir/src/hir_def/declaration.rs @@ -112,9 +112,22 @@ pub enum NetStrength { #[derive(Debug, PartialEq, Eq, Clone)] pub struct ParamDecl { pub ty: DataTy, + pub kind: ParamDeclKind, pub decls: DeclsRange, } +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum ParamDeclKind { + Parameter, + LocalParam, +} + +impl ParamDeclKind { + pub fn is_overridable(self) -> bool { + matches!(self, Self::Parameter) + } +} + #[derive(Debug, PartialEq, Eq, Clone)] pub struct GenvarDecl { pub ty: DataTy, @@ -246,12 +259,20 @@ impl LowerDeclarationCtx<'_> { pub(crate) fn lower_param_decl_base( &mut self, param_decl: ast::ParameterDeclarationBase, + ) -> DeclarationId { + self.lower_param_decl_base_with_inherited_kind(param_decl, None) + } + + pub(crate) fn lower_param_decl_base_with_inherited_kind( + &mut self, + param_decl: ast::ParameterDeclarationBase, + inherited_kind: Option, ) -> DeclarationId { use ast::ParameterDeclarationBase::*; match param_decl { - ParameterDeclaration(param_decl) => self.lower_param_decl(param_decl), + ParameterDeclaration(param_decl) => self.lower_param_decl(param_decl, inherited_kind), TypeParameterDeclaration(type_param_decl) => { - self.lower_type_param_decl(type_param_decl) + self.lower_type_param_decl(type_param_decl, inherited_kind) } } } @@ -259,7 +280,12 @@ impl LowerDeclarationCtx<'_> { fn lower_type_param_decl( &mut self, type_param_decl: ast::TypeParameterDeclaration, + inherited_kind: Option, ) -> DeclarationId { + let kind = lower_param_decl_kind( + type_param_decl.keyword().map(|keyword| keyword.kind()), + inherited_kind, + ); let start = self.decls.nxt_idx(); let ty = DataTy::Builtin( self.db.intern_ty(crate::hir_def::expr::data_ty::BuiltinDataTy::default()), @@ -267,19 +293,27 @@ impl LowerDeclarationCtx<'_> { let decls = DeclsRange::new(start..self.decls.nxt_idx()); alloc_idx_and_src! { - ParamDecl { ty, decls } => self.declarations, + ParamDecl { ty, kind, decls } => self.declarations, type_param_decl => self.declaration_srcs, } } - fn lower_param_decl(&mut self, param_decl: ast::ParameterDeclaration) -> DeclarationId { + fn lower_param_decl( + &mut self, + param_decl: ast::ParameterDeclaration, + inherited_kind: Option, + ) -> DeclarationId { + let kind = lower_param_decl_kind( + param_decl.keyword().map(|keyword| keyword.kind()), + inherited_kind, + ); let ty = self.expr_ctx().lower_data_ty(param_decl.type_()); let parent = self.declarations.nxt_idx().into(); let decls = self.decl_ctx().lower_declarators(param_decl.declarators(), parent); alloc_idx_and_src! { - ParamDecl { ty, decls } => self.declarations, + ParamDecl { ty, kind, decls } => self.declarations, param_decl => self.declaration_srcs, } } @@ -315,3 +349,14 @@ impl LowerDeclarationCtx<'_> { } } } + +fn lower_param_decl_kind( + keyword: Option, + inherited_kind: Option, +) -> ParamDeclKind { + match keyword { + Some(TokenKind::LOCAL_PARAM_KEYWORD) => ParamDeclKind::LocalParam, + Some(TokenKind::PARAMETER_KEYWORD) => ParamDeclKind::Parameter, + _ => inherited_kind.unwrap_or(ParamDeclKind::Parameter), + } +} diff --git a/crates/hir/src/hir_def/module.rs b/crates/hir/src/hir_def/module.rs index c2856f46..ba399e7c 100644 --- a/crates/hir/src/hir_def/module.rs +++ b/crates/hir/src/hir_def/module.rs @@ -32,7 +32,8 @@ use super::{ alloc_idx_and_src, block::{BlockInfo, BlockSrc, LocalBlockId}, declaration::{ - Declaration, DeclarationId, DeclarationSrc, LowerDeclaration, impl_lower_declaration, + Declaration, DeclarationId, DeclarationSrc, LowerDeclaration, ParamDeclKind, + impl_lower_declaration, }, expr::{ Expr, ExprSrc, LowerExpr, @@ -332,8 +333,14 @@ impl LowerModuleCtx<'_> { pub(crate) fn lower_module_decl(&mut self, decl: ast::ModuleDeclaration) { let header = decl.header(); if let Some(param_ports) = header.parameters() { + let mut inherited_kind = ParamDeclKind::Parameter; for decls in param_ports.declarations().children() { - self.declaration_ctx().lower_param_decl_base(decls); + let decl_id = self + .declaration_ctx() + .lower_param_decl_base_with_inherited_kind(decls, Some(inherited_kind)); + if let Declaration::ParamDecl(param_decl) = self.module.get(decl_id) { + inherited_kind = param_decl.kind; + } self.region_tree.handle_node(decls.syntax()); } diff --git a/crates/ide/src/code_action.rs b/crates/ide/src/code_action.rs index 03de8863..4426f9fd 100644 --- a/crates/ide/src/code_action.rs +++ b/crates/ide/src/code_action.rs @@ -16,8 +16,8 @@ pub use diagnostics::{ pub(crate) use edits::{apply_missing_list_edit, line_indent}; pub(crate) use engine::code_action; pub(crate) use module_members::{ - all_parameter_names, leading_parameter_names, missing_member_entry_text, port_names, - remaining_ordered_port_names, + all_overridable_parameter_names, leading_overridable_parameter_names, + missing_member_entry_text, port_names, remaining_ordered_port_names, }; #[cfg(test)] diff --git a/crates/ide/src/code_action/handlers/add_missing_parameters.rs b/crates/ide/src/code_action/handlers/add_missing_parameters.rs index f0dee54f..9e0896cc 100644 --- a/crates/ide/src/code_action/handlers/add_missing_parameters.rs +++ b/crates/ide/src/code_action/handlers/add_missing_parameters.rs @@ -12,8 +12,8 @@ use utils::get::GetRef; use crate::{ code_action::{ CodeActionCollector, CodeActionCtx, CodeActionId, CodeActionKind, RepairKind, - all_parameter_names, apply_missing_list_edit, leading_parameter_names, - missing_member_entry_text, + all_overridable_parameter_names, apply_missing_list_edit, + leading_overridable_parameter_names, missing_member_entry_text, }, module_resolution::resolve_instantiation_target, }; @@ -54,7 +54,7 @@ pub(super) fn add_missing_parameters( .unwrap_or_default(); let names: Vec<_> = if is_ordered { - leading_parameter_names(&target_module) + leading_overridable_parameter_names(&target_module) .into_iter() .skip(instantiation.param_assigns.len()) .collect() @@ -70,7 +70,7 @@ pub(super) fn add_missing_parameters( } } - all_parameter_names(&target_module) + all_overridable_parameter_names(&target_module) .into_iter() .filter(|name| !assigned_names.contains(name)) .collect() diff --git a/crates/ide/src/code_action/handlers/convert_ordered_connections.rs b/crates/ide/src/code_action/handlers/convert_ordered_connections.rs index df2fbb29..0758d702 100644 --- a/crates/ide/src/code_action/handlers/convert_ordered_connections.rs +++ b/crates/ide/src/code_action/handlers/convert_ordered_connections.rs @@ -10,7 +10,7 @@ use syntax::{ use crate::{ code_action::{ CodeActionCollector, CodeActionCtx, CodeActionId, CodeActionKind, RepairKind, - leading_parameter_names, port_names, + leading_overridable_parameter_names, port_names, }, module_resolution::resolve_instantiation_target, }; @@ -80,7 +80,7 @@ pub(super) fn convert_ordered_params( let target_module_id = resolve_instantiation_target(db, ctx.file_id(), ast_instantiation).unique()?; let target_module = db.module(target_module_id); - let param_names = leading_parameter_names(&target_module); + let param_names = leading_overridable_parameter_names(&target_module); let replacements = ast_instantiation .parameters()? diff --git a/crates/ide/src/code_action/handlers/sort_named_instantiation_items.rs b/crates/ide/src/code_action/handlers/sort_named_instantiation_items.rs index 917f7699..90011b15 100644 --- a/crates/ide/src/code_action/handlers/sort_named_instantiation_items.rs +++ b/crates/ide/src/code_action/handlers/sort_named_instantiation_items.rs @@ -12,8 +12,8 @@ use utils::text_edit::TextRange; use crate::{ code_action::{ - CodeActionCollector, CodeActionCtx, CodeActionId, CodeActionKind, all_parameter_names, - line_indent, port_names, + CodeActionCollector, CodeActionCtx, CodeActionId, CodeActionKind, + all_overridable_parameter_names, line_indent, port_names, }, module_resolution::resolve_instantiation_target, }; @@ -37,7 +37,8 @@ pub(super) fn sort_named_parameter_assignments( let db = ctx.sema().db; let target_module_id = resolve_instantiation_target(db, ctx.file_id(), instantiation).unique()?; - let parameter_order = all_parameter_names(&db.module(target_module_id)); + let target_module = db.module(target_module_id); + let parameter_order = all_overridable_parameter_names(&target_module); let parameter_order_map: FxHashMap<_, _> = parameter_order.iter().enumerate().map(|(index, name)| (name.as_ref(), index)).collect(); diff --git a/crates/ide/src/code_action/module_members.rs b/crates/ide/src/code_action/module_members.rs index 6e7df959..d1f37c7d 100644 --- a/crates/ide/src/code_action/module_members.rs +++ b/crates/ide/src/code_action/module_members.rs @@ -38,21 +38,26 @@ pub(crate) fn remaining_ordered_port_names(module: &Module, connected: usize) -> } } -pub(crate) fn leading_parameter_names(module: &Module) -> Vec { +pub(crate) fn leading_overridable_parameter_names(module: &Module) -> Vec { module .declarations .values() .take_while(|declaration| matches!(declaration, Declaration::ParamDecl(_))) + .filter(|declaration| { + matches!(declaration, Declaration::ParamDecl(param_decl) if param_decl.kind.is_overridable()) + }) .flat_map(|declaration| declaration.decls()) .filter_map(|decl| module.get(decl).name.clone()) .collect() } -pub(crate) fn all_parameter_names(module: &Module) -> Vec { +pub(crate) fn all_overridable_parameter_names(module: &Module) -> Vec { module .declarations .values() - .filter(|declaration| matches!(declaration, Declaration::ParamDecl(_))) + .filter(|declaration| { + matches!(declaration, Declaration::ParamDecl(param_decl) if param_decl.kind.is_overridable()) + }) .flat_map(|declaration| declaration.decls()) .filter_map(|decl| module.get(decl).name.clone()) .collect() diff --git a/crates/ide/src/code_action/tests.rs b/crates/ide/src/code_action/tests.rs index 2f7161ad..8db45ba1 100644 --- a/crates/ide/src/code_action/tests.rs +++ b/crates/ide/src/code_action/tests.rs @@ -411,6 +411,26 @@ fn missing_parameter_repair_fills_named_parameters() { ); } +#[test] +fn missing_parameter_repair_skips_localparams() { + let text = "module child #(parameter A = 1, parameter B = 2) (); localparam L = 3; endmodule\nmodule top; localparam L = 4; child #(/*caret*/.A(1)) u(); endmodule\n"; + let fixed = apply_action(text, RepairKind::MissingParameter).unwrap(); + assert_eq!( + fixed, + "module child #(parameter A = 1, parameter B = 2) (); localparam L = 3; endmodule\nmodule top; localparam L = 4; child #(.A(1), .B()) u(); endmodule\n" + ); +} + +#[test] +fn missing_parameter_repair_honors_parameter_port_keyword_inheritance() { + let text = "module child #(localparam int L = 1, int H = 2, parameter int A = 3, int B = 4) (); endmodule\nmodule top; child #(/*caret*/.A(1)) u(); endmodule\n"; + let fixed = apply_action(text, RepairKind::MissingParameter).unwrap(); + assert_eq!( + fixed, + "module child #(localparam int L = 1, int H = 2, parameter int A = 3, int B = 4) (); endmodule\nmodule top; child #(.A(1), .B()) u(); endmodule\n" + ); +} + #[test] fn missing_parameter_repair_is_available_without_diagnostics() { let text = "module child #(parameter A = 1, parameter B) (); endmodule\nmodule top; child #(/*caret*/.A(1)) u(); endmodule\n"; diff --git a/crates/ide/src/completion/engine/instantiation.rs b/crates/ide/src/completion/engine/instantiation.rs index f184ff05..7277151b 100644 --- a/crates/ide/src/completion/engine/instantiation.rs +++ b/crates/ide/src/completion/engine/instantiation.rs @@ -2,7 +2,7 @@ use hir::{ db::HirDb, hir_def::{ Ident, - declaration::{Declaration, DeclarationSrc}, + declaration::Declaration, expr::declarator::DeclaratorParent, module::{ModuleId, port::Ports}, }, @@ -11,7 +11,7 @@ use syntax::{ SyntaxAncestors, ast::{self, AstNode}, }; -use utils::get::{Get, GetRef}; +use utils::get::GetRef; use crate::db::root_db::RootDb; @@ -59,8 +59,7 @@ pub(super) fn overridable_params_of_module_in_order( db: &RootDb, module_id: ModuleId, ) -> Vec { - let (module, module_src_map) = db.module_with_source_map(module_id); - let tree = db.parse(module_id.file_id); + let module = db.module(module_id); let mut names = Vec::new(); @@ -71,22 +70,10 @@ pub(super) fn overridable_params_of_module_in_order( let DeclaratorParent::DeclarationId(declaration_id) = decl.parent else { continue; }; - let Declaration::ParamDecl(_) = module.get(declaration_id) else { - continue; - }; - - let Some(DeclarationSrc::ParameterDeclaration(ptr)) = module_src_map.get(declaration_id) - else { - continue; - }; - let Some(ast_decl) = ptr.to_node(&tree).and_then(ast::ParameterDeclaration::cast) else { - continue; - }; - - let Some(keyword) = ast_decl.keyword() else { + let Declaration::ParamDecl(param_decl) = module.get(declaration_id) else { continue; }; - if keyword.kind() != syntax::Token![parameter] { + if !param_decl.kind.is_overridable() { continue; } diff --git a/crates/ide/src/completion/engine/typed_filter.rs b/crates/ide/src/completion/engine/typed_filter.rs index 7b32e85b..51baee45 100644 --- a/crates/ide/src/completion/engine/typed_filter.rs +++ b/crates/ide/src/completion/engine/typed_filter.rs @@ -3,7 +3,7 @@ use hir::{ db::HirDb, hir_def::{ Ident, - declaration::{Declaration, DeclarationId, DeclarationSrc}, + declaration::Declaration, expr::declarator::DeclaratorParent, module::{ModuleId, port::Ports}, }, @@ -13,7 +13,7 @@ use hir::{ Ty, TyClass, packed_bit_width, type_class, type_of_decl, type_of_path_resolution, }, }; -use utils::get::{Get, GetRef}; +use utils::get::GetRef; use crate::db::root_db::RootDb; @@ -51,11 +51,13 @@ pub(super) fn expected_param_ty( let DeclaratorParent::DeclarationId(declaration_id) = target_module.get(decl_id).parent else { return None; }; - let Declaration::ParamDecl(_) = target_module.get(declaration_id) else { + let Declaration::ParamDecl(param_decl) = target_module.get(declaration_id) else { return None; }; - is_overridable_parameter_decl(db, target_module_id, declaration_id) + param_decl + .kind + .is_overridable() .then(|| type_of_decl(db, InContainer::new(target_module_id.into(), decl_id)).ty) } @@ -148,19 +150,3 @@ pub(super) fn is_compatible_typed_value(db: &RootDb, expected: &Ty, candidate: & _ => false, } } - -fn is_overridable_parameter_decl( - db: &RootDb, - module_id: ModuleId, - declaration_id: DeclarationId, -) -> bool { - let (_, module_src_map) = db.module_with_source_map(module_id); - let tree = db.parse(module_id.file_id); - let Some(DeclarationSrc::ParameterDeclaration(ptr)) = module_src_map.get(declaration_id) else { - return false; - }; - let Some(node) = ptr.to_node(&tree) else { - return false; - }; - node.first_token().is_some_and(|kw| kw.kind() == syntax::Token![parameter]) -}