Repository: amaranth-lang/amaranth
Pull request #832 (2023-07-02 11:45:54)

lib.enum: allow empty enums.
Fixes the error noted in https://github.com/amaranth-lang/amaranth/pull/830#issuecomment-1613690529 where empty Python `Enum`s are shape-castable to `unsigned(0)`.

~~We move the member check from the Amaranth `Enum` implicit shape-cast into the Python `Enum` shape-cast, which the former backs onto.~~

Allows empty Amaranth `Enum`s to shape-cast to `unsigned(0)`, like Python `Enum`s, per https://github.com/amaranth-lang/amaranth/pull/832#issuecomment-1617639806.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/832?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) Report
> Merging [#832](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/832?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) (431b17d) into [main](https://app.codecov.io/gh/amaranth-lang/amaranth/commit/f4b013ac7338923230aa03929a69ab5761a98ef7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) (f4b013a) will **not change** coverage.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##             main     #832   +/-   ##
=======================================
  Coverage   82.80%   82.80%           
=======================================
  Files          52       52           
  Lines        7082     7082           
  Branches     1696     1696           
=======================================
  Hits         5864     5864           
  Misses       1027     1027           
  Partials      191      191           
```


| [Impacted Files](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/832?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) | Coverage Δ | |
|---|---|---|
| [amaranth/hdl/ast.py](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/832?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang#diff-YW1hcmFudGgvaGRsL2FzdC5weQ==) | `89.85% <100.00%> (+0.02%)` | :arrow_up: |
| [amaranth/lib/enum.py](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/832?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang#diff-YW1hcmFudGgvbGliL2VudW0ucHk=) | `100.00% <100.00%> (ø)` | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang)

whitequark: I was actually thinking of allowing it everywhere instead. (This would technically be a breaking change, but even ignoring that, I can't think of any reason to disallow such enums. Do you?)
charlottia: I think it's totally fine! I'll see if anything comes up.
charlottia: Seems OK. https://github.com/amaranth-lang/amaranth/pull/830#issuecomment-1613690529 mentions inheritance concerns (presumably because the base enum is also an empty enum and so things can get weird), but I can't think of a scenario that's properly impacted by allowing this.

Patch:
diff --git a/amaranth/lib/enum.py b/amaranth/lib/enum.py
index 61e5f28..10c3666 100644
--- a/amaranth/lib/enum.py
+++ b/amaranth/lib/enum.py
@@ -112,16 +112,11 @@ class EnumMeta(ShapeCastable, py_enum.EnumMeta):
         if hasattr(cls, "_amaranth_shape_"):
             # Shape was provided explicitly; return it.
             return cls._amaranth_shape_
-        elif cls.__members__:
-            # Shape was not provided explicitly, but enumeration has members; treat it
-            # the same way `Shape.cast` treats standard library enumerations, so that
-            # `amaranth.lib.enum.Enum` can be a drop-in replacement for `enum.Enum`.
-            return Shape._cast_plain_enum(cls)
         else:
-            # Shape was not provided explicitly, and enumeration has no members.
-            # This is a base or mixin class that cannot be instantiated directly.
-            raise TypeError("Enumeration '{}.{}' does not have a defined shape"
-                            .format(cls.__module__, cls.__qualname__))
+            # Shape was not provided explicitly; treat it the same way `Shape.cast` treats
+            # standard library enumerations, so that `amaranth.lib.enum.Enum` can be a drop-in
+            # replacement for `enum.Enum`.
+            return Shape._cast_plain_enum(cls)
 
     def __call__(cls, value):
         # :class:`py_enum.Enum` uses ``__call__()`` for type casting: ``E(x)`` returns


Test patch:
diff --git a/tests/test_lib_enum.py b/tests/test_lib_enum.py
index 2bf2d19..9d5a27e 100644
--- a/tests/test_lib_enum.py
+++ b/tests/test_lib_enum.py
@@ -1,3 +1,5 @@
+import enum as py_enum
+
 from amaranth import *
 from amaranth.lib.enum import Enum
 
@@ -21,9 +23,10 @@ class EnumTestCase(FHDLTestCase):
     def test_shape_no_members(self):
         class EnumA(Enum):
             pass
-        with self.assertRaisesRegex(TypeError,
-                r"^Enumeration '.+?\.EnumA' does not have a defined shape$"):
-            Shape.cast(EnumA)
+        class PyEnumA(py_enum.Enum):
+            pass
+        self.assertEqual(Shape.cast(EnumA), unsigned(0))
+        self.assertEqual(Shape.cast(PyEnumA), unsigned(0))
 
     def test_shape_explicit(self):
         class EnumA(Enum, shape=signed(2)):