Repository: amaranth-lang/amaranth
Pull request #1025 (2024-01-10 04:18:40)

Implement RFC 17: Remove `log2_int`.
Reexports of `amaranth.utils` functions are removed from `amaranth._utils` to avoid a circular import issue (for `deprecated`). Since this is a private module, this should not be a problem.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1025?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) Report
Attention: `1 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`d851580`)](https://app.codecov.io/gh/amaranth-lang/amaranth/commit/d8515807c28a638d359798b56b9a9eaea928857f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) 86.54% compared to head [(`c44cc46`)](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1025?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) 86.44%.

| [Files](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1025?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) | Patch % | Lines |
|---|---|---|
| [amaranth/utils.py](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1025?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang#diff-YW1hcmFudGgvdXRpbHMucHk=) | 95.00% | [1 Missing :warning: ](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1025?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) |

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


```diff
@@            Coverage Diff             @@
##             main    #1025      +/-   ##
==========================================
- Coverage   86.54%   86.44%   -0.10%     
==========================================
  Files          41       41              
  Lines        7053     7070      +17     
  Branches     1679     1685       +6     
==========================================
+ Hits         6104     6112       +8     
- Misses        788      797       +9     
  Partials      161      161              
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/1025?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/_utils.py b/amaranth/_utils.py
index 54499e2..7faf21e 100644
--- a/amaranth/_utils.py
+++ b/amaranth/_utils.py
@@ -6,11 +6,9 @@ import re
 from collections import OrderedDict
 from collections.abc import Iterable
 
-from .utils import *
 
-
-__all__ = ["flatten", "union" , "log2_int", "bits_for", "memoize", "final", "deprecated",
-           "get_linter_options", "get_linter_option"]
+__all__ = ["flatten", "union", "memoize", "final", "deprecated", "get_linter_options",
+           "get_linter_option"]
 
 
 def flatten(i):
diff --git a/amaranth/back/rtlil.py b/amaranth/back/rtlil.py
index 3af2f0a..1966407 100644
--- a/amaranth/back/rtlil.py
+++ b/amaranth/back/rtlil.py
@@ -4,7 +4,8 @@ from contextlib import contextmanager
 import warnings
 import re
 
-from .._utils import bits_for, flatten
+from .._utils import flatten
+from ..utils import bits_for
 from ..hdl import ast, ir, mem, xfrm, _repr
 from ..lib import wiring
 
diff --git a/amaranth/hdl/ast.py b/amaranth/hdl/ast.py
index 9bd3da3..957d71b 100644
--- a/amaranth/hdl/ast.py
+++ b/amaranth/hdl/ast.py
@@ -10,6 +10,7 @@ from itertools import chain
 
 from ._repr import *
 from .. import tracer
+from ..utils import *
 from .._utils import *
 from .._utils import _ignore_deprecated
 from .._unused import *
diff --git a/amaranth/hdl/dsl.py b/amaranth/hdl/dsl.py
index 6833c52..e23d161 100644
--- a/amaranth/hdl/dsl.py
+++ b/amaranth/hdl/dsl.py
@@ -5,7 +5,8 @@ from enum import Enum
 import warnings
 import sys
 
-from .._utils import flatten, bits_for
+from .._utils import flatten
+from ..utils import bits_for
 from .. import tracer
 from .ast import *
 from .ir import *
diff --git a/amaranth/lib/coding.py b/amaranth/lib/coding.py
index 8813f70..bacedbd 100644
--- a/amaranth/lib/coding.py
+++ b/amaranth/lib/coding.py
@@ -41,7 +41,7 @@ class Encoder(Elaboratable):
             for j in range(self.width):
                 with m.Case(1 << j):
                     m.d.comb += self.o.eq(j)
-            with m.Case():
+            with m.Default():
                 m.d.comb += self.n.eq(1)
         return m
 
diff --git a/amaranth/lib/fifo.py b/amaranth/lib/fifo.py
index a586866..b2b542f 100644
--- a/amaranth/lib/fifo.py
+++ b/amaranth/lib/fifo.py
@@ -4,7 +4,7 @@ import warnings
 
 from .. import *
 from ..asserts import *
-from .._utils import log2_int
+from ..utils import ceil_log2
 from .coding import GrayEncoder, GrayDecoder
 from .cdc import FFSynchronizer, AsyncFFSynchronizer
 
@@ -353,13 +353,12 @@ class AsyncFIFO(Elaboratable, FIFOInterface):
 
     def __init__(self, *, width, depth, r_domain="read", w_domain="write", exact_depth=False):
         if depth != 0:
-            try:
-                depth_bits = log2_int(depth, need_pow2=exact_depth)
-                depth = 1 << depth_bits
-            except ValueError:
+            depth_bits = ceil_log2(depth)
+            if exact_depth and depth != 1 << depth_bits:
                 raise ValueError("AsyncFIFO only supports depths that are powers of 2; requested "
                                  "exact depth {} is not"
                                  .format(depth)) from None
+            depth = 1 << depth_bits
         else:
             depth_bits = 0
         super().__init__(width=width, depth=depth)
@@ -530,13 +529,12 @@ class AsyncFIFOBuffered(Elaboratable, FIFOInterface):
 
     def __init__(self, *, width, depth, r_domain="read", w_domain="write", exact_depth=False):
         if depth != 0:
-            try:
-                depth_bits = log2_int(max(0, depth - 1), need_pow2=exact_depth)
-                depth = (1 << depth_bits) + 1
-            except ValueError:
+            depth_bits = ceil_log2(max(0, depth - 1))
+            if exact_depth and depth != (1 << depth_bits) + 1:
                 raise ValueError("AsyncFIFOBuffered only supports depths that are one higher "
                                  "than powers of 2; requested exact depth {} is not"
                                  .format(depth)) from None
+            depth = (1 << depth_bits) + 1
         super().__init__(width=width, depth=depth)
 
         self.r_rst = Signal()
diff --git a/amaranth/utils.py b/amaranth/utils.py
index cf98f3d..28d02fd 100644
--- a/amaranth/utils.py
+++ b/amaranth/utils.py
@@ -1,7 +1,35 @@
-__all__ = ["log2_int", "bits_for"]
+import operator
 
+from ._utils import deprecated
 
+__all__ = ["ceil_log2", "exact_log2", "log2_int", "bits_for"]
+
+
+def ceil_log2(n):
+    """Returns the integer log2 of the smallest power-of-2 greater than or equal to `n`.
+
+    Raises a `ValueError` for negative inputs."""
+    n = operator.index(n)
+    if n < 0:
+        raise ValueError("{n} is negative")
+    if n == 0:
+        return 0
+    return (n - 1).bit_length()
+
+
+def exact_log2(n):
+    """Returns the integer log2 of `n`, which must be an exact power of two.
+
+    Raises a `ValueError` if `n` is not a power of two."""
+    n = operator.index(n)
+    if n <= 0 or (n & (n - 1)):
+        raise ValueError("{n} is not a power of 2")
+    return (n - 1).bit_length()
+
+
+@deprecated("instead of `log2_int(n, True)`, use `exact_log2(n)`; instead of `log2_int(n, False)` use `ceil_log2(n)`")
 def log2_int(n, need_pow2=True):
+    n = operator.index(n)
     if n == 0:
         return 0
     r = (n - 1).bit_length()
@@ -11,11 +39,12 @@ def log2_int(n, need_pow2=True):
 
 
 def bits_for(n, require_sign_bit=False):
+    n = operator.index(n)
     if n > 0:
-        r = log2_int(n + 1, False)
+        r = ceil_log2(n + 1)
     else:
         require_sign_bit = True
-        r = log2_int(-n, False)
+        r = ceil_log2(-n)
     if require_sign_bit:
         r += 1
     return r
diff --git a/docs/changes.rst b/docs/changes.rst
index ce9f87b..dac65f2 100644
--- a/docs/changes.rst
+++ b/docs/changes.rst
@@ -10,13 +10,31 @@ Version 0.5 (unreleased)
 The Migen compatibility layer has been removed.
 
 
+Migrating from version 0.4
+--------------------------
+
+Apply the following changes to code written against Amaranth 0.4 to migrate it to version 0.5:
+
+* Update uses of :func:`amaranth.utils.log2_int(need_pow2=False)` to :func:`amaranth.utils.ceil_log2`
+* Update uses of :func:`amaranth.utils.log2_int(need_pow2=True)` to :func:`amaranth.utils.exact_log2`
+
+
+Implemented RFCs
+----------------
+
+.. _RFC 17: https://amaranth-lang.org/rfcs/0017-remove-log2-int.html
+
+* `RFC 17`_: Remove ``log2_int``
+
+
 Language changes
 ----------------
 
 .. currentmodule:: amaranth.hdl
 
-* Deprecated: argument `run_script=` in :meth:`BuildPlan.execute_local`
-* Added: `class:ast.Slice` objects have been made const-castable.
+* Added: :class:`ast.Slice` objects have been made const-castable.
+* Added: :func:`amaranth.utils.ceil_log2`, :func:`amaranth.utils.exact_log2`. (`RFC 17`_)
+* Deprecated: :func:`amaranth.utils.log2_int`. (`RFC 17`_)
 * Removed: (deprecated in 0.4) :meth:`Const.normalize`. (`RFC 5`_)
 * Removed: (deprecated in 0.4) :class:`ast.Sample`, :class:`ast.Past`, :class:`ast.Stable`, :class:`ast.Rose`, :class:`ast.Fell`.
 
@@ -39,6 +57,7 @@ Platform integration changes
 * Added: :meth:`BuildPlan.execute_local_docker`.
 * Added: :meth:`BuildPlan.extract`.
 * Added: ``build.sh``  begins with ``#!/bin/sh``.
+* Deprecated: argument `run_script=` in :meth:`BuildPlan.execute_local`
 * Removed: (deprecated in 0.4) :mod:`vendor.intel`, :mod:`vendor.lattice_ecp5`, :mod:`vendor.lattice_ice40`, :mod:`vendor.lattice_machxo2_3l`, :mod:`vendor.quicklogic`, :mod:`vendor.xilinx`. (`RFC 18`_)
 
 


Test patch:
diff --git a/tests/test_sim.py b/tests/test_sim.py
index 6f561a4..e80c2ef 100644
--- a/tests/test_sim.py
+++ b/tests/test_sim.py
@@ -509,7 +509,7 @@ class SimulatorIntegrationTestCase(FHDLTestCase):
                 self.m.d.sync += self.o.eq(self.a + self.b)
             with self.m.Case(1):
                 self.m.d.sync += self.o.eq(self.a - self.b)
-            with self.m.Case():
+            with self.m.Default():
                 self.m.d.sync += self.o.eq(0)
         self.m.domains += self.sync
 
diff --git a/tests/test_utils.py b/tests/test_utils.py
new file mode 100644
index 0000000..feb5147
--- /dev/null
+++ b/tests/test_utils.py
@@ -0,0 +1,75 @@
+import unittest
+
+from amaranth.utils import *
+from amaranth._utils import _ignore_deprecated
+
+
+class Log2TestCase(unittest.TestCase):
+    def test_ceil_log2(self):
+        self.assertEqual(ceil_log2(0), 0)
+        self.assertEqual(ceil_log2(1), 0)
+        self.assertEqual(ceil_log2(2), 1)
+        self.assertEqual(ceil_log2(3), 2)
+        self.assertEqual(ceil_log2(4), 2)
+        self.assertEqual(ceil_log2(5), 3)
+        self.assertEqual(ceil_log2(8), 3)
+        self.assertEqual(ceil_log2(9), 4)
+        with self.assertRaises(TypeError):
+            ceil_log2(1.5)
+        with self.assertRaises(ValueError):
+            ceil_log2(-1)
+
+    def test_exact_log2(self):
+        self.assertEqual(exact_log2(1), 0)
+        self.assertEqual(exact_log2(2), 1)
+        self.assertEqual(exact_log2(4), 2)
+        self.assertEqual(exact_log2(8), 3)
+        for val in [-1, 0, 3, 5, 6, 7, 9]:
+            with self.assertRaises(ValueError):
+                exact_log2(val)
+        with self.assertRaises(TypeError):
+            exact_log2(1.5)
+
+    @_ignore_deprecated
+    def test_log2_int(self):
+        self.assertEqual(log2_int(1), 0)
+        self.assertEqual(log2_int(2), 1)
+        self.assertEqual(log2_int(4), 2)
+        self.assertEqual(log2_int(8), 3)
+        for val in [-1, 3, 5, 6, 7, 9]:
+            with self.assertRaises(ValueError):
+                log2_int(val)
+        with self.assertRaises(TypeError):
+            log2_int(1.5)
+        self.assertEqual(log2_int(0, False), 0)
+        self.assertEqual(log2_int(1, False), 0)
+        self.assertEqual(log2_int(2, False), 1)
+        self.assertEqual(log2_int(3, False), 2)
+        self.assertEqual(log2_int(4, False), 2)
+        self.assertEqual(log2_int(5, False), 3)
+        self.assertEqual(log2_int(8, False), 3)
+        self.assertEqual(log2_int(9, False), 4)
+
+    def test_bits_for(self):
+        self.assertEqual(bits_for(-4), 3)
+        self.assertEqual(bits_for(-3), 3)
+        self.assertEqual(bits_for(-2), 2)
+        self.assertEqual(bits_for(-1), 1)
+        self.assertEqual(bits_for(0), 1)
+        self.assertEqual(bits_for(1), 1)
+        self.assertEqual(bits_for(2), 2)
+        self.assertEqual(bits_for(3), 2)
+        self.assertEqual(bits_for(4), 3)
+        self.assertEqual(bits_for(5), 3)
+        self.assertEqual(bits_for(-4, True), 3)
+        self.assertEqual(bits_for(-3, True), 3)
+        self.assertEqual(bits_for(-2, True), 2)
+        self.assertEqual(bits_for(-1, True), 1)
+        self.assertEqual(bits_for(0, True), 1)
+        self.assertEqual(bits_for(1, True), 2)
+        self.assertEqual(bits_for(2, True), 3)
+        self.assertEqual(bits_for(3, True), 3)
+        self.assertEqual(bits_for(4, True), 4)
+        self.assertEqual(bits_for(5, True), 4)
+        with self.assertRaises(TypeError):
+            bits_for(1.5)