diff --git a/vhdl_lang/src/analysis/association.rs b/vhdl_lang/src/analysis/association.rs index baa10156..a78c03d9 100644 --- a/vhdl_lang/src/analysis/association.rs +++ b/vhdl_lang/src/analysis/association.rs @@ -7,7 +7,7 @@ #![allow(clippy::only_used_in_recursion)] use fnv::FnvHashMap; -use fnv::FnvHashSet; +use itertools::Itertools; use super::analyze::*; use super::names::ResolvedName; @@ -297,147 +297,169 @@ impl<'a> AnalyzeContext<'a> { } } - pub fn resolve_association_formals<'e>( + fn combine_formal_with_actuals<'e>( &self, - error_pos: &SrcPos, // The position of the instance/call-site formal_region: &FormalRegion<'a>, scope: &Scope<'a>, elems: &'e mut [AssociationElement], diagnostics: &mut dyn DiagnosticHandler, - ) -> EvalResult>> { + ) -> EvalResult, Diagnostic>)>> { self.check_positional_before_named(elems, diagnostics)?; // Formal region index => actual position, resolved formal - let mut result: Vec<(&SrcPos, ResolvedFormal)> = Vec::default(); - - let mut missing = false; - let mut extra_associations: Vec = Default::default(); + let mut result: Vec<(&SrcPos, Result)> = Vec::default(); for (actual_idx, AssociationElement { formal, actual }) in elems.iter_mut().enumerate() { if let Some(ref mut formal) = formal { // Named argument - match self.resolve_formal( + let resolved_formal = match self.resolve_formal( formal_region, scope, &formal.pos, &mut formal.item, diagnostics, ) { - Err(err) => { - missing = true; - diagnostics.push(err.into_non_fatal()?); - } - Ok(resolved_formal) => { - result.push((&formal.pos, resolved_formal)); - } - } + Err(err) => Err(err.into_non_fatal()?), + Ok(resolved_formal) => Ok(resolved_formal), + }; + + result.push((&formal.pos, resolved_formal)); } else if let Some(formal) = formal_region.nth(actual_idx) { // Actual index is same as formal index for positional argument let formal = ResolvedFormal::new_basic(actual_idx, formal); - result.push((&actual.pos, formal)); + result.push((&actual.pos, Ok(formal))); } else { - extra_associations.push(actual.pos.clone()); + result.push(( + &actual.pos, + Err(Diagnostic::error(&actual.pos, "Unexpected extra argument")), + )); }; } + Ok(result) + } + + fn check_missing_and_duplicates<'e>( + &self, + error_pos: &SrcPos, // The position of the instance/call-site + resolved_pairs: &[(&'e SrcPos, Result, Diagnostic>)], + formal_region: &FormalRegion<'a>, + diagnostics: &mut dyn DiagnosticHandler, + ) -> EvalResult>> { + let mut is_error = false; + let mut result = Vec::default(); + + let mut associated: FnvHashMap = Default::default(); + for (actual_pos, resolved_formal) in resolved_pairs.iter() { + match resolved_formal { + Ok(resolved_formal) => { + if let Some((prev_pos, prev_formal)) = associated.get(&resolved_formal.idx) { + if !(resolved_formal.is_partial && prev_formal.is_partial) { + let mut diag = Diagnostic::error( + actual_pos, + format!( + "{} has already been associated", + resolved_formal.iface.describe() + ), + ); - let mut not_associated = Vec::new(); - let mut has_duplicates = false; + diag.add_related(prev_pos, "Previously associated here"); + is_error = true; + diagnostics.push(diag); + } + } + result.push(resolved_formal.type_mark); + associated.insert(resolved_formal.idx, (actual_pos, *resolved_formal)); + } + Err(diagnostic) => { + is_error = true; + diagnostics.push(diagnostic.clone()); + } + } + } - { - let associated_indexes = - FnvHashSet::from_iter(result.iter().map(|(_, formal)| formal.idx)); - for (idx, formal) in formal_region.iter().enumerate() { - if !(associated_indexes.contains(&idx) + for (idx, formal) in formal_region.iter().enumerate() { + if !(associated.contains_key(&idx) // Default may be unconnected || formal.has_default() // Output ports are allowed to be unconnected || (formal_region.typ == InterfaceType::Port && formal.is_out_or_inout_signal())) - { - not_associated.push(idx); - } - } - } + { + let mut diagnostic = Diagnostic::error( + error_pos, + format!("No association of {}", formal.describe()), + ); - { - let mut seen: FnvHashMap = Default::default(); - for (actual_pos, resolved_formal) in result.iter() { - if let Some((prev_pos, prev_formal)) = seen.get(&resolved_formal.idx) { - if !(resolved_formal.is_partial && prev_formal.is_partial) { - let mut diag = Diagnostic::error( - actual_pos, - format!( - "{} has already been associated", - resolved_formal.iface.describe() - ), - ); - - diag.add_related(prev_pos, "Previously associated here"); - diagnostics.push(diag); - has_duplicates = true; - } + if let Some(decl_pos) = formal.decl_pos() { + diagnostic.add_related(decl_pos, "Defined here"); } - seen.insert(resolved_formal.idx, (*actual_pos, *resolved_formal)); + + is_error = true; + diagnostics.push(diagnostic); } } - if not_associated.is_empty() && extra_associations.is_empty() && !missing && !has_duplicates - { - Ok(result - .into_iter() - .map(|(_, formal)| formal.type_mark) - .collect()) + if !is_error { + Ok(result) } else { - // Only complain if nothing else is wrong - for idx in not_associated { - if let Some(formal) = formal_region.nth(idx) { - let mut diagnostic = Diagnostic::error( - error_pos, - format!("No association of {}", formal.describe()), - ); - - if let Some(decl_pos) = formal.decl_pos() { - diagnostic.add_related(decl_pos, "Defined here"); - } - - diagnostics.push(diagnostic); - } - } - for pos in extra_associations.into_iter() { - diagnostics.error(pos, "Unexpected extra argument") - } - Err(EvalError::Unknown) } } - pub fn analyze_assoc_elems_with_formal_region( + pub fn resolve_association_formals<'e>( + &self, + error_pos: &SrcPos, // The position of the instance/call-site + formal_region: &FormalRegion<'a>, + scope: &Scope<'a>, + elems: &'e mut [AssociationElement], + diagnostics: &mut dyn DiagnosticHandler, + ) -> EvalResult>> { + let resolved_pairs = + self.combine_formal_with_actuals(formal_region, scope, elems, diagnostics)?; + self.check_missing_and_duplicates(error_pos, &resolved_pairs, formal_region, diagnostics) + } + + pub fn check_association<'e>( &self, error_pos: &SrcPos, // The position of the instance/call-site formal_region: &FormalRegion<'a>, scope: &Scope<'a>, - elems: &mut [AssociationElement], + elems: &'e mut [AssociationElement], diagnostics: &mut dyn DiagnosticHandler, ) -> FatalResult { - if let Some(formals) = as_fatal(self.resolve_association_formals( - error_pos, - formal_region, - scope, - elems, - diagnostics, - ))? { - for (formal_type, actual) in formals + let resolved_pairs = + as_fatal(self.combine_formal_with_actuals(formal_region, scope, elems, diagnostics))?; + + if let Some(resolved_pairs) = resolved_pairs { + as_fatal(self.check_missing_and_duplicates( + error_pos, + &resolved_pairs, + formal_region, + diagnostics, + ))?; + + let resolved_formals = resolved_pairs + .into_iter() + .map(|(_, resolved_formal)| resolved_formal) + .collect_vec(); + + for (resolved_formal, actual) in resolved_formals .iter() .zip(elems.iter_mut().map(|assoc| &mut assoc.actual)) { match &mut actual.item { ActualPart::Expression(expr) => { - self.expr_pos_with_ttyp( - scope, - *formal_type, - &actual.pos, - expr, - diagnostics, - )?; + if let Ok(resolved_formal) = resolved_formal { + // Error case is already checked in check_missing_and_duplicates + self.expr_pos_with_ttyp( + scope, + resolved_formal.type_mark, + &actual.pos, + expr, + diagnostics, + )?; + } else { + self.expr_pos_unknown_ttyp(scope, &actual.pos, expr, diagnostics)?; + } } ActualPart::Open => {} } diff --git a/vhdl_lang/src/analysis/concurrent.rs b/vhdl_lang/src/analysis/concurrent.rs index 971a7f17..ec6cc1b3 100644 --- a/vhdl_lang/src/analysis/concurrent.rs +++ b/vhdl_lang/src/analysis/concurrent.rs @@ -293,7 +293,7 @@ impl<'a> AnalyzeContext<'a> { let (generic_region, port_region) = ent_region.to_entity_formal(); - self.analyze_assoc_elems_with_formal_region( + self.check_association( &entity_name.pos, &generic_region, scope, @@ -304,7 +304,7 @@ impl<'a> AnalyzeContext<'a> { .unwrap_or(&mut []), diagnostics, )?; - self.analyze_assoc_elems_with_formal_region( + self.check_association( &entity_name.pos, &port_region, scope, @@ -339,7 +339,7 @@ impl<'a> AnalyzeContext<'a> { if let AnyEntKind::Component(ent_region) = ent.kind() { let (generic_region, port_region) = ent_region.to_entity_formal(); - self.analyze_assoc_elems_with_formal_region( + self.check_association( &component_name.pos, &generic_region, scope, @@ -350,7 +350,7 @@ impl<'a> AnalyzeContext<'a> { .unwrap_or(&mut []), diagnostics, )?; - self.analyze_assoc_elems_with_formal_region( + self.check_association( &component_name.pos, &port_region, scope, diff --git a/vhdl_lang/src/analysis/overloaded.rs b/vhdl_lang/src/analysis/overloaded.rs index a99160f3..2e3890d9 100644 --- a/vhdl_lang/src/analysis/overloaded.rs +++ b/vhdl_lang/src/analysis/overloaded.rs @@ -191,13 +191,7 @@ impl<'a> AnalyzeContext<'a> { assocs: &mut [AssociationElement], diagnostics: &mut dyn DiagnosticHandler, ) -> FatalResult { - self.analyze_assoc_elems_with_formal_region( - error_pos, - ent.formals(), - scope, - assocs, - diagnostics, - )?; + self.check_association(error_pos, ent.formals(), scope, assocs, diagnostics)?; Ok(()) } diff --git a/vhdl_lang/src/analysis/semantic.rs b/vhdl_lang/src/analysis/semantic.rs index df2d48ea..f3d5923f 100644 --- a/vhdl_lang/src/analysis/semantic.rs +++ b/vhdl_lang/src/analysis/semantic.rs @@ -204,14 +204,14 @@ impl<'a> AnalyzeContext<'a> { if let AnyEntKind::Component(region) = ent.kind() { name.set_unique_reference(ent); let (generic_region, port_region) = region.to_entity_formal(); - self.analyze_assoc_elems_with_formal_region( + self.check_association( &fcall.item.name.pos, &generic_region, scope, &mut [], diagnostics, )?; - self.analyze_assoc_elems_with_formal_region( + self.check_association( &fcall.item.name.pos, &port_region, scope, diff --git a/vhdl_lang/src/analysis/tests/association_formal.rs b/vhdl_lang/src/analysis/tests/association_formal.rs index 261eb1dc..6ad149d9 100644 --- a/vhdl_lang/src/analysis/tests/association_formal.rs +++ b/vhdl_lang/src/analysis/tests/association_formal.rs @@ -505,3 +505,65 @@ end architecture; .search_reference(code.source(), code.s1("inport => sig").s1("sig").start()) .is_some()) } + +#[test] +fn does_not_stop_on_first_error() { + let mut builder = LibraryBuilder::new(); + let code = builder.code( + "libname", + " +entity ent_inst is + port ( + prt0 : in boolean; + prt1 : in boolean + ); +end entity; + +architecture a of ent_inst is +begin +end architecture; + +entity ent is +end entity; + +architecture a of ent is + signal sig0, sig1 : boolean; +begin + ent: entity work.ent_inst + port map ( + missing => sig0, + prt1 => sig1); +end architecture; + ", + ); + + let (root, diagnostics) = builder.get_analyzed_root(); + check_diagnostics( + diagnostics, + vec![ + Diagnostic::error(code.s1("missing"), "No declaration of 'missing'"), + Diagnostic::error( + code.s1("work.ent_inst"), + "No association of port 'prt0' : in", + ) + .related(code.s1("prt0"), "Defined here"), + ], + ); + + // Still sets the reference even if it fails + assert_eq!( + root.search_reference_pos(code.source(), code.s1("=> sig0").s1("sig0").pos().start()) + .unwrap(), + code.s1("sig0").pos() + ); + assert_eq!( + root.search_reference_pos(code.source(), code.s1("=> sig1").s1("sig1").pos().start()) + .unwrap(), + code.s1("sig1").pos() + ); + assert_eq!( + root.search_reference_pos(code.source(), code.s1("prt1 => ").s1("prt1").pos().start()) + .unwrap(), + code.s1("prt1").pos() + ); +}