Change type signatures of `omapM_`? #28

Closed
snoyberg opened this Issue May 4, 2014 · 7 comments

Projects

None yet

3 participants

@snoyberg
Owner
snoyberg commented May 4, 2014

I was working on an article about the problems of using mapM_ with Maybe:

https://www.fpcomplete.com/tutorial-preview/3825/PmOliilpIA

I thought I'd fixed the issue with mono-traversable, but apparently I was wrong: omapM_ is still not tail recursive. Looking at the implementation:

    omapM_ _ Nothing = return ()
    omapM_ f (Just x) = f x >> return ()

The issue is fx >> return (); having to explicitly drop the return value from f breaks tail recursion. I'd like to propose changing the type signature of omapM_ from:

omapM_ :: (MonoFoldable mono, Monad m) => (Element mono -> m b) -> mono -> m ()

to:

omapM_ :: (MonoFoldable mono, Monad m) => (Element mono -> m ()) -> mono -> m ()

I actually prefer this type signature overall, but it does break compatibility with Data.Foldable.

Pinging @gregwebs.

@gregwebs
Collaborator
gregwebs commented May 4, 2014

That seems fine to me. It might be good to put this change in your article and see if anyone has feedback before putting the change up on hackage.

@snoyberg
Owner
snoyberg commented May 5, 2014

OK, I've published the article on the Yesod blog. Le's figure that if we don't hear any complaints on this in the next week, we go ahead with this change.

Side note: we should change the definition of omapM_ for Either to be tail-recursive as well.

@Davorak
Davorak commented May 6, 2014

For those of you who are following alone with the published article forM_Maybe:

forM_Maybe :: Monad m => Maybe a -> (a -> m ()) -> m ()
forM_Maybe Nothing _ = return ()
forM_Maybe (Just x) f = f x

Causes a stack overflow me on ghc 7.6.3 but not ghc 7.8.2.

@snoyberg
Owner
snoyberg commented May 6, 2014

@Davorak That version of the function never gave me a stack overflow, whether on GHC 7.6 or 7.8. Can you provide a full program and the compilation options you used to trigger the stack overflow?

@Davorak
Davorak commented May 6, 2014

@snoyberg - Yes here is a gist:
https://gist.github.com/Davorak/9e5def76c17c20d24bcc

I was just manually switching out the different versions of printChars and running:

ghc -O2 Main.hs --fforce-recomp -rtsopts
./Main +RTS -K8m

for ghc 7.8.2.

After changing the stack size for ghc 7.8.2 to match ghc 7.6.3, given by roconnor, I am getting all versions to stackoverflow. If something is different in my development environment that is causing this disparity I want to find out what it is.

I am seeing the same behavior on my linux and my mac with ghc 7.6.3, where all versions cause an overflow.

@Davorak
Davorak commented May 6, 2014

@snoyberg - I spotted my mistake, sorry for wasting your time. I was not correctly recursing in my alternative functions. I am able to reproduce the same results.

@snoyberg
Owner

It doesn't sound like there's any objection to this change, so I'm going to move ahead with it.

@snoyberg snoyberg closed this in 896cacf May 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment