From 3b91037b2e4747b04b32a794fdce855f37376218 Mon Sep 17 00:00:00 2001 From: Lukas Scheller <45085299+Schottkyc137@users.noreply.github.com> Date: Sun, 18 Aug 2024 17:22:30 +0200 Subject: [PATCH] Fix: views in generic packages no longer cause type mismatch (#334) --- vhdl_lang/src/analysis.rs | 1 + vhdl_lang/src/analysis/declarative.rs | 88 ++++++------ vhdl_lang/src/analysis/design_unit.rs | 3 +- vhdl_lang/src/analysis/expression.rs | 43 +++--- vhdl_lang/src/analysis/literals.rs | 9 +- vhdl_lang/src/analysis/names.rs | 5 +- vhdl_lang/src/analysis/overloaded.rs | 6 +- vhdl_lang/src/analysis/package_instance.rs | 136 +++++++++++------- vhdl_lang/src/analysis/scope.rs | 105 ++++++++------ vhdl_lang/src/analysis/sequential.rs | 8 +- vhdl_lang/src/analysis/subprogram.rs | 3 + vhdl_lang/src/analysis/tests/mod.rs | 52 +------ .../src/analysis/tests/package_instance.rs | 31 ++++ .../src/analysis/tests/view_declarations.rs | 47 ++++++ vhdl_lang/src/named_entity.rs | 1 + vhdl_lang/src/named_entity/region.rs | 2 +- vhdl_lang/src/named_entity/visibility.rs | 68 ++++++--- 17 files changed, 357 insertions(+), 251 deletions(-) diff --git a/vhdl_lang/src/analysis.rs b/vhdl_lang/src/analysis.rs index 85b3653f..cd1c79be 100644 --- a/vhdl_lang/src/analysis.rs +++ b/vhdl_lang/src/analysis.rs @@ -30,6 +30,7 @@ mod types; #[cfg(test)] pub(crate) mod tests; + pub(crate) use root::{Library, LockedUnit}; pub use self::root::{DesignRoot, EntHierarchy}; diff --git a/vhdl_lang/src/analysis/declarative.rs b/vhdl_lang/src/analysis/declarative.rs index 0916d67e..c6f8288c 100644 --- a/vhdl_lang/src/analysis/declarative.rs +++ b/vhdl_lang/src/analysis/declarative.rs @@ -714,11 +714,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { colon_token: _, } = attr_spec; - let attr_ent = match scope.lookup( - self.ctx, - ident.item.token, - &Designator::Identifier(ident.item.name().clone()), - ) { + let attr_ent = match scope.lookup(&Designator::Identifier(ident.item.name().clone())) { Ok(NamedEntities::Single(ent)) => { ident.set_unique_reference(ent); if let Some(attr_ent) = AttributeEnt::from_any(ent) { @@ -748,7 +744,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { return Ok(()); } Err(err) => { - diagnostics.push(err); + diagnostics.push(err.into_diagnostic(self.ctx, ident.item.token)); return Ok(()); } }; @@ -758,55 +754,53 @@ impl<'a, 't> AnalyzeContext<'a, 't> { signature, }) = entity_name { - let ent: EntRef<'_> = - match scope.lookup(self.ctx, designator.token, &designator.item.item) { - Ok(NamedEntities::Single(ent)) => { - designator.set_unique_reference(ent); + let ent: EntRef<'_> = match scope.lookup(&designator.item.item) { + Ok(NamedEntities::Single(ent)) => { + designator.set_unique_reference(ent); - if let Some(signature) = signature { - diagnostics.push(Diagnostic::should_not_have_signature( - "Attribute specification", - signature.pos(self.ctx), - )); - } - ent + if let Some(signature) = signature { + diagnostics.push(Diagnostic::should_not_have_signature( + "Attribute specification", + signature.pos(self.ctx), + )); } - Ok(NamedEntities::Overloaded(overloaded)) => { - if let Some(signature) = signature { - match as_fatal(self.resolve_signature(scope, signature, diagnostics))? { - Some(signature_key) => { - if let Some(ent) = - overloaded.get(&SubprogramKey::Normal(signature_key)) - { - designator.set_unique_reference(&ent); - ent.into() - } else { - diagnostics.push(Diagnostic::no_overloaded_with_signature( - designator.pos(self.ctx), - &designator.item.item, - &overloaded, - )); - return Ok(()); - } - } - None => { + ent + } + Ok(NamedEntities::Overloaded(overloaded)) => { + if let Some(signature) = signature { + match as_fatal(self.resolve_signature(scope, signature, diagnostics))? { + Some(signature_key) => { + if let Some(ent) = + overloaded.get(&SubprogramKey::Normal(signature_key)) + { + designator.set_unique_reference(&ent); + ent.into() + } else { + diagnostics.push(Diagnostic::no_overloaded_with_signature( + designator.pos(self.ctx), + &designator.item.item, + &overloaded, + )); return Ok(()); } } - } else if let Some(ent) = overloaded.as_unique() { - designator.set_unique_reference(ent); - ent - } else { - diagnostics - .push(Diagnostic::signature_required(designator.pos(self.ctx))); - return Ok(()); + None => { + return Ok(()); + } } - } - Err(err) => { - diagnostics.push(err); + } else if let Some(ent) = overloaded.as_unique() { + designator.set_unique_reference(ent); + ent + } else { + diagnostics.push(Diagnostic::signature_required(designator.pos(self.ctx))); return Ok(()); } - }; + } + Err(err) => { + diagnostics.push(err.into_diagnostic(self.ctx, designator.token)); + return Ok(()); + } + }; // Attributes affect the underlying entity and cannot be set directly on aliases let ent = ent.as_actual(); diff --git a/vhdl_lang/src/analysis/design_unit.rs b/vhdl_lang/src/analysis/design_unit.rs index 44f62e2e..1b02ffa1 100644 --- a/vhdl_lang/src/analysis/design_unit.rs +++ b/vhdl_lang/src/analysis/design_unit.rs @@ -519,7 +519,8 @@ impl<'a, 't> AnalyzeContext<'a, 't> { } Name::Designator(designator) => { let visible = scope - .lookup(self.ctx, name.span, designator.designator()) + .lookup(designator.designator()) + .map_err(|err| err.into_diagnostic(self.ctx, name.span)) .into_eval_result(diagnostics)?; designator.set_reference(&visible); Ok(UsedNames::Single(visible)) diff --git a/vhdl_lang/src/analysis/expression.rs b/vhdl_lang/src/analysis/expression.rs index d15b36bb..afc87315 100644 --- a/vhdl_lang/src/analysis/expression.rs +++ b/vhdl_lang/src/analysis/expression.rs @@ -238,7 +238,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { expr.pos(self.ctx), "Ambiguous expression. You can use a qualified expression type'(expr) to disambiguate.", ErrorCode::AmbiguousExpression, - ); + ); Err(EvalError::Unknown) } } @@ -254,7 +254,8 @@ impl<'a, 't> AnalyzeContext<'a, 't> { ) -> EvalResult>> { let designator = Designator::OperatorSymbol(op); match scope - .lookup(self.ctx, op_pos, &designator) + .lookup(&designator) + .map_err(|err| err.into_diagnostic(self.ctx, op_pos)) .into_eval_result(diagnostics)? { NamedEntities::Single(ent) => { @@ -503,7 +504,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { Literal::String(_) => Ok(ExpressionType::String), Literal::BitString(_) => Ok(ExpressionType::String), Literal::Character(chr) => { - match scope.lookup(self.ctx, span, &Designator::Character(*chr)) { + match scope.lookup(&Designator::Character(*chr)) { Ok(NamedEntities::Single(ent)) => { // Should never happen but better know if it does diagnostics.add( @@ -543,7 +544,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { } } Err(e) => { - diagnostics.push(e); + diagnostics.push(e.into_diagnostic(self.ctx, span)); Err(EvalError::Unknown) } } @@ -624,12 +625,10 @@ impl<'a, 't> AnalyzeContext<'a, 't> { self.expr_pos_with_ttyp(scope, target_type, expr.span, &mut expr.item, diagnostics) } - fn implicit_bool_types(&self, scope: &Scope<'a>, span: TokenSpan) -> FnvHashSet> { - if let Ok(NamedEntities::Overloaded(overloaded)) = scope.lookup( - self.ctx, - span, - &Designator::OperatorSymbol(Operator::QueQue), - ) { + fn implicit_bool_types(&self, scope: &Scope<'a>) -> FnvHashSet> { + if let Ok(NamedEntities::Overloaded(overloaded)) = + scope.lookup(&Designator::OperatorSymbol(Operator::QueQue)) + { overloaded .entities() .filter_map(|ent| ent.formals().nth(0).map(|typ| typ.type_mark().base())) @@ -650,7 +649,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { match types { ExpressionType::Unambiguous(typ) => { if typ.base() != self.boolean().base() { - let implicit_bools = self.implicit_bool_types(scope, expr.span); + let implicit_bools = self.implicit_bool_types(scope); if !implicit_bools.contains(&typ.base()) { diagnostics.add( expr.pos(self.ctx), @@ -659,7 +658,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { typ.describe(), self.boolean().describe() ), - ErrorCode::NoImplicitConversion + ErrorCode::NoImplicitConversion, ); } } @@ -669,7 +668,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { self.expr_with_ttyp(scope, self.boolean(), expr, diagnostics)?; } else { let implicit_bool_types: FnvHashSet<_> = self - .implicit_bool_types(scope, expr.span) + .implicit_bool_types(scope) .intersection(&types) .cloned() .collect(); @@ -1012,18 +1011,18 @@ impl<'a, 't> AnalyzeContext<'a, 't> { Diagnostic::new( choice.pos(self.ctx), format!( - "All elements of record '{}' are already associated", - record_type.designator() - ), + "All elements of record '{}' are already associated", + record_type.designator() + ), ErrorCode::AlreadyAssociated, ) - .opt_related( - record_type.decl_pos(), - format!( - "Record '{}' defined here", - record_type.designator() + .opt_related( + record_type.decl_pos(), + format!( + "Record '{}' defined here", + record_type.designator() + ), ), - ), ) } diff --git a/vhdl_lang/src/analysis/literals.rs b/vhdl_lang/src/analysis/literals.rs index 0e2a07b2..64ad6e54 100644 --- a/vhdl_lang/src/analysis/literals.rs +++ b/vhdl_lang/src/analysis/literals.rs @@ -188,11 +188,10 @@ impl<'a, 't> AnalyzeContext<'a, 't> { scope: &Scope<'a>, unit: &mut WithRef, ) -> Result, Diagnostic> { - match scope.lookup( - self.ctx, - unit.item.token, - &Designator::Identifier(unit.item.item.clone()), - )? { + match scope + .lookup(&Designator::Identifier(unit.item.item.clone())) + .map_err(|err| err.into_diagnostic(self.ctx, unit.item.token))? + { NamedEntities::Single(unit_ent) => { unit.set_unique_reference(unit_ent); if let AnyEntKind::PhysicalLiteral(physical_ent) = unit_ent.actual_kind() { diff --git a/vhdl_lang/src/analysis/names.rs b/vhdl_lang/src/analysis/names.rs index 66aed5f2..70123b03 100644 --- a/vhdl_lang/src/analysis/names.rs +++ b/vhdl_lang/src/analysis/names.rs @@ -251,7 +251,7 @@ impl<'a> ResolvedName<'a> { } AnyEntKind::Overloaded(_) => { return Err(( - "Internal error. Unreachable as overloded is handled outside this function" + "Internal error. Unreachable as overloaded is handled outside this function" .to_string(), ErrorCode::Internal, )); @@ -1213,7 +1213,8 @@ impl<'a, 't> AnalyzeContext<'a, 't> { let mut resolved = match SplitName::from_name(name) { SplitName::Designator(designator) => { let name = scope - .lookup(self.ctx, span, designator.designator()) + .lookup(designator.designator()) + .map_err(|err| err.into_diagnostic(self.ctx, span)) .into_eval_result(diagnostics)?; return Ok(match name { NamedEntities::Single(ent) => { diff --git a/vhdl_lang/src/analysis/overloaded.rs b/vhdl_lang/src/analysis/overloaded.rs index 91f4c3e9..2763a935 100644 --- a/vhdl_lang/src/analysis/overloaded.rs +++ b/vhdl_lang/src/analysis/overloaded.rs @@ -453,10 +453,8 @@ mod tests { panic!("Expected designator") }; - let overloaded = if let NamedEntities::Overloaded(overloaded) = self - .scope - .lookup(&self.tokens, fcall.span, &des.item) - .unwrap() + let overloaded = if let NamedEntities::Overloaded(overloaded) = + self.scope.lookup(&des.item).unwrap() { overloaded } else { diff --git a/vhdl_lang/src/analysis/package_instance.rs b/vhdl_lang/src/analysis/package_instance.rs index 171f10f6..95b3fdea 100644 --- a/vhdl_lang/src/analysis/package_instance.rs +++ b/vhdl_lang/src/analysis/package_instance.rs @@ -152,7 +152,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { } GpkgInterfaceEnt::Constant(obj) => self.expr_pos_with_ttyp( scope, - self.map_type_ent(&mapping, obj.type_mark()), + self.map_type_ent(&mapping, obj.type_mark(), scope), assoc.actual.span, expr, diagnostics, @@ -240,7 +240,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { uninst_region: &Region<'a>, generic_map: &mut Option, diagnostics: &mut dyn DiagnosticHandler, - ) -> EvalResult<(Region<'a>, FnvHashMap>)> { + ) -> EvalResult<(Region<'a>, FnvHashMap>)> { let nested = scope.nested().in_package_declaration(); let (generics, other) = uninst_region.to_package_generic(); @@ -256,7 +256,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { }; for uninst in other { - match self.instantiate(Some(ent), &mapping, uninst) { + match self.instantiate(Some(ent), &mapping, uninst, &nested) { Ok(inst) => { // We ignore diagnostics here, for example when adding implicit operators EQ and NE for interface types // They can collide if there are more than one interface type that map to the same actual type @@ -280,6 +280,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { parent: Option>, mapping: &FnvHashMap>, uninst: EntRef<'a>, + scope: &Scope<'a>, ) -> Result, (String, ErrorCode)> { let designator = uninst.designator().clone(); @@ -294,7 +295,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { uninst.src_span, Some(self.source()), ); - let kind = self.map_kind(Some(inst), mapping, uninst.kind())?; + let kind = self.map_kind(Some(inst), mapping, uninst.kind(), scope)?; unsafe { inst.set_kind(kind); } @@ -303,7 +304,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { unsafe { self.arena.add_implicit( inst.id(), - self.instantiate(Some(inst), mapping, implicit_uninst)?, + self.instantiate(Some(inst), mapping, implicit_uninst, scope)?, ); } } @@ -316,18 +317,19 @@ impl<'a, 't> AnalyzeContext<'a, 't> { parent: Option>, mapping: &FnvHashMap>, kind: &'a AnyEntKind<'a>, + scope: &Scope<'a>, ) -> Result, (String, ErrorCode)> { Ok(match kind { AnyEntKind::ExternalAlias { class, type_mark } => AnyEntKind::ExternalAlias { class: *class, - type_mark: self.map_type_ent(mapping, *type_mark), + type_mark: self.map_type_ent(mapping, *type_mark, scope), }, AnyEntKind::ObjectAlias { base_object, type_mark, } => AnyEntKind::ObjectAlias { base_object: if let Some(obj) = - ObjectEnt::from_any(self.instantiate(None, mapping, base_object)?) + ObjectEnt::from_any(self.instantiate(None, mapping, base_object, scope)?) { obj } else { @@ -336,43 +338,49 @@ impl<'a, 't> AnalyzeContext<'a, 't> { ErrorCode::Internal, )); }, - type_mark: self.map_type_ent(mapping, *type_mark), + type_mark: self.map_type_ent(mapping, *type_mark, scope), }, - AnyEntKind::File(subtype) => AnyEntKind::File(self.map_subtype(mapping, *subtype)), + AnyEntKind::File(subtype) => { + AnyEntKind::File(self.map_subtype(mapping, *subtype, scope)) + } AnyEntKind::InterfaceFile(typ) => { - AnyEntKind::InterfaceFile(self.map_type_ent(mapping, *typ)) + AnyEntKind::InterfaceFile(self.map_type_ent(mapping, *typ, scope)) } AnyEntKind::Component(region) => { - AnyEntKind::Component(self.map_region(parent, mapping, region)?) + AnyEntKind::Component(self.map_region(parent, mapping, region, scope)?) + } + AnyEntKind::Attribute(typ) => { + AnyEntKind::Attribute(self.map_type_ent(mapping, *typ, scope)) } - AnyEntKind::Attribute(typ) => AnyEntKind::Attribute(self.map_type_ent(mapping, *typ)), AnyEntKind::Overloaded(overloaded) => { - AnyEntKind::Overloaded(self.map_overloaded(parent, mapping, overloaded)?) + AnyEntKind::Overloaded(self.map_overloaded(parent, mapping, overloaded, scope)?) } - AnyEntKind::Type(typ) => AnyEntKind::Type(self.map_type(parent, mapping, typ)?), + AnyEntKind::Type(typ) => AnyEntKind::Type(self.map_type(parent, mapping, typ, scope)?), AnyEntKind::ElementDeclaration(subtype) => { - AnyEntKind::ElementDeclaration(self.map_subtype(mapping, *subtype)) + AnyEntKind::ElementDeclaration(self.map_subtype(mapping, *subtype, scope)) } AnyEntKind::Sequential(s) => AnyEntKind::Sequential(*s), AnyEntKind::Concurrent(c) => AnyEntKind::Concurrent(*c), - AnyEntKind::Object(obj) => AnyEntKind::Object(self.map_object(mapping, obj)), + AnyEntKind::Object(obj) => AnyEntKind::Object(self.map_object(mapping, obj, scope)), AnyEntKind::LoopParameter(typ) => AnyEntKind::LoopParameter( - typ.map(|typ| self.map_type_ent(mapping, typ.into()).base()), + typ.map(|typ| self.map_type_ent(mapping, typ.into(), scope).base()), ), AnyEntKind::PhysicalLiteral(typ) => { - AnyEntKind::PhysicalLiteral(self.map_type_ent(mapping, *typ)) + AnyEntKind::PhysicalLiteral(self.map_type_ent(mapping, *typ, scope)) } AnyEntKind::DeferredConstant(subtype) => { - AnyEntKind::DeferredConstant(self.map_subtype(mapping, *subtype)) + AnyEntKind::DeferredConstant(self.map_subtype(mapping, *subtype, scope)) } AnyEntKind::Library => AnyEntKind::Library, AnyEntKind::Design(design) => match design { Design::PackageInstance(region) => AnyEntKind::Design(Design::PackageInstance( - self.map_region(parent, mapping, region)?, + self.map_region(parent, mapping, region, scope)?, )), - Design::InterfacePackageInstance(region) => AnyEntKind::Design( - Design::InterfacePackageInstance(self.map_region(parent, mapping, region)?), - ), + Design::InterfacePackageInstance(region) => { + AnyEntKind::Design(Design::InterfacePackageInstance( + self.map_region(parent, mapping, region, scope)?, + )) + } _ => { return Err(( format!( @@ -383,7 +391,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { )); } }, - AnyEntKind::View(typ) => AnyEntKind::View(self.map_subtype(mapping, *typ)), + AnyEntKind::View(typ) => AnyEntKind::View(self.map_subtype(mapping, *typ, scope)), }) } @@ -392,32 +400,33 @@ impl<'a, 't> AnalyzeContext<'a, 't> { parent: Option>, mapping: &FnvHashMap>, overloaded: &'a Overloaded<'a>, + scope: &Scope<'a>, ) -> Result, (String, ErrorCode)> { Ok(match overloaded { Overloaded::SubprogramDecl(signature) => { - Overloaded::SubprogramDecl(self.map_signature(parent, mapping, signature)?) + Overloaded::SubprogramDecl(self.map_signature(parent, mapping, signature, scope)?) } Overloaded::Subprogram(signature) => { - Overloaded::Subprogram(self.map_signature(parent, mapping, signature)?) + Overloaded::Subprogram(self.map_signature(parent, mapping, signature, scope)?) } Overloaded::UninstSubprogramDecl(signature, generic_map) => { Overloaded::UninstSubprogramDecl( - self.map_signature(parent, mapping, signature)?, + self.map_signature(parent, mapping, signature, scope)?, generic_map.clone(), ) } Overloaded::UninstSubprogram(signature, generic_map) => Overloaded::UninstSubprogram( - self.map_signature(parent, mapping, signature)?, + self.map_signature(parent, mapping, signature, scope)?, generic_map.clone(), ), - Overloaded::InterfaceSubprogram(signature) => { - Overloaded::InterfaceSubprogram(self.map_signature(parent, mapping, signature)?) - } + Overloaded::InterfaceSubprogram(signature) => Overloaded::InterfaceSubprogram( + self.map_signature(parent, mapping, signature, scope)?, + ), Overloaded::EnumLiteral(signature) => { - Overloaded::EnumLiteral(self.map_signature(parent, mapping, signature)?) + Overloaded::EnumLiteral(self.map_signature(parent, mapping, signature, scope)?) } Overloaded::Alias(alias) => { - let alias_inst = self.instantiate(parent, mapping, alias)?; + let alias_inst = self.instantiate(parent, mapping, alias, scope)?; if let Some(overloaded) = OverloadedEnt::from_any(alias_inst) { Overloaded::Alias(overloaded) @@ -437,6 +446,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { parent: Option>, mapping: &FnvHashMap>, signature: &'a Signature<'a>, + scope: &Scope<'a>, ) -> Result, (String, ErrorCode)> { let Signature { formals, @@ -450,7 +460,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { let mut inst_entities = Vec::with_capacity(uninst_entities.len()); for uninst in uninst_entities { - let inst = self.instantiate(parent, mapping, uninst)?; + let inst = self.instantiate(parent, mapping, uninst, scope)?; if let Some(inst) = InterfaceEnt::from_any(inst) { inst_entities.push(inst); @@ -467,7 +477,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { typ: *typ, entities: inst_entities, }, - return_type: return_type.map(|typ| self.map_type_ent(mapping, typ)), + return_type: return_type.map(|typ| self.map_type_ent(mapping, typ, scope)), }) } @@ -476,6 +486,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { parent: Option>, mapping: &FnvHashMap>, region: &'a Region<'a>, + scope: &Scope<'a>, ) -> Result, (String, ErrorCode)> { let Region { entities: uninst_entities, @@ -488,15 +499,15 @@ impl<'a, 't> AnalyzeContext<'a, 't> { ..Region::default() }; - for (_, uninst) in uninst_entities.iter() { + for uninst in uninst_entities.values() { match uninst { NamedEntities::Single(uninst) => { - let inst = self.instantiate(parent, mapping, uninst)?; + let inst = self.instantiate(parent, mapping, uninst, scope)?; inst_region.add(inst, &mut NullDiagnostics); } NamedEntities::Overloaded(overloaded) => { for uninst in overloaded.entities() { - let inst = self.instantiate(parent, mapping, uninst.into())?; + let inst = self.instantiate(parent, mapping, uninst.into(), scope)?; inst_region.add(inst, &mut NullDiagnostics); } } @@ -511,31 +522,31 @@ impl<'a, 't> AnalyzeContext<'a, 't> { parent: Option>, mapping: &FnvHashMap>, typ: &'a Type<'a>, + scope: &Scope<'a>, ) -> Result, (String, ErrorCode)> { Ok(match typ { Type::Array { indexes, elem_type } => { let mut mapped_indexes = Vec::with_capacity(indexes.len()); for index_typ in indexes.iter() { - mapped_indexes.push( - index_typ - .map(|index_typ| self.map_type_ent(mapping, index_typ.into()).base()), - ) + mapped_indexes.push(index_typ.map(|index_typ| { + self.map_type_ent(mapping, index_typ.into(), scope).base() + })) } Type::Array { indexes: mapped_indexes, - elem_type: self.map_type_ent(mapping, *elem_type), + elem_type: self.map_type_ent(mapping, *elem_type, scope), } } Type::Enum(symbols) => Type::Enum(symbols.clone()), Type::Integer => Type::Integer, Type::Real => Type::Real, Type::Physical => Type::Physical, - Type::Access(subtype) => Type::Access(self.map_subtype(mapping, *subtype)), - Type::Record(region) => { - let mut elems = Vec::with_capacity(region.elems.len()); - for uninst in region.elems.iter() { - let inst = self.instantiate(parent, mapping, uninst)?; + Type::Access(subtype) => Type::Access(self.map_subtype(mapping, *subtype, scope)), + Type::Record(record_region) => { + let mut elems = Vec::with_capacity(record_region.elems.len()); + for uninst in record_region.elems.iter() { + let inst = self.instantiate(parent, mapping, uninst, scope)?; if let Some(inst) = RecordElement::from_any(inst) { elems.push(inst); @@ -545,12 +556,12 @@ impl<'a, 't> AnalyzeContext<'a, 't> { } Type::Record(RecordRegion { elems }) } - Type::Subtype(subtype) => Type::Subtype(self.map_subtype(mapping, *subtype)), + Type::Subtype(subtype) => Type::Subtype(self.map_subtype(mapping, *subtype, scope)), Type::Protected(region, is_body) => { - Type::Protected(self.map_region(parent, mapping, region)?, *is_body) + Type::Protected(self.map_region(parent, mapping, region, scope)?, *is_body) } Type::File => Type::File, - Type::Alias(typ) => Type::Alias(self.map_type_ent(mapping, *typ)), + Type::Alias(typ) => Type::Alias(self.map_type_ent(mapping, *typ, scope)), Type::Universal(utyp) => Type::Universal(*utyp), Type::Incomplete => Type::Incomplete, Type::Interface => Type::Interface, @@ -561,6 +572,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { &self, mapping: &FnvHashMap>, obj: &Object<'a>, + scope: &Scope<'a>, ) -> Object<'a> { let Object { class, @@ -572,7 +584,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { Object { class: *class, iface: iface.clone(), - subtype: self.map_subtype(mapping, *subtype), + subtype: self.map_subtype(mapping, *subtype, scope), has_default: *has_default, } } @@ -581,19 +593,35 @@ impl<'a, 't> AnalyzeContext<'a, 't> { &self, mapping: &FnvHashMap>, typ: TypeEnt<'a>, + scope: &Scope<'a>, ) -> TypeEnt<'a> { - mapping.get(&typ.id()).cloned().unwrap_or(typ) + match mapping.get(&typ.id()) { + None => { + if let Some(entity) = scope + .lookup(&typ.designator) + .ok() + .and_then(|result| result.into_non_overloaded().ok()) + .and_then(TypeEnt::from_any) + { + entity + } else { + typ + } + } + Some(typ) => *typ, + } } fn map_subtype( &self, mapping: &FnvHashMap>, subtype: Subtype<'a>, + scope: &Scope<'a>, ) -> Subtype<'a> { let Subtype { type_mark } = subtype; Subtype { - type_mark: self.map_type_ent(mapping, type_mark), + type_mark: self.map_type_ent(mapping, type_mark, scope), } } } diff --git a/vhdl_lang/src/analysis/scope.rs b/vhdl_lang/src/analysis/scope.rs index c4110c60..3b6fbf8e 100644 --- a/vhdl_lang/src/analysis/scope.rs +++ b/vhdl_lang/src/analysis/scope.rs @@ -9,11 +9,12 @@ use crate::data::*; use crate::named_entity::*; use crate::data::error_codes::ErrorCode; -use crate::{TokenAccess, TokenSpan}; +use crate::TokenSpan; use fnv::FnvHashMap; use std::cell::RefCell; use std::collections::hash_map::Entry; use std::rc::Rc; +use vhdl_lang::TokenAccess; #[derive(Default, Clone)] pub(crate) struct Scope<'a>(Rc>>); @@ -26,6 +27,46 @@ struct ScopeInner<'a> { anon_idx: usize, } +#[derive(Debug)] +pub(crate) enum UndeclaredKind { + Identifier(Symbol), + Operator(Operator), + Character(u8), + Anonymous, +} + +#[derive(Debug)] +pub(crate) enum LookupError { + IntoUnambiguousError(IntoUnambiguousError), + Undeclared(UndeclaredKind), +} + +impl From for LookupError { + fn from(value: IntoUnambiguousError) -> Self { + Self::IntoUnambiguousError(value) + } +} + +impl LookupError { + pub fn into_diagnostic(self, ctx: &dyn TokenAccess, span: impl Into) -> Diagnostic { + let span = span.into(); + match self { + LookupError::IntoUnambiguousError(err) => err.into_diagnostic(ctx, span), + LookupError::Undeclared(kind) => { + let msg = match kind { + UndeclaredKind::Identifier(ident) => format!("No declaration of '{ident}'"), + UndeclaredKind::Operator(operator) => { + format!("No declaration of operator '{operator}'") + } + UndeclaredKind::Character(chr) => format!("No declaration of '{chr}'"), + UndeclaredKind::Anonymous => "No declaration of ".to_owned(), + }; + Diagnostic::new(span.pos(ctx), msg, ErrorCode::Unresolved) + } + } + } +} + impl<'a> ScopeInner<'a> { pub fn into_region(self) -> Region<'a> { self.region @@ -122,23 +163,18 @@ impl<'a> ScopeInner<'a> { /// Lookup a named entity that was made potentially visible via a use clause fn lookup_visible( &self, - ctx: &dyn TokenAccess, - span: TokenSpan, designator: &Designator, - ) -> Result>, Diagnostic> { + ) -> Result>, LookupError> { let mut visible = Visible::default(); self.lookup_visiblity_into(designator, &mut visible); - visible.into_unambiguous(ctx, span, designator) + visible + .into_unambiguous(designator) + .map_err(|err| err.into()) } /// Lookup a designator from within the region itself /// Thus all parent regions and visibility is relevant - fn lookup_uncached( - &self, - ctx: &dyn TokenAccess, - span: TokenSpan, - designator: &Designator, - ) -> Result, Diagnostic> { + fn lookup_uncached(&self, designator: &Designator) -> Result, LookupError> { let result = if let Some(enclosing) = self.lookup_enclosing(designator) { match enclosing { // non overloaded in enclosing region ignores any visible overloaded names @@ -146,7 +182,7 @@ impl<'a> ScopeInner<'a> { // In case of overloaded local, non-conflicting visible names are still relevant NamedEntities::Overloaded(enclosing_overloaded) => { if let Ok(Some(NamedEntities::Overloaded(overloaded))) = - self.lookup_visible(ctx, span, designator) + self.lookup_visible(designator) { Some(NamedEntities::Overloaded( enclosing_overloaded.with_visible(overloaded), @@ -157,41 +193,26 @@ impl<'a> ScopeInner<'a> { } } } else { - self.lookup_visible(ctx, span, designator)? + self.lookup_visible(designator)? }; match result { Some(visible) => Ok(visible), - None => Err(Diagnostic::new( - span.pos(ctx), - match designator { - Designator::Identifier(ident) => { - format!("No declaration of '{ident}'") - } - Designator::OperatorSymbol(operator) => { - format!("No declaration of operator '{operator}'") - } - Designator::Character(chr) => { - format!("No declaration of '{chr}'") - } - Designator::Anonymous(_) => "No declaration of ".to_owned(), - }, - ErrorCode::Unresolved, - )), + None => Err(LookupError::Undeclared(match designator { + Designator::Identifier(ident) => UndeclaredKind::Identifier(ident.clone()), + Designator::OperatorSymbol(operator) => UndeclaredKind::Operator(*operator), + Designator::Character(chr) => UndeclaredKind::Character(*chr), + Designator::Anonymous(_) => UndeclaredKind::Anonymous, + })), } } - fn lookup( - &mut self, - ctx: &dyn TokenAccess, - span: TokenSpan, - designator: &Designator, - ) -> Result, Diagnostic> { + fn lookup(&mut self, designator: &Designator) -> Result, LookupError> { if let Some(res) = self.cache.get(designator) { return Ok(res.clone()); } - let ents = self.lookup_uncached(ctx, span, designator)?; + let ents = self.lookup_uncached(designator)?; if let Entry::Vacant(vacant) = self.cache.entry(designator.clone()) { Ok(vacant.insert(ents).clone()) } else { @@ -320,16 +341,8 @@ impl<'a> Scope<'a> { Some(names.clone()) } - pub fn lookup( - &self, - ctx: &dyn TokenAccess, - span: impl Into, - designator: &Designator, - ) -> Result, Diagnostic> { - self.0 - .as_ref() - .borrow_mut() - .lookup(ctx, span.into(), designator) + pub fn lookup(&self, designator: &Designator) -> Result, LookupError> { + self.0.as_ref().borrow_mut().lookup(designator) } /// Used when using context clauses diff --git a/vhdl_lang/src/analysis/sequential.rs b/vhdl_lang/src/analysis/sequential.rs index 1a972fa3..795c436b 100644 --- a/vhdl_lang/src/analysis/sequential.rs +++ b/vhdl_lang/src/analysis/sequential.rs @@ -336,11 +336,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { label: &mut WithRef, diagnostics: &mut dyn DiagnosticHandler, ) { - match scope.lookup( - self.ctx, - label.item.token, - &Designator::Identifier(label.item.item.clone()), - ) { + match scope.lookup(&Designator::Identifier(label.item.item.clone())) { Ok(NamedEntities::Single(ent)) => { label.set_unique_reference(ent); if matches!(ent.kind(), AnyEntKind::Sequential(Some(Sequential::Loop))) { @@ -368,7 +364,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { ErrorCode::MismatchedKinds, ), Err(diag) => { - diagnostics.push(diag); + diagnostics.push(diag.into_diagnostic(self.ctx, label.item.token)); } } } diff --git a/vhdl_lang/src/analysis/subprogram.rs b/vhdl_lang/src/analysis/subprogram.rs index 584d2790..abfabbcd 100644 --- a/vhdl_lang/src/analysis/subprogram.rs +++ b/vhdl_lang/src/analysis/subprogram.rs @@ -265,6 +265,8 @@ impl<'a, 't> AnalyzeContext<'a, 't> { _ => unreachable!(), }; + let nested = scope.nested(); + match as_fatal(self.generic_instance( inst_subprogram_ent, scope, @@ -279,6 +281,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> { Some(inst_subprogram_ent), &mapping, uninstantiated_subprogram.signature(), + &nested, ) { Ok(signature) => Ok(signature), Err((err, code)) => { diff --git a/vhdl_lang/src/analysis/tests/mod.rs b/vhdl_lang/src/analysis/tests/mod.rs index 5f8caa1c..e64a6e23 100644 --- a/vhdl_lang/src/analysis/tests/mod.rs +++ b/vhdl_lang/src/analysis/tests/mod.rs @@ -32,8 +32,7 @@ mod visibility; use std::cell::RefCell; use std::path::PathBuf; -use vhdl_lang::syntax::Kind; -use vhdl_lang::{TokenAccess, TokenSpan}; +use vhdl_lang::TokenSpan; pub use self::util::*; use crate::ast::Designator; @@ -41,55 +40,19 @@ use crate::ast::UnitId; pub use crate::data::Diagnostic; use crate::data::NoDiagnostics; pub use crate::syntax::test::*; -use crate::syntax::{Token, Value}; +use crate::syntax::Token; use super::analyze::AnalyzeContext; use super::scope::*; use super::DesignRoot; use crate::named_entity::*; -use crate::{Position, Range, Source, SrcPos, TokenId}; +use crate::Source; pub(super) struct TestSetup<'a> { builder: RefCell, root: DesignRoot, arena: Arena, pub scope: Scope<'a>, - pub(crate) tokens: MockTokenAccess, -} - -#[cfg(test)] -pub(crate) struct MockTokenAccess { - token: Token, -} - -impl MockTokenAccess { - pub fn new() -> MockTokenAccess { - MockTokenAccess { - token: Token { - pos: SrcPos::new( - Source::inline(PathBuf::default().as_path(), ""), - Range::new(Position::default(), Position::default()), - ), - value: Value::None, - kind: Kind::Xnor, - comments: None, - }, - } - } -} - -impl TokenAccess for MockTokenAccess { - fn get_token(&self, _id: TokenId) -> Option<&Token> { - Some(&self.token) - } - - fn index(&self, _id: TokenId) -> &Token { - &self.token - } - - fn get_token_slice(&self, _start_id: TokenId, _end_id: TokenId) -> &[Token] { - unimplemented!() - } } impl<'a> TestSetup<'a> { @@ -104,7 +67,6 @@ impl<'a> TestSetup<'a> { root, builder: RefCell::new(builder), scope: Scope::new(Region::default()), - tokens: MockTokenAccess::new(), } } @@ -155,7 +117,7 @@ impl<'a> TestSetup<'a> { let designator = self.snippet(sym).designator(); self.scope - .lookup(&self.tokens, designator.token, &designator.item) + .lookup(&designator.item) .unwrap() .into_non_overloaded() .unwrap() @@ -164,11 +126,7 @@ impl<'a> TestSetup<'a> { pub fn lookup_overloaded(&'a self, code: Code) -> OverloadedEnt<'a> { let des = code.designator(); - if let NamedEntities::Overloaded(overloaded) = self - .scope - .lookup(&self.tokens, des.token, &des.item) - .unwrap() - { + if let NamedEntities::Overloaded(overloaded) = self.scope.lookup(&des.item).unwrap() { overloaded .entities() .find(|ent| ent.decl_pos() == Some(&code.pos())) diff --git a/vhdl_lang/src/analysis/tests/package_instance.rs b/vhdl_lang/src/analysis/tests/package_instance.rs index 9186905f..ead876bc 100644 --- a/vhdl_lang/src/analysis/tests/package_instance.rs +++ b/vhdl_lang/src/analysis/tests/package_instance.rs @@ -654,3 +654,34 @@ end package; let diag = builder.analyze(); check_no_diagnostics(&diag); } + +#[test] +pub fn aliases_in_generic_packages() { + let mut builder = LibraryBuilder::new(); + builder.code( + "libname", + " +package test_pkg is + generic (N : integer); + type my_type is array (N - 1 downto 0) of bit; + alias also_my_type is my_type; +end package; + +package pkg_inst is new work.test_pkg generic map (N => 8); +use work.pkg_inst.all; + +entity test_top_entity is +end entity; + +architecture rtl of test_top_entity is + signal my_sig : my_type; + signal my_sig2: also_my_type; +begin + my_sig <= my_sig2; +end architecture; + ", + ); + + let diag = builder.analyze(); + check_no_diagnostics(&diag); +} diff --git a/vhdl_lang/src/analysis/tests/view_declarations.rs b/vhdl_lang/src/analysis/tests/view_declarations.rs index e9134d7c..cd88868b 100644 --- a/vhdl_lang/src/analysis/tests/view_declarations.rs +++ b/vhdl_lang/src/analysis/tests/view_declarations.rs @@ -466,6 +466,53 @@ end arch; ); } +// GitHub issue #324 +#[test] +fn view_in_generic_package() { + let mut builder = LibraryBuilder::with_standard(VHDL2019); + builder.code( + "libname", + "\ +package test_pkg is + generic ( width : integer ); + type test_t is record + a : bit; + end record; + + view vone of test_t is + a : in; + end view; + alias vtwo is vone'converse; +end package; + +package w8_pkg is new work.test_pkg generic map (width => 8); + +use work.w8_pkg.all; + +entity test_sub_entity is + port ( + my_if : view vone + ); +end entity; + +use work.w8_pkg.all; + +entity test_top_entity is +end entity; + +architecture rtl of test_top_entity is + signal my_sig : test_t; +begin + my_if : entity work.test_sub_entity + port map ( + my_if => my_sig + ); +end architecture; + ", + ); + check_no_diagnostics(&builder.analyze()); +} + #[test] fn arrays_of_views_with_matching_type() { let mut builder = LibraryBuilder::with_standard(VHDL2019); diff --git a/vhdl_lang/src/named_entity.rs b/vhdl_lang/src/named_entity.rs index 5b419c86..9098cfb5 100644 --- a/vhdl_lang/src/named_entity.rs +++ b/vhdl_lang/src/named_entity.rs @@ -26,6 +26,7 @@ pub use attribute::AttributeEnt; mod arena; pub use arena::{Arena, ArenaId, EntityId, FinalArena, Reference}; mod visibility; +pub(crate) use visibility::IntoUnambiguousError; pub use visibility::{Visibility, Visible}; mod region; pub(crate) use region::RegionKind; diff --git a/vhdl_lang/src/named_entity/region.rs b/vhdl_lang/src/named_entity/region.rs index 3c29b753..779a5a58 100644 --- a/vhdl_lang/src/named_entity/region.rs +++ b/vhdl_lang/src/named_entity/region.rs @@ -162,7 +162,7 @@ impl<'a> Region<'a> { ent.error( diagnostics, "Full declaration of deferred constant is only allowed in a package body", - ErrorCode::IllegalDeferredConstant + ErrorCode::IllegalDeferredConstant, ); } else { *prev_ent = ent; diff --git a/vhdl_lang/src/named_entity/visibility.rs b/vhdl_lang/src/named_entity/visibility.rs index baba2eff..c7192649 100644 --- a/vhdl_lang/src/named_entity/visibility.rs +++ b/vhdl_lang/src/named_entity/visibility.rs @@ -16,6 +16,54 @@ struct VisibleEntity<'a> { entity: EntRef<'a>, } +#[derive(Debug)] +pub enum ConflictingName { + MadeVisible, + Declared, +} + +#[derive(Debug)] +pub struct IntoUnambiguousError { + designator: Designator, + conflicting_names: Vec<(SrcPos, ConflictingName)>, +} + +impl IntoUnambiguousError { + pub fn new(designator: Designator) -> IntoUnambiguousError { + IntoUnambiguousError { + designator, + conflicting_names: Vec::new(), + } + } + + pub fn add_conflicting(&mut self, pos: SrcPos, name: ConflictingName) { + self.conflicting_names.push((pos, name)) + } + + pub fn into_diagnostic(self, ctx: &dyn TokenAccess, span: TokenSpan) -> Diagnostic { + let mut error = Diagnostic::new( + span.pos(ctx), + format!( + "Name '{}' is hidden by conflicting use clause", + self.designator + ), + ErrorCode::ConflictingUseClause, + ); + for (pos, name) in self.conflicting_names { + let msg = match name { + ConflictingName::MadeVisible => { + format!("Conflicting name '{}' made visible here", self.designator) + } + ConflictingName::Declared => { + format!("Conflicting name '{}' declared here", self.designator) + } + }; + error.add_related(pos, msg) + } + error + } +} + impl<'a> VisibleEntity<'a> { fn clone_with_more_visiblity(&self, visible_pos: Option<&SrcPos>) -> VisibleEntity<'a> { let mut more = self.clone(); @@ -185,10 +233,8 @@ impl<'a> Visible<'a> { pub fn into_unambiguous( self, - ctx: &dyn TokenAccess, - span: TokenSpan, designator: &Designator, - ) -> Result>, Diagnostic> { + ) -> Result>, IntoUnambiguousError> { let mut named_entities: Vec<_> = self .visible_entities .values() @@ -207,12 +253,8 @@ impl<'a> Visible<'a> { } else if named_entities.len() == 1 { Ok(Some(NamedEntities::new(named_entities.pop().unwrap()))) } else { + let mut error = IntoUnambiguousError::new(designator.clone()); // Duplicate visible items hide each other - let mut error = Diagnostic::new( - span.pos(ctx), - format!("Name '{designator}' is hidden by conflicting use clause"), - ErrorCode::ConflictingUseClause, - ); fn last_visible_pos(visible_entity: &VisibleEntity<'_>) -> u32 { if let Some(pos) = visible_entity.visible_pos.iter().rev().flatten().next() { @@ -227,16 +269,10 @@ impl<'a> Visible<'a> { for visible_entity in visible_entities { for visible_pos in visible_entity.visible_pos.iter().rev().flatten() { - error.add_related( - visible_pos, - format!("Conflicting name '{designator}' made visible here"), - ); + error.add_conflicting(visible_pos.clone(), ConflictingName::MadeVisible); } if let Some(pos) = visible_entity.entity.decl_pos() { - error.add_related( - pos, - format!("Conflicting name '{designator}' declared here"), - ); + error.add_conflicting(pos.clone(), ConflictingName::Declared); } }