potential bug with abs

[symbiont-alexey-babkin]

Hi, I'm trying to create a custom bounded Decimal with Precision of 38 and Scale of 10

this is how I define my newtype for the bounded Integer that underlies that Decimal

type Scale = 10

type DefaultRounding = D.RoundHalfFromZero

newtype IntegerPrec38 = IntegerPrec38 {unIntegerPrec38 :: Integer}
  deriving (Eq, Show, Ord)
  deriving newtype (Num, Integral, Real, Enum, NFData, D.Round DefaultRounding, D.Round D.RoundHalfToZero, D.Round D.RoundHalfEven)
  deriving newtype Seri.Flat
  deriving stock Data

instance Bounded IntegerPrec38 where
  minBound = -10 ^ (38 :: Int)
  maxBound = 10 ^ (38 :: Int) - 1
  
type UnderlyingIntegral = IntegerPrec38

type D = D.Decimal DefaultRounding Scale UnderlyingIntegral

I have the subtract 1 like this from maxBound to mimic the hard bounds of Int64 for example, its minBound, negated is always larger by 1 than maxBound (due to 2s compliment encoding of signed numbers)

The bounding seems to mostly work with the bounded operations like addition, multiplication ...

I however had this failed test:

      it "overflow due to having more negative values than positive" $ do
       first displayException (abs minBound) `shouldBe` Left "arithmetic overflow"

which should pass given the bounds I specified above.

I'm catching this corner case in my user code, but I believe this should be caught at the library level and hence I think it's a bug.

Please let me know if I make any erroneous assumptions on my part or if you have a suggestion how to better use the lib for my case.

Thank you

[lehins]

My bad I read your ticket wrong, I'll try to investigate it a bit better tomorrow

[symbiont-alexey-babkin]

Thanks Alexey. Please let me know if I can fill in any other details. I'll do my part of the legwork.
To be clear, D.Decimal and abs are coming from safe-decimal (sorry I was not explicit with the imports)

[lehins]

@symbiont-alexey-babkin you usage is totally on point, I initially completely misunderstood it.

I think you are right in that there is a problem. Quick look makes me believe that the current abs implementation relies on the overflow of underlying integral type used for Decimal, which is not the case for IntegerPrec38.

In other words this should be smarter:

safe-decimal/src/Numeric/Decimal/BoundedArithmetic.hs

Lines 156 to 159 in 50c037b

	| absd < 0 = throwM Overflow
	| otherwise = pure absd
	where
	absd = abs d

Maybe something like:

absBounded :: (MonadThrow m, Num p, Ord p, Bounded p) => p -> m p
absBounded d
  | absd < 0 || absd > maxBound = throwM Overflow
  | otherwise = pure absd
  where
    absd = abs d

If you'd like giving it a try and submitting a PR that would be awesome, otherwise I'll try to figure it when I get a chance

[symbiont-alexey-babkin]

sure, i'll submit a PR, thanks