core: Allowing overload of ClassVar from superclass (#2690)

This is to enhance PR [#2687](#2687), which allowed uppercase `ClassVar` fields on ops. This PR adds support for the case that an uppercase `ClassVar` is defined (without a value) in a superclass and assigned a value in a subclass. This is confirmed by performing the same check as in the other PR. --------- Co-authored-by: n-io <[email protected]>
xdslproject · Jun 6, 2024 · a83b98b · a83b98b
1 parent e8f57bf
commit a83b98b
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 10 deletions.
diff --git a/tests/dialects/test_irdl.py b/tests/dialects/test_irdl.py
@@ -168,3 +168,20 @@ def test_class_var_on_op():
 def test_class_var_on_op_invalid():
     with pytest.raises(PyRDLOpDefinitionError, match="is neither a"):
         irdl_op_definition(MyOpWithClassVarInvalid)
+
+
+class MySuperWithClassVarDef(IRDLOperation):
+    name = "test.super_has_class_var"
+
+    VAR: ClassVar[str]
+
+
+class MySubWithClassVarOverload(MySuperWithClassVarDef):
+    name = "test.super_has_class_var"
+
+    VAR: ClassVar[str] = "hello_world"
+
+
+def test_class_var_on_super():
+    irdl_op_definition(MySuperWithClassVarDef)
+    irdl_op_definition(MySubWithClassVarOverload)
diff --git a/tests/dialects/test_irdl_with_annotations.py b/tests/dialects/test_irdl_with_annotations.py
@@ -28,3 +28,20 @@ def test_class_var_on_op():
 def test_class_var_on_op_invalid():
     with pytest.raises(PyRDLOpDefinitionError, match="is neither a"):
         irdl_op_definition(MyOpWithClassVarInvalid)
+
+
+class MySuperWithClassVarDef(IRDLOperation):
+    name = "test.super_has_class_var"
+
+    VAR: ClassVar[str]
+
+
+class MySubWithClassVarOverload(MySuperWithClassVarDef):
+    name = "test.super_has_class_var"
+
+    VAR: ClassVar[str] = "hello_world"
+
+
+def test_class_var_on_super():
+    irdl_op_definition(MySuperWithClassVarDef)
+    irdl_op_definition(MySubWithClassVarOverload)
diff --git a/xdsl/irdl/irdl.py b/xdsl/irdl/irdl.py
@@ -1451,6 +1451,15 @@ def wrong_field_exception(field_name: str) -> PyRDLOpDefinitionError:
                 "and constants (indicated by uppercase field names) as ClassVar."
             )
 
+        def is_const_classvar(field_name: str, annotations: dict[str, Any]) -> bool:
+            return field_name.isupper() and (
+                get_origin(annotations[field_name]) is ClassVar
+                or (
+                    isinstance(annotations[field_name], str)
+                    and annotations[field_name].startswith("ClassVar")
+                )
+            )
+
         op_def = OpDef(pyrdl_def.name)
 
         # If an operation subclass overrides a superclass field, only keep the definition
@@ -1472,6 +1481,8 @@ def wrong_field_exception(field_name: str) -> PyRDLOpDefinitionError:
             annotations = parent_cls.__annotations__
 
             for field_name in annotations:
+                if is_const_classvar(field_name, annotations):
+                    continue
                 if field_name not in clsdict:
                     raise wrong_field_exception(field_name)
 
@@ -1484,16 +1495,8 @@ def wrong_field_exception(field_name: str) -> PyRDLOpDefinitionError:
                 if field_name in field_names:
                     # already registered value for field name
                     continue
-                if (
-                    field_name in annotations
-                    and field_name.isupper()
-                    and (
-                        get_origin(annotations[field_name]) is ClassVar
-                        or (
-                            isinstance(annotations[field_name], str)
-                            and annotations[field_name].startswith("ClassVar")
-                        )
-                    )
+                if field_name in annotations and is_const_classvar(
+                    field_name, annotations
                 ):
                     continue