Ticket #7071 (closed bug: fixed)

Opened 11 months ago

Last modified 7 months ago

Refactoring arrows

Reported by: simonpj Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.4.2
Keywords: Cc: ross@…, dwc@…, jeltsch
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description (last modified by simonpj) (diff)

At the moment arrow commands re-use HsExpr for Proc expressions. But with Dan Winograd-Cort I concluded that the best thing by far would be to separate the data type of HsExpr from that of arrow commands. I think that would lead to a substantial tidy up.

There is also a nasty lurking bug in the type checking of commands; see line 290 of TcArrows. Here we call the unifier, but do not do anything with the coercion it returns. This is plainly wrong and will bite eventually. But I don't understand this code well enough to fix it.

In short, I am not confident of the arrows implementation at the moment.

Several tickets are blocked on this

Change History

  Changed 11 months ago by simonpj

  • description modified (diff)

  Changed 10 months ago by simonpj

  • description modified (diff)

in reply to: ↑ description   Changed 10 months ago by jeltsch

Replying to simonpj:

#4359 proposal for proc case and multi-branch if

There is now a separate feature request #7081 for arrow analogs of lambda case and multi-way if, since ticket #4359 (which actually was about the non-arrow variants) has been closed.

  Changed 10 months ago by simonpj

Daniel asks what kind of Stmts can occur in arrow commands. Here's a table that expresses what can occur where.

           -------------------------- HsStmtContext -------------------
           Pattern       List  PArr                   Inside   Inside 
Stmt       guards   GHCi comp  comp   do  mdo  arrow  ParStmt  TransStmt
------------------------------------------------------------------------
LastStmt                   x     x     x   x    ??
BindStmt    x        x     x     x     x   x    x       x        x
ExprStmt    x        x     x     x     x   x    x       x        x
LetStmt     x        x     x     x     x   x    x       x        x
ParStmt                    x                            x        x
TransStmt                  x                            x        x
RecStmt                                x   x    x       ??       ??

Once we are sure it's right, it'd be good to add it to HsExpr.

  Changed 9 months ago by guest

See also #1777

follow-up: ↓ 8   Changed 7 months ago by igloo

  • status changed from new to closed
  • resolution set to fixed

I believe this was fixed by:

commit ba56d20d767f0425f6f7515fa9c78b186589b896

Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Wed Oct 3 11:16:22 2012 +0100

    This big patch re-factors the way in which arrow-syntax is handled
    
    All the work was done by Dan Winograd-Cort.
    
    The main thing is that arrow comamnds now have their own
    data type HsCmd (defined in HsExpr).  Previously it was
    punned with the HsExpr type, which was jolly confusing,
    and made it hard to do anything arrow-specific.
    
    To make this work, we now parameterise
      * MatchGroup
      * Match
      * GRHSs, GRHS
      * StmtLR and friends
    over the "body", that is the kind of thing they
    enclose.  This "body" parameter can be instantiated to
    either LHsExpr or LHsCmd respectively.
    
    Everything else is really a knock-on effect; there should
    be no change (yet!) in behaviour.  But it should be a sounder
    basis for fixing bugs.

follow-up: ↓ 9   Changed 7 months ago by jeltsch

Does the fix of this bug also resolve feature request #7081 or parts of it, that is, does desugaring for “lambda case” and “multi-way if” now work generically for expressions and arrows?

in reply to: ↑ 6   Changed 7 months ago by jeltsch

Replying to 6:

All the work was done by Dan Winograd-Cort.

Thanks a lot, Dan.

in reply to: ↑ 7   Changed 7 months ago by simonpj

Replying to jeltsch:

Does the fix of this bug also resolve feature request #7081 or parts of it, that is, does desugaring for “lambda case” and “multi-way if” now work generically for expressions and arrows?

I'm not sure, but even if not it shouldn't be hard. If those who care about arrows would like to try it, say what works and what doesn't, propose changes, offer patches, I'm very open to that.

Simon

Note: See TracTickets for help on using tickets.