Opened 17 months ago

Last modified 9 months ago

#15100 new bug

`ApplicativeDo` needlessly uses `join` too much

Reported by: ekmett Owned by:
Priority: normal Milestone: 8.10.1
Component: Compiler (CodeGen) Version: 8.4.2
Keywords: ApplicativeDo Cc: dfeuer, simonmar
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

foo = do
  A 1
  B 2
  C 3
  return () 

produces very different code than

foo = do
  A 1
  B 2
  C 3

for no good reason. Currently we check to see if the last clause is a return or something else, and then bolt a join in if it isn't return. But this check is too conservative. If it isn't a return but doesn't reference any of the variables above, you can still desugar it with (<*>).

We found this during a twitch livestream in front of an audience of 127 people. =)

Change History (6)

comment:1 Changed 17 months ago by adamse

Keywords: ApplicativeDo added

comment:2 Changed 17 months ago by dfeuer

This is all around stmtTreeToStmts in compiler/rename/RnExpr, and needJoin seems rather relevant.

comment:3 Changed 17 months ago by dfeuer

Cc: dfeuer added

Another thing that should work doesn't quite:

foo a b c = do
  a
  b
  res <- c
  return res

This is even more surprising, because it desugars without join (and without >>=), but with >>!

comment:4 Changed 17 months ago by simonpj

Cc: simonmar added

cc'ing Simon Marlow, the king of ApplicativeDo.

Whatever emerges, I'd love to see it expressed in the language of the paper. I found the division in the paper (into a dependency analysis followed by desugaring) to be very helpful. If we could have a typeset appendix that gives the actual rules followed by GHC (including improvements described above) that would be incredibly helpful. Both for understanding the code, and for helping others who implement ApplicativeDo in their compilers.

We can easily make the paper source code available.

comment:5 Changed 15 months ago by bgamari

Milestone: 8.6.18.8.1

These won't be addressed for GHC 8.6.

comment:6 Changed 9 months ago by osa1

Milestone: 8.8.18.10.1

Bumping milestones of low-priority tickets.

Note: See TracTickets for help on using tickets.