Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#7071 closed bug (fixed)

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 Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description (last modified by simonpj)

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 (9)

comment:1 Changed 3 years ago by simonpj

  • Description modified (diff)

comment:2 Changed 3 years ago by simonpj

  • Description modified (diff)

comment:3 in reply to: ↑ description Changed 3 years 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.

comment:4 Changed 3 years 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.

comment:5 Changed 3 years ago by guest

See also #1777

comment:6 follow-up: Changed 3 years ago by igloo

  • Resolution set to fixed
  • Status changed from new to closed

I believe this was fixed by:

commit ba56d20d767f0425f6f7515fa9c78b186589b896

Author: Simon Peyton Jones <[email protected]>
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.

comment:7 follow-up: Changed 3 years 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?

comment:8 in reply to: ↑ 6 Changed 3 years ago by jeltsch

Replying to 6:

All the work was done by Dan Winograd-Cort.

Thanks a lot, Dan.

comment:9 in reply to: ↑ 7 Changed 3 years 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.