|
| 1 | +- Start Date: 2024-02-05 |
| 2 | +- RFC PR: [amaranth-lang/rfcs#46](https://github.com/amaranth-lang/rfcs/pull/46) |
| 3 | +- Amaranth Issue: [amaranth-lang/amaranth#1081](https://github.com/amaranth-lang/amaranth/issues/1081) |
| 4 | + |
| 5 | +# Change `Shape.cast(range(1))` to `unsigned(0)` |
| 6 | + |
| 7 | +## Summary |
| 8 | +[summary]: #summary |
| 9 | + |
| 10 | +Change `Shape.cast(range(1))` to return `unsigned(0)` instead of `unsigned(1)`. |
| 11 | + |
| 12 | +## Motivation |
| 13 | +[motivation]: #motivation |
| 14 | + |
| 15 | +Currently, `Shape.cast(range(1))` returns `unsigned(1)`. This is inconsistent with the expectation that casting `range` to a shape will return the minimal shape capable of representing all elements of that range, which is clearly `unsigned(0)`. |
| 16 | + |
| 17 | +This behavior is an accidental result of using `bits_for` internally to determine required width for both endpoints, which returns `1` for an input of `0`. |
| 18 | + |
| 19 | +The behavior introduces edge cases in unexpected places. For example, one may expect that `Memory(depth=depth).read_port().addr.width == ceil_log2(depth)`. This is currently false for `depth` of 1 for no particularly good reason. |
| 20 | + |
| 21 | +## Guide-level explanation |
| 22 | +[guide-level-explanation]: #guide-level-explanation |
| 23 | + |
| 24 | +`Shape.cast(range(1))` is changed to return `unsigned(0)`. The same applies to any other range whose only element is `0`, like `Shape.cast(range(0, 2, 2))`. |
| 25 | + |
| 26 | +Arguably, this change requires a negative amount of exlanation, since it removes an edge case and brings the behavior into alignment with the language reference. |
| 27 | + |
| 28 | +## Reference-level explanation |
| 29 | +[reference-level-explanation]: #reference-level-explanation |
| 30 | + |
| 31 | +See above. |
| 32 | + |
| 33 | +## Drawbacks |
| 34 | +[drawbacks]: #drawbacks |
| 35 | + |
| 36 | +This is a minor backwards compatibility hazard. |
| 37 | + |
| 38 | +## Rationale and alternatives |
| 39 | +[rationale-and-alternatives]: #rationale-and-alternatives |
| 40 | + |
| 41 | +The change itself is simple enough that it cannot really be done any other way. |
| 42 | + |
| 43 | +It would be possible to introduce a compatibility warning to the 0.4 branch. However, a `range()` signal having a shape that's slightly too large is unlikely to cause problems in the first place, so the warning would cause a mass of false positives without a nice way to turn it off. |
| 44 | + |
| 45 | +## Prior art |
| 46 | +[prior-art]: #prior-art |
| 47 | + |
| 48 | +None. |
| 49 | + |
| 50 | +## Unresolved questions |
| 51 | +[unresolved-questions]: #unresolved-questions |
| 52 | + |
| 53 | +None. |
| 54 | + |
| 55 | +## Future possibilities |
| 56 | +[future-possibilities]: #future-possibilities |
| 57 | + |
| 58 | +The `bits_for` function, which led to this issue in the first place, could be deprecated and removed from public interface to avoid introducing similar problems to external code. Otherwise, it should at the very least be documented and loudly call out this special case. |
0 commit comments