Fix BitVector shifts

[kleinreact]

Fixes #2730 such that (+>>.) and (.<<+) are compliant with (+>>) and (<<+) for bit vectors of zero length, respectively.

Still TODO:

• [x] Write a changelog entry (see changelog/README.md)
• [x] Check copyright notices are up to date in edited files

[DigitalBrains1]

Thanks!

We usually use Clash.Promoted.Nat.compareSNat for this. I don't know if there are any downsides to picking GHC.TypeNats.cmpNat, though. Both apparently provide proof for the comparison result, but the proof wasn't needed here in the first place since the old code type-checked.

That raises a separate point: perhaps we need to look at Bits (BitVector n) or Enum (BitVector n) if apparently one of those has some partial functions? We might need a 1 <= n on one of those. So it won't type-check anymore.

And since this is a fix that pertains to a released version of Clash, it should have a changelog entry.

[edit] If you think: why is this guy going on about Enum (BitVector n)? Well it's because I wasn't paying attention and there is an Enum constraint on replaceBit. That constraint just didn't actually refer to the BitVector argument. I checked Enum for sanity as well; it is fine. It also wasn't germane to the issue at hand. [/edit]

[DigitalBrains1]

Ah no, wait, it was in the bit shifters themselves. They call

>>> replaceBit# (0 :: BitVector 0) (-1) 1
*** Exception: replaceBit: -1 is out of range [-1..0]
CallStack (from HasCallStack):
  error, called at src/Clash/Sized/Internal/BitVector.hs:1143:12 in clash-prelude-1.8.1-72gLWAPSXgPDozrrxr9IhV:Clash.Sized.Internal.BitVector

for n = 0. It's a pity that that error message is a bit garbled. We should fix that as well, beause it's just not very nice.

[edit] Some Bits (BitVector 0) functions raise exceptions: bit, setBit, clearBit, complementBit, testBit (all these take bit indices) and rotateL, rotateR. Those latter two give division by zero.

I suppose the ones that take bit indices are fair that they raise an exception. If you passed them a valid bit index, they'd surely work right. It's just nobody found it yet.

Those rotates I'd like to improve. [/edit]

[DigitalBrains1]

Ah ideally it'd have FIXED: as the first word of the changelog; this helps the person writing the actual changelog for a release in quickly putting it under the right heading. And we'd like to automate a part of this one day; now it's still all manual labor.

Sorry for initially missing this. I'm rather tired, perhaps I should have just let it lie until tomorrow, in retrospect.

I don't think it's that bad if it is missing from this entry. This'll be part of 1.8.2, which will have a reasonably short changelog, I expect. It would not have been nice for 1.8.0, since that introduced a lot of changes, so that changelog was quite some work to write.