From 9585903ce9232f5d98f176fa1daa6804bea0f34a Mon Sep 17 00:00:00 2001
From: Mikael Henriksson <mike.zx@hotmail.com>
Date: Wed, 10 May 2023 13:37:20 +0200
Subject: [PATCH] add support for testing valid VHDL identifiers (closes #246)

---
 b_asic/architecture.py                        |   9 +-
 b_asic/codegen/vhdl/common.py                 | 167 +++++++++++++++++-
 b_asic/resources.py                           |  25 ++-
 .../secondorderdirectformiir_architecture.py  |   4 +-
 test/test_codegen.py                          |  38 ++++
 5 files changed, 226 insertions(+), 17 deletions(-)
 create mode 100644 test/test_codegen.py

diff --git a/b_asic/architecture.py b/b_asic/architecture.py
index cc73a389..2a9a6074 100644
--- a/b_asic/architecture.py
+++ b/b_asic/architecture.py
@@ -8,6 +8,7 @@ from typing import Dict, Iterable, Iterator, List, Optional, Set, Tuple, Union,
 import matplotlib.pyplot as plt
 from graphviz import Digraph
 
+from b_asic.codegen.vhdl.common import is_valid_vhdl_identifier
 from b_asic.port import InputPort, OutputPort
 from b_asic.process import MemoryVariable, OperatorProcess, PlainMemoryVariable
 from b_asic.resources import ProcessCollection
@@ -42,10 +43,8 @@ class HardwareBlock:
         entity_name : str
             The entity name.
         """
-        # Should be a better check.
-        # See https://stackoverflow.com/questions/7959587/regex-for-vhdl-identifier
-        if " " in entity_name:
-            raise ValueError("Cannot have space in entity name")
+        if not is_valid_vhdl_identifier(entity_name):
+            raise ValueError(f'{entity_name} is not a valid VHDL indentifier')
         self._entity_name = entity_name
 
     def write_code(self, path: str) -> None:
@@ -57,7 +56,7 @@ class HardwareBlock:
         path : str
             Directory to write code in.
         """
-        if not self._entity_name:
+        if not self.entity_name:
             raise ValueError("Entity name must be set")
         raise NotImplementedError
 
diff --git a/b_asic/codegen/vhdl/common.py b/b_asic/codegen/vhdl/common.py
index 5e51e20b..1fc188b2 100644
--- a/b_asic/codegen/vhdl/common.py
+++ b/b_asic/codegen/vhdl/common.py
@@ -2,6 +2,7 @@
 Generation of common VHDL constructs
 """
 
+import re
 from datetime import datetime
 from io import TextIOWrapper
 from subprocess import PIPE, Popen
@@ -350,11 +351,11 @@ def synchronous_memory(
     assert len(read_ports) >= 1
     assert len(write_ports) >= 1
     synchronous_process_prologue(f, clk=clk, name=name)
-    for read_name, address, re in read_ports:
+    for read_name, address, read_enable in read_ports:
         vhdl.write_lines(
             f,
             [
-                (3, f'if {re} = \'1\' then'),
+                (3, f'if {read_enable} = \'1\' then'),
                 (4, f'{read_name} <= memory({address});'),
                 (3, 'end if;'),
             ],
@@ -409,3 +410,165 @@ def asynchronous_read_memory(
     synchronous_process_epilogue(f, clk=clk, name=name)
     for read_name, address, _ in read_ports:
         vhdl.write(f, 1, f'{read_name} <= memory({address});')
+
+
+def is_valid_vhdl_identifier(identifier: str) -> bool:
+    """
+    Test if identifier is a valid VHDL identifier, as specified by VHDL 2019.
+
+    An indentifier is a valid VHDL identifier if it is not a VHDL reserved keyword and
+    it is a valid basic identifier as specified by IEEE STD 1076-2019 (VHDL standard).
+
+    Parameters
+    ----------
+    identifier : str
+        The identifier to test.
+
+    Returns
+    -------
+    Returns True if identifier is a valid VHDL identifier, False otherwise.
+    """
+    # IEEE STD 1076-2019:
+    # Sec. 15.4.2, Basic identifiers:
+    # * A basic identifier consists only of letters, digits, and underlines.
+    # * A basic identifier is not a reserved VHDL keyword
+    is_basic_identifier = (
+        re.fullmatch(pattern=r'[a-zA-Z][0-9a-zA-Z_]*', string=identifier) is not None
+    )
+    return is_basic_identifier and not is_vhdl_reserved_keyword(identifier)
+
+
+def is_vhdl_reserved_keyword(identifier: str) -> bool:
+    """
+    Test if identifier is a reserved VHDL keyword.
+
+    Parameters
+    ----------
+    identifier : str
+        The identifier to test.
+
+    Returns
+    -------
+    Returns True if identifier is reserved, False otherwise.
+    """
+    # List of reserved keyword in IEEE STD 1076-2019.
+    # Sec. 15.10, Reserved words:
+    reserved_keywords = (
+        "abs",
+        "access",
+        "after",
+        "alias",
+        "all",
+        "and",
+        "architecture",
+        "array",
+        "assert",
+        "assume",
+        "attribute",
+        "begin",
+        "block",
+        "body",
+        "buffer",
+        "bus",
+        "case",
+        "component",
+        "configuration",
+        "constant",
+        "context",
+        "cover",
+        "default",
+        "disconnect",
+        "downto",
+        "else",
+        "elsif",
+        "end",
+        "entity",
+        "exit",
+        "fairness",
+        "file",
+        "for",
+        "force",
+        "function",
+        "generate",
+        "generic",
+        "group",
+        "guarded",
+        "if",
+        "impure",
+        "in",
+        "inertial",
+        "inout",
+        "is",
+        "label",
+        "library",
+        "linkage",
+        "literal",
+        "loop",
+        "map",
+        "mod",
+        "nand",
+        "new",
+        "next",
+        "nor",
+        "not",
+        "null",
+        "of",
+        "on",
+        "open",
+        "or",
+        "others",
+        "out",
+        "package",
+        "parameter",
+        "port",
+        "postponed",
+        "procedure",
+        "process",
+        "property",
+        "protected",
+        "private",
+        "pure",
+        "range",
+        "record",
+        "register",
+        "reject",
+        "release",
+        "rem",
+        "report",
+        "restrict",
+        "return",
+        "rol",
+        "ror",
+        "select",
+        "sequence",
+        "severity",
+        "signal",
+        "shared",
+        "sla",
+        "sll",
+        "sra",
+        "srl",
+        "strong",
+        "subtype",
+        "then",
+        "to",
+        "transport",
+        "type",
+        "unaffected",
+        "units",
+        "until",
+        "use",
+        "variable",
+        "view",
+        "vpkg",
+        "vmode",
+        "vprop",
+        "vunit",
+        "wait",
+        "when",
+        "while",
+        "with",
+        "xnor",
+        "xor",
+    )
+    return identifier.lower() in reserved_keywords
diff --git a/b_asic/resources.py b/b_asic/resources.py
index e6f27bb4..a56bf32a 100644
--- a/b_asic/resources.py
+++ b/b_asic/resources.py
@@ -10,6 +10,7 @@ from matplotlib.axes import Axes
 from matplotlib.ticker import MaxNLocator
 
 from b_asic._preferences import LATENCY_COLOR, WARNING_COLOR
+from b_asic.codegen.vhdl.common import is_valid_vhdl_identifier
 from b_asic.process import MemoryVariable, OperatorProcess, PlainMemoryVariable, Process
 from b_asic.types import TypeName
 
@@ -818,7 +819,7 @@ class ProcessCollection:
         self,
         heuristic: str = "graph_color",
         coloring_strategy: str = "saturation_largest_first",
-    ) -> Set["ProcessCollection"]:
+    ) -> List["ProcessCollection"]:
         """
         Split a ProcessCollection based on overlapping execution time.
 
@@ -843,7 +844,7 @@ class ProcessCollection:
 
         Returns
         -------
-        A set of new ProcessCollection objects with the process splitting.
+        A list of new ProcessCollection objects with the process splitting.
         """
         if heuristic == "graph_color":
             exclusion_graph = self.create_exclusion_graph_from_execution_time()
@@ -862,7 +863,7 @@ class ProcessCollection:
         read_ports: Optional[int] = None,
         write_ports: Optional[int] = None,
         total_ports: Optional[int] = None,
-    ) -> Set["ProcessCollection"]:
+    ) -> List["ProcessCollection"]:
         """
         Split this process storage based on concurrent read/write times according.
 
@@ -907,7 +908,7 @@ class ProcessCollection:
         write_ports: int,
         total_ports: int,
         coloring_strategy: str = "saturation_largest_first",
-    ) -> Set["ProcessCollection"]:
+    ) -> List["ProcessCollection"]:
         """
         Parameters
         ----------
@@ -944,7 +945,7 @@ class ProcessCollection:
     def _split_from_graph_coloring(
         self,
         coloring: Dict[Process, int],
-    ) -> Set["ProcessCollection"]:
+    ) -> List["ProcessCollection"]:
         """
         Split :class:`Process` objects into a set of :class:`ProcessesCollection`
         objects based on a provided graph coloring.
@@ -964,10 +965,10 @@ class ProcessCollection:
         process_collection_set_list = [set() for _ in range(max(coloring.values()) + 1)]
         for process, color in coloring.items():
             process_collection_set_list[color].add(process)
-        return {
+        return [
             ProcessCollection(process_collection_set, self._schedule_time, self._cyclic)
             for process_collection_set in process_collection_set_list
-        }
+        ]
 
     def _repr_svg_(self) -> str:
         """
@@ -1076,7 +1077,7 @@ class ProcessCollection:
         filename: str,
         entity_name: str,
         word_length: int,
-        assignment: Set['ProcessCollection'],
+        assignment: List['ProcessCollection'],
         read_ports: int = 1,
         write_ports: int = 1,
         total_ports: int = 2,
@@ -1116,6 +1117,10 @@ class ProcessCollection:
             (which is added automatically). For large interleavers, this can improve
             timing significantly.
         """
+        # Check that entity name is a valid VHDL identifier
+        if not is_valid_vhdl_identifier(entity_name):
+            raise KeyError(f'{entity_name} is not a valid identifier')
+
         # Check that this is a ProcessCollection of (Plain)MemoryVariables
         is_memory_variable = all(
             isinstance(process, MemoryVariable) for process in self._collection
@@ -1249,6 +1254,10 @@ class ProcessCollection:
             The total number of ports used when splitting process collection based on
             memory variable access.
         """
+        # Check that entity name is a valid VHDL identifier
+        if not is_valid_vhdl_identifier(entity_name):
+            raise KeyError(f'{entity_name} is not a valid identifier')
+
         # Check that this is a ProcessCollection of (Plain)MemoryVariables
         is_memory_variable = all(
             isinstance(process, MemoryVariable) for process in self._collection
diff --git a/examples/secondorderdirectformiir_architecture.py b/examples/secondorderdirectformiir_architecture.py
index 2cb20224..f4e240a4 100644
--- a/examples/secondorderdirectformiir_architecture.py
+++ b/examples/secondorderdirectformiir_architecture.py
@@ -64,8 +64,8 @@ outputs.show(title="Output executions")
 
 p1 = ProcessingElement(adders, entity_name="adder")
 p2 = ProcessingElement(mults, entity_name="cmul")
-p_in = ProcessingElement(inputs, entity_name='in')
-p_out = ProcessingElement(outputs, entity_name='out')
+p_in = ProcessingElement(inputs, entity_name='input')
+p_out = ProcessingElement(outputs, entity_name='output')
 
 # %%
 # Extract memory variables
diff --git a/test/test_codegen.py b/test/test_codegen.py
new file mode 100644
index 00000000..b52be275
--- /dev/null
+++ b/test/test_codegen.py
@@ -0,0 +1,38 @@
+from b_asic.codegen.vhdl.common import is_valid_vhdl_identifier
+
+
+def test_is_valid_vhdl_identifier():
+    identifier_pass = {
+        "COUNT",
+        "X",
+        "c_out",
+        "FFT",
+        "Decoder",
+        "VHSIC",
+        "X1",
+        "PageCount",
+        "STORE_NEXT_ITEM",
+        "ValidIdentifier123",
+        "valid_identifier",
+    }
+    identifier_fail = {
+        "",
+        "architecture",
+        "Architecture",
+        "ArChItEctUrE",
+        "architectURE",
+        "entity",
+        "invalid+",
+        "invalid}",
+        "not-valid",
+        "(invalid)",
+        "invalid£",
+        "1nvalid",
+        "_abc",
+    }
+
+    for identifier in identifier_pass:
+        assert is_valid_vhdl_identifier(identifier)
+
+    for identifier in identifier_fail:
+        assert not is_valid_vhdl_identifier(identifier)
-- 
GitLab