Opened 5 years ago
Closed 5 years ago
#9231 closed bug (fixed)
7.8 regression in Read instance of Data.Fixed.Pico
Reported by: | leonbaum2 | Owned by: | |
---|---|---|---|
Priority: | highest | Milestone: | 7.8.3 |
Component: | libraries/base | Version: | 7.8.2 |
Keywords: | regression | Cc: | hvr, ekmett, thoughtpolice, igloo, ashley@… |
Operating System: | Unknown/Multiple | Architecture: | Unknown/Multiple |
Type of failure: | None/Unknown | Test Case: | |
Blocked By: | Blocking: | ||
Related Tickets: | #9240 #7483 | Differential Rev(s): | |
Wiki Page: |
Description
If I run the following with ghc-7.8.2, the runtime system eats up all of my memory and doesn't produce a result:
read "1" :: Data.Fixed.Pico
Change History (12)
comment:1 Changed 5 years ago by
comment:2 Changed 5 years ago by
Cc: | thoughtpolice added |
---|---|
Keywords: | regression added |
Milestone: | → 7.8.3 |
Priority: | normal → highest |
comment:3 Changed 5 years ago by
I can't think of any cleaner fix for this other than radically rewriting Data.Fixed
or extracting the 10-base exponent via
e = ceiling (logBase 10 (fromInteger r) :: Double)
and having the code read like
convertFixed :: forall a . HasResolution a => Lexeme -> ReadPrec (Fixed a) convertFixed (Number n) | Just (i, f) <- numberToFixed e n = return (fromInteger i + (fromInteger f / fromInteger r)) where r = resolution (undefined :: Fixed a) e = ceiling (logBase 10 (fromInteger r) :: Double) convertFixed _ = pfail
(Note that numberToFixed
also expects an exponent rather than a power-of-10)
comment:4 Changed 5 years ago by
The really weird thing though is why the test-case at libraries/base/tests/readFixed001.hs actually works as expected....
comment:5 Changed 5 years ago by
Cc: | igloo ashley@… added |
---|
convertFixed
was last modified (or written) by Ian Lynagh. Much of the rest of Data.Fixed
is by Ashley Yakeley.
I'm copying both, not in a blaming way, but because they would both be well positioned to offer help on fixing.
But others, please go ahead and suggest fixes.
Simon
comment:6 Changed 5 years ago by
I've just submitted https://phabricator.haskell.org/D25 for review to get the ball rolling
comment:8 Changed 5 years ago by
Status: | new → merge |
---|
comment:10 Changed 5 years ago by
Replying to Ashley Yakeley:
Just to clarify, r is not necessarily a power of 10.
Now that you say it, I had the impression an implicit power-of-10 assumption was baked into Data.Fixed
, for instance consider the following weird behaviour (can be observed in GHC 7.6 as well):
Prelude> import Data.Fixed Prelude Data.Fixed> data B7 Prelude Data.Fixed> instance HasResolution B7 where resolution _ = 128 Prelude Data.Fixed> 1.070 :: Fixed B7 1.062 Prelude Data.Fixed> 1.062 :: Fixed B7 1.054 Prelude Data.Fixed> 1.054 :: Fixed B7 1.046 Prelude Data.Fixed> 1.046 :: Fixed B7 1.039
comment:12 Changed 5 years ago by
Related Tickets: | → #9240 #7483 |
---|---|
Resolution: | → fixed |
Status: | merge → closed |
The fix for the regression has been merged to ghc-7.8 via [99462b6877308003442942cbddb3296f29cfb9a8/base] and [4254e158e05ac86b922dea6ba3c3c330732d991b/base]. The remaining "read . show == id" has been moved into a ticket of its own (#9240)
The parser code seems totally wrong, e.g. consider
And note that
resolution
doesn't return an exponent but rather a power of 10.