Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#12490 closed bug (fixed)

With RebindableSyntax, ApplicativeDo should eliminate return/pure

Reported by: AaronFriel Owned by: simonmar
Priority: normal Milestone: 8.0.2
Component: Compiler Version: 8.0.1
Keywords: ApplicativeDo Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D2499
Wiki Page:

Description (last modified by AaronFriel)

In a module with -XApplicativeDo, -XRebindableSyntax, and local definitions for everything in the Functor-Applicative-Monad hierarchy, do-notation always desugars to "join (... (return ...))" (or /s/return/pure/). This forces the result to have at least the constraints of join, which in my case is "IxMonad m".

{-# LANGUAGE RebindableSyntax, ApplicativeDo #-}

module My where

-- straightforward definitions of fmap, pure, (<*>), join, return, (>>=), 
-- (>>) and fail in terms of IxFunctor, IxPointed, IxApplicative, IxMonad

fPure m = do
  a <- m
  b <- m
  pure (a, b)

fReturn m = do
  a <- m
  b <- m
  return (a, b)

According to -ddump-ds, these desugar to:

fPure :: IxMonad m => m k1 k1 a -> m k1 k1 (a, a)
fPure m = My.join ( My.(<*>) (My.fmap (\a b -> My.pure (a, b)) m) m )

fReturn :: IxMonad m => m k1 k1 a -> m k1 k1 (a, a)
fReturn m = My.join ( My.(<*>) (My.fmap (\a b -> My.return (a, b)) m) m )

But I would expect:

fPure m = My.(<*>) (My.fmap (\a b -> (a, b)) m) m

fReturn m = -- same

It appears that when "return" is not from base, ApplicativeDo only partially desugars to the specification, and the final "return" or "pure" remains in the output.

Change History (12)

comment:1 Changed 3 years ago by AaronFriel

Description: modified (diff)

comment:2 Changed 3 years ago by AaronFriel

Description: modified (diff)
Summary: ApplicativeDo and RebindableSyntax do not desugar as expectedWith RebindableSyntax, ApplicativeDo should eliminate return/pure

comment:3 Changed 3 years ago by simonpj

Owner: set to simonmar

comment:4 Changed 3 years ago by bgamari

For the record, there is actually a TODO in the code describing this exact shortcoming. See isReturnApp in RnExpr (https://ghc.haskell.org/trac/ghc/browser/ghc/compiler/rename/RnExpr.hs#L1774)

I believe that the problem is that the Applicative do desugaring happens in the renamer, where we are unable to lookup whether return should be the normal Control.Monad.return or some rebound alternative. It's not clear to me how to best fix this short of moving the Applicative do implementation to the typechecker, but I'll make sure Simon Marlow knows about this when we next meet.

comment:5 Changed 3 years ago by bgamari

My proposed solution here would be to lookup the Name of return in the environment and compare against it in isReturnApp.

comment:6 Changed 3 years ago by AaronFriel

Please permit me some additional questions, I would like to know more about the compiler internals.

I think I see why you might want to do this in the type checker, to make sure that return or pure has the right type before performing the transformation? If so, I don't know how important that is, but with RebindableSyntax and ApplicativeDo I would assume the user is saying, "I know what I'm doing and I want the transformation to be assumed to be lawful." And, as the ApplicativeDo desugaring will still contain join or return in many cases, incorrectly typed pure and return definitions ought to cause a compiler error sooner or later. Does that line up with your thoughts?

When you say "Name of return", do you mean in case return has been aliased by the user? Does Name then correspond to what I would call the fully qualified name, e.g.: Control.Monad.foo?

If I understand your solution correctly, will any top-level return or pure permit the desugaring I wrote? (Duplicated here.)

fPure m = do
  a <- m
  b <- m
  pure (a, b)

-- to:

fPure m = My.(<*>) (My.fmap (\a b -> (a, b)) m) m

comment:7 Changed 3 years ago by bgamari

Differential Rev(s): Phab:D2499
Milestone: 8.0.2
Status: newpatch

My original concern was that we wouldn't have a environment handy in the renamer to lookup which binding should be used for return. As it turns out this concern was ill-founded.

I have a fix in Phab:D2499. Happily I think it's even simple enough to include in 8.0.2.

comment:8 Changed 3 years ago by Ben Gamari <ben@…>

In 043604c/ghc:

RnExpr: Fix ApplicativeDo desugaring with RebindableSyntax

We need to compare against the local return and pure, not returnMName
and pureAName.

Fixes #12490.

Test Plan: Validate, add testcase

Reviewers: austin, simonmar

Reviewed By: simonmar

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D2499

GHC Trac Issues: #12490

comment:9 Changed 3 years ago by bgamari

Status: patchmerge

comment:10 Changed 3 years ago by bgamari

comment:11 Changed 3 years ago by bgamari

Resolution: fixed
Status: mergeclosed

comment:12 Changed 2 years ago by RyanGlScott

Keywords: ApplicativeDo added
Note: See TracTickets for help on using tickets.