Skip to content

Commit d6c50a2

Browse files
committed
Add RFC for Shape.cast(range(1)) change.
1 parent e10875b commit d6c50a2

File tree

1 file changed

+58
-0
lines changed

1 file changed

+58
-0
lines changed

text/0046-shape-range-1.md

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
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

Comments
 (0)