Add maybeResize (and friends)
#2780
Conversation
jvnknvlgl
force-pushed
the
jvnknvlgl/clash-prelude/resize
branch
from
efb3cc6
to
dd3c71c
Compare
jvnknvlgl
force-pushed
the
jvnknvlgl/clash-prelude/resize
branch
from
dd3c71c
to
c5b9bef
Compare
| -- | Like 'resize', but returns 'Nothing' if /f a/ is out of bounds for /f b/. | ||
| -- Useful when you do not know /f a/ can be out of bounds, and would like to | ||
| -- have your assumptions checked. | ||
| maybeResize :: |
There was a problem hiding this comment.
This will break for anything larger than native word size (64-bit usually) in hardware. At least mention this limitation in the doc.
edit: I think it would be possible to rewrite this using only Ord, Bounded, Resize and BitPack. resize to Max (BitSize a) (BitSize b) and compare to maxBound/minBound of b. That won't have this limitation. It will also sidestep the notorious clash Integer warning.
There was a problem hiding this comment.
Oh in fact you can do a short circuit on the type level if BitSize a >= BitSize b which means this function will be free in that case in hardware.
There was a problem hiding this comment.
I agree this shouldn't go through dynamically sized constructs.
Something along the lines of:
case cmpNat @a @b Proxy Proxy of
LTI | v > resize (maxBound @(f b)) -> Nothing
EQI -> Just v -- optional, for performance reasons?
_ -> Just (resize v)
| -- | Like 'fromIntegral', but returns 'Nothing' if /a/ is out of bounds for /b/. | ||
| -- Useful when you do not know /a/ can be out of bounds, and would like to have | ||
| -- your assumptions checked. | ||
|
|
There was a problem hiding this comment.
The Haddock for maybeFromIntegral is not generated, and that's probably because of this blank line. This blank line means that what follows the blank line is a regular Haskell comment rather than a Haddock comment. I would have expected the Haddock to still be associated with the function, but apparently it's cut off and lost?
TL;DR: Remove the blank line :-). And always check the Haddock output :-).
To view the Haddock, you can click on the Details button next to the ci/gitlab/gitlab.com CI test result, and then click on haddock (in the column test), and then click the big Browse button in the rightmost column of that webpage.
Or just generate Haddock on your own system of course, before you run CI.
| -- Useful when you do not know /f a/ can be out of bounds, and would like to | ||
| -- have your assumptions checked. |
There was a problem hiding this comment.
I think all these maybe variants can be described much shorter:
Like
resize, but returnsNothingif f a is out of bounds for f b.
The rest seems unnecessary, and also it mentions assumptions, but we don't go in with assumptions. This function is total; the assumption with the checked variant was that it would fit, but we want to error when that assumption is wrong. This is different.
Also, Martijn and I discussed that we'd best rephrase all these functions a bit, but that can be done in a followup PR. Or I can tack a commit onto this PR...
Add
maybeFromIntegral,maybeResizeandmaybeTruncateB. Resolves #2779. No tests yet.Still TODO: