haskell
Flag to introduce pedantic invariant checks?
I was speaking to a client with a large codebase which is somehow breaking the internal invariants of a Map, and wondering how best to track down the offending code. My first guess is that a function with a precondition such as fromDistinctAscList is being misused, or perhaps the internals API is used incorrectly, but it's difficult to figure out where when there are many transitive dependencies in a large codebase.
Would it make sense to have a manual Cabal flag for containers that turns on a "pedantic" mode via CPP, wherein functions like fromDistinctAscList perform a runtime check of their precondition and error out with a callstack if it is not satisfied? Even the internal API could potentially perform invariant checks, to help with situations like #913 where the user has shot themselves in the foot and needs to figure out where they fired from.
Obviously enabling this flag would hurt performance, but it could be very helpful for diagnosis. Potentially it could guard new HasCallStack constraints as well (cf. #489), to avoid any performance worries introducing them by default.
one wrinkle I found is the Data.Map.Internal.Debug.ordered requires the Ord constraint added to functions which don't have them, and this also breaks e.g. binary https://github.com/kolmodin/binary/blob/master/src/Data/Binary/Class.hs#L658-L660
@jberryman good point. For the public API alone, I suspect we could get away with introducing a wrapper datatype that captures the Ord dictionary in the data constructor. But it wouldn't work for the internal API, where you can't get away from the fact that the constructors of Map are exposed and don't have Ord dictionaries available. Maybe with some pattern synonyms one could get away with changing the internal API just a little, but it does seem tricky.
It would be quite unusual to add such a flag to containers.
Doesn't it make more sense to add the flag in the client code? After all, that's where the potential misuse is.
Well, if one has a large application with many transitive dependencies that themselves depend on containers, the problem is in identifying which one violates the invariant. One could introduce a wrapper module that added runtime checks, but then it's nontrivial to get all the other packages in the dependency stack to use it instead of containers.
Thinking more about the problem @jberryman raised, it does seem tricky, because e.g. the Ord constraint will need to bubble up from calls to fromDistinctAscList, which may well require code changes in third-party dependencies (e.g. the Binary instance for Map). So perhaps this issue is a non-starter.
The basic difficulty is that the call-stack based error reporting mechanisms tell you where the error was detected, not where it was introduced. One could imagine potentially invalid values storing the call-stack to their allocation site, so that if an error was detected, the allocation site was known. But figuring out how to propagate this information through transformations of the values seems nontrivial, and is perhaps better handled with compiler-level debug information.
In short, feel free to just close this issue.
The problem seems like it might reasonably arise, though I'm not sure of the best way to solve it. It certainly doesn't sound appealing to change containers to effectively offer two different APIs toggled by a flag. This doesn't scale either, many libraries (including base) trust the programmer to satisfy preconditions when using certain functions.
So yes, I don't know what we can do here. Please reopen if there is more to be discussed.