Remove MultiOr's Functor instance #2134
Conversation
ttuegel
left a comment
ttuegel
left a comment
There was a problem hiding this comment.
Could you also remove MultiOr.extractPatterns and replace it with toList? It seems weird to have a function extractPatterns for MultiOr, when the thing inside the MultiOr might not be a pattern at all.
kore/src/Kore/Internal/MultiOr.hs
Outdated
| traverseLogic | ||
| :: Ord child2 | ||
| => TopBottom child2 | ||
| => Monad m | ||
| => (child1 -> m child2) | ||
| -> MultiOr child1 | ||
| -> m (MultiOr child2) | ||
| traverseLogic f multiOr = observeAllT $ Logic.scatter multiOr >>= lift . f | ||
| {-# INLINE traverseLogic #-} |
There was a problem hiding this comment.
Why do we need this? It seems exactly the same as traverse.
There was a problem hiding this comment.
I didn't end up using it, but left it in case anyone would want to run effects with LogicT instead of using the traverse instance on lists.
There was a problem hiding this comment.
But, there's no reason to do that. The only reason to use LogicT is if you want to run multiple traversals in sequence and build up the results lazily. There's no reason to use LogicT here because we immediately call observeAllT.
By the way, I just remembered: traverse should have a prominent warning that it should not be used with LogicT.
Co-authored-by: Thomas Tuegel <ttuegel@mailbox.org>
Co-authored-by: Thomas Tuegel <ttuegel@mailbox.org>
Co-authored-by: Thomas Tuegel <ttuegel@mailbox.org>
Co-authored-by: Thomas Tuegel <ttuegel@mailbox.org>
… into remove-multior-functor
2 participants
Part of #1037
Reviewer checklist
stack test --coveragestack haddock