Skip to content
New issue

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

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

Already on GitHub? # to your account

should fromAscList check its precondition? #633

Open
jwaldmann opened this issue May 9, 2019 · 3 comments
Open

should fromAscList check its precondition? #633

jwaldmann opened this issue May 9, 2019 · 3 comments

Comments

@jwaldmann
Copy link
Contributor

Currently we have

import Data.Set
member 0 $ fromAscList [2,0]    -- value is False

since https://hackage.haskell.org/package/containers-0.6.0.1/docs/Data-Set.html#v:fromAscList does not check monotonicity. Should it? Could it? (I mean, how much extra time would this need?) Was this ever discussed?

(similar question: twanvl/multiset#9 (comment) )

@3noch
Copy link

3noch commented May 9, 2019

The docs clearly say The precondition (input list is ascending) is not checked. so I think it's safe to say it was discussed. Would it be worth adding fromAscListChecked? Maybe. The point of this function is largely performance. If you don't know for certain that your input is ascending, you should use fromList.

@treeowl
Copy link
Contributor

treeowl commented May 9, 2019

It should be a matter of a few minutes to strengthen the constraint from Eq to Ord (a potentially breaking change; others' code may have to be modified to adjust, but likely not a lot of code will break, and the fix is always easy) and use compare instead of Eq. Then benchmark fromListWith with several element types and list lengths and see if we can get away with it performance-wise.

@adamgundry
Copy link
Member

See also #981. I think it would make sense to have a Cabal flag that turns on precondition checks for debugging purposes, without needing to modify the calling code (in case it is deep inside some dependency).

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

No branches or pull requests

5 participants