Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider removing Num a => Num (SBV a) instance #706

Closed
LeventErkok opened this issue Jun 8, 2024 · 0 comments
Closed

Consider removing Num a => Num (SBV a) instance #706

LeventErkok opened this issue Jun 8, 2024 · 0 comments
Assignees

Comments

@LeventErkok
Copy link
Owner

As exemplified in #698 and #598, the (Old a, Num a, SymVal a) => Num (SBV a) instance creates lots of type-checking programs that actually do not work in practice.

The instance itself, however, is useful. It takes care of lifting of arithmetic operations over all basic types (Integers, signed/unsigned words, floats etc.); and it works like a charm. But when someone defines their own instance of Num for a custom type T, then they automatically get an Num (SBV T) for that type as well, which doesn't do what they'd have expected it to do. (It errors out in general; but there might be situations it silently does the "wrong" unexpected thing as well.)

We should consider removing the instance:

sbv/Data/SBV/Core/Model.hs

Lines 1322 to 1329 in 720255d

-- Num instance for symbolic words.
instance (Ord a, Num a, SymVal a) => Num (SBV a) where
fromInteger = literal . fromIntegral
SBV x + SBV y = SBV (svPlus x y)
SBV x * SBV y = SBV (svTimes x y)
SBV x - SBV y = SBV (svMinus x y)
-- Abs is problematic for floating point, due to -0; case, so we carefully shuttle it down
-- to the solver to avoid the can of worms. (Alternative would be to do an if-then-else here.)

This might lead to more boiler-plate code in the SBV code base itself, but it might things better in the long run. In particular, as Andreas pointed out, we can have more of "if it type checks it works" behavior that Haskell programmers expect, which is currently not the case for this instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant