nikita-volkov / stm-containers

Containers for STM
http://hackage.haskell.org/package/stm-containers
MIT License
67 stars 13 forks source link

Cannot compose with Focus.insert? #25

Closed michalrus closed 5 years ago

michalrus commented 5 years ago

Hi,

I was bumping dependency versions and noticed this:

    , testCase "StmContainers behave as expected" $ do
        xs <- TMap.newIO @Int @String
        wasSet <- atomically $ TMap.focus (do prev <- Focus.lookup
                                              Focus.insert "one"
                                              pure (isJust prev)) 1 xs
        wasSet @?= False
        xs' <- atomically . ListT.toList . TMap.listT $ xs
        [(1,"one")] @=? xs'

resulting in:

    StmContainers behave as expected: FAIL
      test/Backend/Common/Test.hs:152:
      expected: [(1,"one")]
       but got: []

That’s kind of unexpected, given that the code compiles, and the Focus docs say:

Abstraction over the modification of an element of a datastructure. It is composable using the standard typeclasses, e.g.:

Is this really an expected behavior with this composition? If so, why is it useful?

michalrus commented 5 years ago

I guess so, given this instance: https://github.com/nikita-volkov/focus/blob/409950dfefaa070b1d77b2a72c01fcb4d2a310e7/library/Focus.hs#L22-L39 … but it’s so misleading…

I much preferred the previous design, which was explicit in not eating my inserts.

I would prefer the current one, if such silent bugs were impossible…

Halp.

michalrus commented 5 years ago

E.g. now, how do I implement a replace to set a new v for a given k, but return the previous Maybe v?

Any immediate way that comes to mind will have this bug, and worst of all, it will compile…

nikita-volkov commented 5 years ago

This is a bug in implementation of "focus". Thanks for reporting. It might take me a few days to fix.

michalrus commented 5 years ago

Oh, I thought that was on purpose. Can’t wait, then, to see the new implementation. =) Great, thanks!

nikita-volkov commented 5 years ago

The issue is fixed in "focus-1.0.1.3". However I can't release yet, because Hackage is down today.

michalrus commented 5 years ago

Thank you very much!

nikita-volkov commented 5 years ago

Published.