Repository: amaranth-lang/amaranth
Pull request #1116 (2024-02-13 04:32:50)

hdl._ast: fix `shift_right` and `as_signed` edge cases.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1116?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`0056e98`)](https://app.codecov.io/gh/amaranth-lang/amaranth/commit/0056e982c5a85b26935b8c750cc00cf4b1e66ebc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) 85.60% compared to head [(`c440bb1`)](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1116?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) 85.57%.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1116      +/-   ##
==========================================
- Coverage   85.60%   85.57%   -0.03%     
==========================================
  Files          42       42              
  Lines        8336     8340       +4     
  Branches     1967     1969       +2     
==========================================
+ Hits         7136     7137       +1     
- Misses       1004     1005       +1     
- Partials      196      198       +2     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1116?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang).


Patch:
diff --git a/amaranth/hdl/_ast.py b/amaranth/hdl/_ast.py
index 6355654..f8860e8 100644
--- a/amaranth/hdl/_ast.py
+++ b/amaranth/hdl/_ast.py
@@ -483,7 +483,14 @@ class Value(metaclass=ABCMeta):
         Returns
         -------
         :class:`Value`, :pc:`signed(len(self))`, :ref:`assignable <lang-assignable>`
+
+        Raises
+        ------
+        ValueError
+            If :pc:`len(self) == 0`.
         """
+        if len(self) == 0:
+            raise ValueError("Cannot create a 0-width signed value")
         return Operator("s", [self])
 
     def __bool__(self):
@@ -900,9 +907,9 @@ class Value(metaclass=ABCMeta):
 
         Returns
         -------
-        :class:`Value`, :pc:`unsigned(len(self) + amount)`
+        :class:`Value`, :pc:`unsigned(max(len(self) + amount, 0))`
             If :pc:`self` is unsigned.
-        :class:`Value`, :pc:`signed(len(self) + amount)`
+        :class:`Value`, :pc:`signed(max(len(self) + amount, 1))`
             If :pc:`self` is signed.
         """
         if not isinstance(amount, int):
@@ -974,6 +981,8 @@ class Value(metaclass=ABCMeta):
         if amount < 0:
             return self.shift_left(-amount)
         if self.shape().signed:
+            if amount >= len(self):
+                amount = len(self) - 1
             return self[amount:].as_signed()
         else:
             return self[amount:] # unsigned


Test patch:
diff --git a/tests/test_hdl_ast.py b/tests/test_hdl_ast.py
index 83bbb77..bcb64f2 100644
--- a/tests/test_hdl_ast.py
+++ b/tests/test_hdl_ast.py
@@ -343,7 +343,7 @@ class ValueTestCase(FHDLTestCase):
         self.assertRepr(Const(256, signed(9)).shift_left(-5),
                         "(s (slice (const 9'sd-256) 5:9))")
         self.assertRepr(Const(256, signed(9)).shift_left(-15),
-                        "(s (slice (const 9'sd-256) 9:9))")
+                        "(s (slice (const 9'sd-256) 8:9))")
 
     def test_shift_left_wrong(self):
         with self.assertRaisesRegex(TypeError,
@@ -367,12 +367,20 @@ class ValueTestCase(FHDLTestCase):
                         "(slice (const 9'd256) 1:9)")
         self.assertRepr(Const(256, unsigned(9)).shift_right(5),
                         "(slice (const 9'd256) 5:9)")
+        self.assertRepr(Const(256, unsigned(9)).shift_right(15),
+                        "(slice (const 9'd256) 9:9)")
         self.assertRepr(Const(256, signed(9)).shift_right(1),
                         "(s (slice (const 9'sd-256) 1:9))")
         self.assertRepr(Const(256, signed(9)).shift_right(5),
                         "(s (slice (const 9'sd-256) 5:9))")
+        self.assertRepr(Const(256, signed(9)).shift_right(7),
+                        "(s (slice (const 9'sd-256) 7:9))")
+        self.assertRepr(Const(256, signed(9)).shift_right(8),
+                        "(s (slice (const 9'sd-256) 8:9))")
+        self.assertRepr(Const(256, signed(9)).shift_right(9),
+                        "(s (slice (const 9'sd-256) 8:9))")
         self.assertRepr(Const(256, signed(9)).shift_right(15),
-                        "(s (slice (const 9'sd-256) 9:9))")
+                        "(s (slice (const 9'sd-256) 8:9))")
 
     def test_shift_right_wrong(self):
         with self.assertRaisesRegex(TypeError,
@@ -516,6 +524,11 @@ class OperatorTestCase(FHDLTestCase):
         self.assertEqual(repr(v), "(s (const 4'd1))")
         self.assertEqual(v.shape(), signed(4))
 
+    def test_as_signed_wrong(self):
+        with self.assertRaisesRegex(ValueError,
+                r"^Cannot create a 0-width signed value$"):
+            Const(0, 0).as_signed()
+
     def test_pos(self):
         self.assertRepr(+Const(10), "(const 4'd10)")