From df85e7eb6ad73d4aa1cd000a3fea1c5db388306e Mon Sep 17 00:00:00 2001 From: Alex Mykyta Date: Tue, 7 Jan 2025 23:00:25 -0800 Subject: [PATCH] Add type hinting consistency test. Fix Root.external value unset --- src/systemrdl/compiler.py | 1 + src/systemrdl/node.py | 2 +- test/lib/__init__.py | 0 test/lib/type_hint_utils.py | 70 +++++++ .../prop_ref_err.rdl} | 41 ---- test/rdl_src/prop_ref-in_array.rdl | 9 + test/rdl_src/prop_ref-inferred_vector.rdl | 13 ++ test/rdl_src/prop_ref-value_ref.rdl | 16 ++ test/run.sh | 2 +- test/test_api_type_hints.py | 179 ++++++++++++++++++ test/test_prop_refs.py | 18 +- 11 files changed, 299 insertions(+), 52 deletions(-) create mode 100644 test/lib/__init__.py create mode 100644 test/lib/type_hint_utils.py rename test/{rdl_src/prop_ref.rdl => rdl_err_src/prop_ref_err.rdl} (59%) create mode 100644 test/rdl_src/prop_ref-in_array.rdl create mode 100644 test/rdl_src/prop_ref-inferred_vector.rdl create mode 100644 test/rdl_src/prop_ref-value_ref.rdl create mode 100644 test/test_api_type_hints.py diff --git a/src/systemrdl/compiler.py b/src/systemrdl/compiler.py index b07a625..c246966 100644 --- a/src/systemrdl/compiler.py +++ b/src/systemrdl/compiler.py @@ -369,6 +369,7 @@ def elaborate(self, top_def_name: Optional[str]=None, inst_name: Optional[str]=N root_inst.is_instance = True root_inst.original_def = self.root root_inst.inst_name = "$root" + root_inst.external = False # meaningless, but must not be None # Create a top-level instance top_inst = top_def._copy_for_inst({}) diff --git a/src/systemrdl/node.py b/src/systemrdl/node.py index 2bd2e2b..85baac9 100644 --- a/src/systemrdl/node.py +++ b/src/systemrdl/node.py @@ -2181,7 +2181,7 @@ def aliases(self, skip_not_present: bool = True) -> List['RegNode']: #=============================================================================== class RegfileNode(AddressableNode): - parent: 'AddrmapNode' + parent: Union['AddrmapNode', 'RegfileNode'] inst: comp.Regfile @overload # type: ignore[override] diff --git a/test/lib/__init__.py b/test/lib/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test/lib/type_hint_utils.py b/test/lib/type_hint_utils.py new file mode 100644 index 0000000..ceaa5ba --- /dev/null +++ b/test/lib/type_hint_utils.py @@ -0,0 +1,70 @@ +import typing +import inspect + +def hint_is_a_generic(hint) -> bool: + """ + Typing generics have an __origin__ attribute + """ + if "__origin__" in dir(hint): + return True + return False + +def hint_is(hint, generic_origin) -> bool: + """ + Compare whether the hint matches a generic origin + """ + if not hint_is_a_generic(hint): + return False + return hint.__origin__ == generic_origin + + +def value_is_compatible(value, hint) -> bool: + """ + Given any value, check whether it is compatible with a type hint annotation + """ + if hint_is_a_generic(hint): + # Unpack the generic + if hint_is(hint, typing.Union): + # Check if value matches any of the types in the Union + for arg_hint in hint.__args__: + if value_is_compatible(value, arg_hint): + return True + return False + elif hint_is(hint, type): + if not inspect.isclass(value): + return False + expected_type = hint.__args__[0] + return issubclass(value, expected_type) + elif hint_is(hint, list): + # Check if value is a list + if inspect.isclass(value): + return False + if not isinstance(value, list): + return False + # Check that all members of the list match the expected list type + expected_hint = hint.__args__[0] + for element in value: + if not value_is_compatible(element, expected_hint): + return False + return True + elif hint_is(hint, dict): + # Check if value is a dict + if inspect.isclass(value): + return False + if not isinstance(value, dict): + return False + expected_key_hint, expected_value_hint = hint.__args__ + # Check that all keys match the expected type + for key in value.keys(): + if not value_is_compatible(key, expected_key_hint): + return False + # Check that all values match the expected type + for element in value.values(): + if not value_is_compatible(element, expected_value_hint): + return False + return True + else: + raise RuntimeError(f"Unhandled generic {hint}: {hint.__origin__}") + + # hint is an actual class + return isinstance(value, hint) diff --git a/test/rdl_src/prop_ref.rdl b/test/rdl_err_src/prop_ref_err.rdl similarity index 59% rename from test/rdl_src/prop_ref.rdl rename to test/rdl_err_src/prop_ref_err.rdl index fb290c6..a367086 100644 --- a/test/rdl_src/prop_ref.rdl +++ b/test/rdl_err_src/prop_ref_err.rdl @@ -1,30 +1,3 @@ - -addrmap prop_value_ref { - reg { - default sw=rw; - default hw=rw; - field {} a = 0; - field {} b; - field {} c; - field {} d; - field {} e; - b->next = a->reset; - c->next = a->reset; - d->next = a->anded; - e->next = b->anded; - }y; -}; - -addrmap ref_in_array { - reg { - default sw=rw; - default hw=rw; - field {} a = 0; - field {} b = 0; - b->next = a->anded; - } myreg[8]; -}; - addrmap err_missing_reset { reg { default sw=rw; @@ -55,20 +28,6 @@ addrmap err_self_reset { }y; }; -addrmap inferred_vector { - reg { - default sw=rw; - default hw=rw; - field { - sw=rw; hw=w; we; - } a; - field {} b; - field {} c; - b->next = a->we; - c->next = a->wel; - }y; -}; - addrmap err_no_inferred { reg { default sw=rw; diff --git a/test/rdl_src/prop_ref-in_array.rdl b/test/rdl_src/prop_ref-in_array.rdl new file mode 100644 index 0000000..1d0d116 --- /dev/null +++ b/test/rdl_src/prop_ref-in_array.rdl @@ -0,0 +1,9 @@ +addrmap ref_in_array { + reg { + default sw=rw; + default hw=rw; + field {} a = 0; + field {} b = 0; + b->next = a->anded; + } myreg[8]; +}; diff --git a/test/rdl_src/prop_ref-inferred_vector.rdl b/test/rdl_src/prop_ref-inferred_vector.rdl new file mode 100644 index 0000000..b2c22df --- /dev/null +++ b/test/rdl_src/prop_ref-inferred_vector.rdl @@ -0,0 +1,13 @@ +addrmap inferred_vector { + reg { + default sw=rw; + default hw=rw; + field { + sw=rw; hw=w; we; + } a; + field {} b; + field {} c; + b->next = a->we; + c->next = a->wel; + }y; +}; diff --git a/test/rdl_src/prop_ref-value_ref.rdl b/test/rdl_src/prop_ref-value_ref.rdl new file mode 100644 index 0000000..0a3e53d --- /dev/null +++ b/test/rdl_src/prop_ref-value_ref.rdl @@ -0,0 +1,16 @@ + +addrmap prop_value_ref { + reg { + default sw=rw; + default hw=rw; + field {} a = 0; + field {} b; + field {} c; + field {} d; + field {} e; + b->next = a->reset; + c->next = a->reset; + d->next = a->anded; + e->next = b->anded; + }y; +}; diff --git a/test/run.sh b/test/run.sh index b6c8fe4..361bd7f 100755 --- a/test/run.sh +++ b/test/run.sh @@ -15,7 +15,7 @@ if exists ccache; then fi # Initialize venv -python3 -m venv .venv +python3.11 -m venv .venv source .venv/bin/activate # Install diff --git a/test/test_api_type_hints.py b/test/test_api_type_hints.py new file mode 100644 index 0000000..fb316fa --- /dev/null +++ b/test/test_api_type_hints.py @@ -0,0 +1,179 @@ +import glob +import os +from typing import Union, List, Optional + +from typing_extensions import Literal, get_overloads, get_type_hints +from parameterized import parameterized_class + +from lib.type_hint_utils import value_is_compatible, hint_is +from systemrdl.node import AddressableNode, FieldNode, MemNode, Node, AddrmapNode, RegNode, RegfileNode, RootNode, SignalNode, VectorNode +from systemrdl import component as comp +from unittest_utils import RDLSourceTestCase + +from systemrdl.walker import RDLListener, RDLWalker, WalkerAction + +# Get all RDL sources +rdl_src_files = glob.glob("rdl_src/*.rdl") + +# Exclude some files as root compile targets: +exclude = { + "rdl_src/preprocessor_incl.rdl", # is an include file + "rdl_src/udp_builtin.rdl", # uses builtins +} + +# Build testcase list +cases = [] +for file in rdl_src_files: + if file in exclude: + continue + args = { + "name": os.path.basename(file), + "src": file, + } + cases.append(args) + +@parameterized_class(cases) +class TestTypeHints(RDLSourceTestCase): + def check_node_properties(self, node): + """ + Check that each of the node's properties returns a value that matches + the expected annotated type hint + """ + gp_overloads = get_overloads(node.get_property) + self.assertGreater(len(gp_overloads), 0) + + for gp_overload in gp_overloads: + hints = get_type_hints(gp_overload) + + # Skip overloads that use default override signature: + # get_property(*, default=...) + if "default" in hints: + continue + + # Skip generic overloads that do not specify an explicit property + if not hint_is(hints["prop_name"], Literal): + continue + + # Currently assuming only one arg to Literal + self.assertEqual(len(hints["prop_name"].__args__), 1) + property_name = hints["prop_name"].__args__[0] + #print(f"Checking {node.get_path()}->{property_name}") + + value = node.get_property(property_name) + self.assertTrue( + value_is_compatible(value, hints["return"]), + f"Value '{value}' does not match expected type: {hints['return']}. " + f"for: {node.get_path()}->{property_name}" + ) + + def assert_attr_type_hint(self, node, attr_name, hint): + """ + Assert a node's attribute matches the expected type hint + """ + value = getattr(node, attr_name) + self.assertTrue( + value_is_compatible(value, hint), + f"Value '{value}' does not match expected type: {hint}." + f"for: {node.get_path()}::{attr_name}" + ) + + def test_all_nodes(self): + root = self.compile( + [self.src], + incl_search_paths=["rdl_src/incdir"] + ) + + walker = RDLWalker(skip_not_present=False) + listener = RDLTestListener(self) + walker.walk(root, listener) + + # Test root itself + self.assertIsNone(root.parent) + self.assert_attr_type_hint(root, "inst", comp.Root) + self.assert_attr_type_hint(root, "inst_name", str) + self.assert_attr_type_hint(root, "type_name", Optional[str]) + self.assert_attr_type_hint(root, "orig_type_name", Optional[str]) + self.assert_attr_type_hint(root, "external", bool) + +class RDLTestListener(RDLListener): + def __init__(self, test_class: TestTypeHints) -> None: + super().__init__() + self.test_class = test_class + + def enter_Component(self, node: Node) -> None: + self.test_class.check_node_properties(node) + self.test_class.assert_attr_type_hint(node, "owning_addrmap", Optional[AddrmapNode]) + self.test_class.assert_attr_type_hint(node, "inst_name", str) + self.test_class.assert_attr_type_hint(node, "type_name", Optional[str]) + self.test_class.assert_attr_type_hint(node, "orig_type_name", Optional[str]) + self.test_class.assert_attr_type_hint(node, "external", bool) + + def enter_AddressableComponent(self, node: AddressableNode) -> None: + self.test_class.assert_attr_type_hint(node, "raw_address_offset", int) + self.test_class.assert_attr_type_hint(node, "raw_absolute_address", int) + self.test_class.assert_attr_type_hint(node, "size", int) + self.test_class.assert_attr_type_hint(node, "total_size", int) + if node.is_array: + self.test_class.assert_attr_type_hint(node, "array_dimensions", List[int]) + self.test_class.assert_attr_type_hint(node, "array_stride", int) + else: + self.test_class.assertIsNone(node.array_dimensions) + self.test_class.assertIsNone(node.array_stride) + + + def enter_VectorComponent(self, node: VectorNode) -> None: + self.test_class.assert_attr_type_hint(node, "width", int) + self.test_class.assert_attr_type_hint(node, "msb", int) + self.test_class.assert_attr_type_hint(node, "lsb", int) + self.test_class.assert_attr_type_hint(node, "high", int) + self.test_class.assert_attr_type_hint(node, "low", int) + + def enter_Signal(self, node: SignalNode) -> None: + self.test_class.assert_attr_type_hint(node, "parent", Node) + self.test_class.assert_attr_type_hint(node, "inst", comp.Signal) + + def enter_Field(self, node: FieldNode) -> None: + self.test_class.assert_attr_type_hint(node, "parent", RegNode) + self.test_class.assert_attr_type_hint(node, "inst", comp.Field) + self.test_class.assert_attr_type_hint(node, "is_virtual", bool) + self.test_class.assert_attr_type_hint(node, "is_volatile", bool) + self.test_class.assert_attr_type_hint(node, "is_sw_writable", bool) + self.test_class.assert_attr_type_hint(node, "is_sw_readable", bool) + self.test_class.assert_attr_type_hint(node, "is_hw_writable", bool) + self.test_class.assert_attr_type_hint(node, "is_hw_readable", bool) + self.test_class.assert_attr_type_hint(node, "implements_storage", bool) + self.test_class.assert_attr_type_hint(node, "is_up_counter", bool) + self.test_class.assert_attr_type_hint(node, "is_down_counter", bool) + self.test_class.assert_attr_type_hint(node, "is_alias", bool) + self.test_class.assert_attr_type_hint(node, "has_aliases", bool) + + def enter_Reg(self, node: RegNode) -> None: + self.test_class.assert_attr_type_hint(node, "parent", Union[AddrmapNode, RegfileNode, MemNode]) + self.test_class.assert_attr_type_hint(node, "inst", comp.Reg) + self.test_class.assert_attr_type_hint(node, "size", int) + self.test_class.assert_attr_type_hint(node, "is_virtual", bool) + self.test_class.assert_attr_type_hint(node, "has_sw_writable", bool) + self.test_class.assert_attr_type_hint(node, "has_sw_readable", bool) + self.test_class.assert_attr_type_hint(node, "has_hw_writable", bool) + self.test_class.assert_attr_type_hint(node, "has_hw_readable", bool) + self.test_class.assert_attr_type_hint(node, "is_interrupt_reg", bool) + self.test_class.assert_attr_type_hint(node, "is_halt_reg", bool) + self.test_class.assert_attr_type_hint(node, "is_alias", bool) + self.test_class.assert_attr_type_hint(node, "has_aliases", bool) + + def enter_Regfile(self, node: RegfileNode) -> None: + self.test_class.assert_attr_type_hint(node, "parent", Union[AddrmapNode, RegfileNode]) + self.test_class.assert_attr_type_hint(node, "inst", comp.Regfile) + self.test_class.assert_attr_type_hint(node, "size", int) + + def enter_Addrmap(self, node: AddrmapNode) -> None: + self.test_class.assert_attr_type_hint(node, "parent", Union[AddrmapNode, RootNode]) + self.test_class.assert_attr_type_hint(node, "inst", comp.Addrmap) + self.test_class.assert_attr_type_hint(node, "size", int) + + def enter_Mem(self, node: MemNode) -> None: + self.test_class.assert_attr_type_hint(node, "parent", AddrmapNode) + self.test_class.assert_attr_type_hint(node, "inst", comp.Mem) + self.test_class.assert_attr_type_hint(node, "size", int) + self.test_class.assert_attr_type_hint(node, "is_sw_writable", bool) + self.test_class.assert_attr_type_hint(node, "is_sw_readable", bool) diff --git a/test/test_prop_refs.py b/test/test_prop_refs.py index d9870c8..145e57f 100644 --- a/test/test_prop_refs.py +++ b/test/test_prop_refs.py @@ -4,7 +4,7 @@ class TestPropRefs(RDLSourceTestCase): def test_prop_value_ref(self): root = self.compile( - ["rdl_src/prop_ref.rdl"], + ["rdl_src/prop_ref-value_ref.rdl"], "prop_value_ref" ) top = root.top @@ -37,7 +37,7 @@ def test_prop_value_ref(self): def test_prop_ref_in_array(self): root = self.compile( - ["rdl_src/prop_ref.rdl"], + ["rdl_src/prop_ref-in_array.rdl"], "ref_in_array" ) a0 = root.find_by_path("ref_in_array.myreg[0].a") @@ -56,7 +56,7 @@ def test_prop_ref_in_array(self): def test_inferred_vector(self): root = self.compile( - ["rdl_src/prop_ref.rdl"], + ["rdl_src/prop_ref-inferred_vector.rdl"], "inferred_vector" ) a = root.find_by_path("inferred_vector.y.a") @@ -72,7 +72,7 @@ def test_inferred_vector(self): def test_err_missing_reset(self): self.assertRDLCompileError( - ["rdl_src/prop_ref.rdl"], + ["rdl_err_src/prop_ref_err.rdl"], "err_missing_reset", r"Assignment references the value of property 'reset', but its value was never set for instance 'a'" ) @@ -80,21 +80,21 @@ def test_err_missing_reset(self): def test_err_circular_ref(self): self.assertRDLCompileError( - ["rdl_src/prop_ref.rdl"], + ["rdl_err_src/prop_ref_err.rdl"], "err_circular_ref", r"Assignment creates a circular reference" ) def test_err_self_reset(self): self.assertRDLCompileError( - ["rdl_src/prop_ref.rdl"], + ["rdl_err_src/prop_ref_err.rdl"], "err_self_reset", r"Field 'a' cannot reference itself in reset property" ) def test_err_no_inferred(self): self.assertRDLCompileError( - ["rdl_src/prop_ref.rdl"], + ["rdl_err_src/prop_ref_err.rdl"], "err_no_inferred", r"Assignment references property 'we', but the signal it represents was never defined or enabled for instance 'a'" ) @@ -102,7 +102,7 @@ def test_err_no_inferred(self): def test_err_not_a_counter(self): self.assertRDLCompileError( - ["rdl_src/prop_ref.rdl"], + ["rdl_err_src/prop_ref_err.rdl"], "err_not_a_counter", r"Reference to property 'incr' is illegal because 'a' is not a counter" ) @@ -110,7 +110,7 @@ def test_err_not_a_counter(self): def test_err_no_counter_threshold(self): self.assertRDLCompileError( - ["rdl_src/prop_ref.rdl"], + ["rdl_err_src/prop_ref_err.rdl"], "err_no_counter_threshold", r"Reference to property 'incrthreshold' is illegal because the target field does not define any thresholds" )