Feat: Emit diagnostic when a declaration is not allowed in the region it resides (#224)

Schottkyc137 · web-flow · commit f9d0ee9c92cd · 2023-11-19T09:27:04.000+01:00
* Feat: Emit diagnostic when a declaration is not allowed in the region it resides

* Disallow shared variables in block contexts; allow them elsewhere

* Allow signals in packages

* Refactor: Use AnyEntKind enum instead of DeclarativeContext

* Add copyright header
diff --git a/vhdl_lang/src/analysis/declarative.rs b/vhdl_lang/src/analysis/declarative.rs
@@ -6,14 +6,95 @@
 
 use super::names::*;
 use super::*;
-use crate::ast;
 use crate::ast::*;
 use crate::data::*;
 use crate::named_entity::{Signature, *};
+use crate::{ast, named_entity, HasTokenSpan};
 use analyze::*;
 use fnv::FnvHashMap;
 use std::collections::hash_map::Entry;
 
+impl Declaration {
+    pub fn is_allowed_in_context(&self, parent: &AnyEntKind) -> bool {
+        use Declaration::*;
+        use ObjectClass::*;
+        match parent {
+            AnyEntKind::Design(Design::Architecture(..))
+            | AnyEntKind::Concurrent(Some(Concurrent::Block | Concurrent::Generate)) => matches!(
+                self,
+                Object(ObjectDeclaration {
+                    class: Constant | Signal | SharedVariable,
+                    ..
+                }) | File(_)
+                    | Type(_)
+                    | Component(_)
+                    | Attribute(_)
+                    | Alias(_)
+                    | SubprogramDeclaration(_)
+                    | SubprogramInstantiation(_)
+                    | SubprogramBody(_)
+                    | Use(_)
+                    | Package(_)
+                    | Configuration(_)
+            ),
+            AnyEntKind::Design(Design::Configuration) => {
+                matches!(self, Use(_) | Attribute(ast::Attribute::Specification(_)))
+            }
+            AnyEntKind::Design(Design::Entity(..)) => matches!(
+                self,
+                Object(_)
+                    | File(_)
+                    | Type(_)
+                    | Attribute(_)
+                    | Alias(_)
+                    | SubprogramDeclaration(_)
+                    | SubprogramInstantiation(_)
+                    | SubprogramBody(_)
+                    | Use(_)
+                    | Package(_)
+            ),
+            AnyEntKind::Design(Design::PackageBody | Design::UninstPackage(..))
+            | AnyEntKind::Overloaded(Overloaded::SubprogramDecl(_) | Overloaded::Subprogram(_))
+            | AnyEntKind::Concurrent(Some(Concurrent::Process))
+            | AnyEntKind::Type(named_entity::Type::Protected(..)) => matches!(
+                self,
+                Object(ObjectDeclaration {
+                    class: Constant | Variable | SharedVariable,
+                    ..
+                }) | File(_)
+                    | Type(_)
+                    | Attribute(_)
+                    | Alias(_)
+                    | SubprogramDeclaration(_)
+                    | SubprogramInstantiation(_)
+                    | SubprogramBody(_)
+                    | Use(_)
+                    | Package(_)
+            ),
+            AnyEntKind::Design(Design::Package(..)) => matches!(
+                self,
+                Object(_)
+                    | File(_)
+                    | Type(_)
+                    | Component(_)
+                    | Attribute(_)
+                    | Alias(_)
+                    | SubprogramDeclaration(_)
+                    | SubprogramInstantiation(_)
+                    | Use(_)
+                    | Package(_)
+            ),
+            _ => {
+                // AnyEntKind::Library is used in tests for a generic declarative region
+                if !(cfg!(test) && matches!(parent, AnyEntKind::Library)) {
+                    debug_assert!(false, "Parent should be a declarative region");
+                }
+                true
+            }
+        }
+    }
+}
+
 impl<'a> AnalyzeContext<'a> {
     pub fn analyze_declarative_part(
         &self,
@@ -29,6 +110,13 @@ impl<'a> AnalyzeContext<'a> {
 
             let (decl, remaining) = declarations[i..].split_first_mut().unwrap();
 
+            if !decl.is_allowed_in_context(parent.kind()) {
+                diagnostics.error(
+                    decl.get_pos(self.ctx),
+                    format!("{} declaration not allowed here", decl.describe(),),
+                )
+            }
+
             match decl {
                 Declaration::Type(type_decl) => match type_decl.def {
                     TypeDefinition::Incomplete(ref mut reference) => {
diff --git a/vhdl_lang/src/analysis/names.rs b/vhdl_lang/src/analysis/names.rs
@@ -1645,6 +1645,36 @@ fn plural(singular: &'static str, plural: &'static str, count: usize) -> &'stati
     }
 }
 
+impl Declaration {
+    pub fn describe(&self) -> &'static str {
+        match self {
+            Declaration::Object(ObjectDeclaration { class, .. }) => match class {
+                ObjectClass::Constant => "constant",
+                ObjectClass::Signal => "signal",
+                ObjectClass::Variable => "variable",
+                ObjectClass::SharedVariable => "shared variable",
+            },
+            Declaration::File(_) => "file",
+            Declaration::Type(TypeDeclaration { def, .. }) => match def {
+                TypeDefinition::Subtype(_) => "subtype",
+                _ => "type",
+            },
+            Declaration::Component(_) => "component",
+            Declaration::Attribute(attribute) => match attribute {
+                Attribute::Specification(_) => "attribute specification",
+                Attribute::Declaration(_) => "attribute",
+            },
+            Declaration::Alias(_) => "alias",
+            Declaration::SubprogramDeclaration(_) => "subprogram",
+            Declaration::SubprogramInstantiation(_) => "subprogram instantiation",
+            Declaration::SubprogramBody(_) => "subprogram body",
+            Declaration::Use(_) => "use",
+            Declaration::Package(_) => "package instantiation",
+            Declaration::Configuration(_) => "configuration",
+        }
+    }
+}
+
 impl Diagnostic {
     fn cannot_be_prefix(prefix_pos: &SrcPos, resolved: ResolvedName, suffix: Suffix) -> Diagnostic {
         let suffix_desc = match suffix {
diff --git a/vhdl_lang/src/analysis/tests/declarations.rs b/vhdl_lang/src/analysis/tests/declarations.rs
@@ -0,0 +1,48 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at http://mozilla.org/MPL/2.0/.
+//
+// Copyright (c) 2023, Olof Kraigher olof.kraigher@gmail.com
+use crate::analysis::tests::{check_diagnostics, LibraryBuilder};
+use crate::Diagnostic;
+
+#[test]
+pub fn declaration_not_allowed_everywhere() {
+    let mut builder = LibraryBuilder::new();
+    let code = builder.code(
+        "libname",
+        "\
+entity ent is
+end entity;
+
+architecture arch of ent is
+
+function my_func return natural is
+    signal x : bit;
+begin
+
+end my_func;
+begin
+
+    my_block : block
+        variable y: natural;
+    begin
+    end block my_block;
+
+end architecture;
+    ",
+    );
+    check_diagnostics(
+        builder.analyze(),
+        vec![
+            Diagnostic::error(
+                code.s1("signal x : bit;"),
+                "signal declaration not allowed here",
+            ),
+            Diagnostic::error(
+                code.s1("variable y: natural;"),
+                "variable declaration not allowed here",
+            ),
+        ],
+    )
+}
diff --git a/vhdl_lang/src/analysis/tests/implicit.rs b/vhdl_lang/src/analysis/tests/implicit.rs
@@ -116,13 +116,15 @@ fn deallocate_is_defined_for_access_type() {
 package pkg is
   type arr_t is array (natural range <>) of character;
   type ptr_t is access arr_t;
+end package;
 
+package body pkg is
   procedure theproc is
       variable theptr: ptr_t;
   begin
       deallocate(theptr);
   end procedure;
-end package;
+end package body;
 ",
     );
 }
diff --git a/vhdl_lang/src/analysis/tests/mod.rs b/vhdl_lang/src/analysis/tests/mod.rs
@@ -9,6 +9,7 @@ mod association_formal;
 mod circular_dependencies;
 mod context_clause;
 mod custom_attributes;
+mod declarations;
 mod deferred_constant;
 mod hierarchy;
 mod homographs;
diff --git a/vhdl_lang/src/analysis/tests/protected_type.rs b/vhdl_lang/src/analysis/tests/protected_type.rs
@@ -249,15 +249,19 @@ fn protected_type_body_reference() {
     let code = builder.code(
         "libname",
         "
-package pkg1 is
+entity ent1 is
+end ent1;
+
+architecture arch of ent1 is
   type prot_t is protected
   end protected;
 
   type prot_t is protected body
   end protected body;
 
   shared variable var : prot_t;
-end package;",
+begin
+end architecture;",
     );
 
     let (root, diagnostics) = builder.get_analyzed_root();
diff --git a/vhdl_lang/src/analysis/tests/resolves_type_mark.rs b/vhdl_lang/src/analysis/tests/resolves_type_mark.rs
@@ -428,6 +428,7 @@ signal sig2 : rec.field'subtype;
 fn check_good_type_marks() {
     check_code_with_no_diagnostics(
         "
+
 package gpkg is
   -- Interface type
   generic (type type_t);
@@ -437,7 +438,10 @@ package gpkg is
   end record;
 end package;
 
-package pkg is
+entity ent is
+end entity ent;
+
+architecture arch of ent is
   type incomplete;
   -- Incomplete type
   type access_t is access incomplete;
@@ -461,11 +465,8 @@ package pkg is
   -- Alias of type
   alias alias_t is enum_t;
   constant const2 : alias_t := alpha;
-end package;
-
-package body pkg is
-end package body;
-
+begin
+end architecture;
 ",
     );
 }
diff --git a/vhdl_lang/src/analysis/tests/visibility.rs b/vhdl_lang/src/analysis/tests/visibility.rs
@@ -772,7 +772,7 @@ fn generics_are_visible_in_procedures_but_not_outside() {
       a := b;
       b := temp;
     end procedure swap;
-    variable temp2: T;
+    shared variable temp2: T;
     ",
     );
     let (_, diagnostics) = builder.get_analyzed_root();
