New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pattern matching in let and where clauses #764

Closed
jutaro opened this Issue Dec 11, 2014 · 32 comments

Comments

Projects
None yet
@jutaro
Copy link

jutaro commented Dec 11, 2014

This is what I miss most currently as bloody beginner (beside more precise parser and type checker error messages). Don't know if it is in the issue list so far but can't find it. e.g:

let (X a) = X b
in a

currently is not working, right? As far as I see the same applies to all other patterns.
I can write something like this instead, but it can make the code less clear (not in this example):

case X b of
  X a -> a

@paf31 paf31 added this to the Backlog milestone Dec 11, 2014

@paf31

This comment has been minimized.

Copy link
Member

paf31 commented Dec 11, 2014

Yes, this could be implemented as sugar for what you wrote. I omitted it because it made parsing tricky, but it could be done.

However, binders in top level declarations should probably be disallowed.

@Fresheyeball

This comment has been minimized.

Copy link

Fresheyeball commented Dec 11, 2014

However, binders in top level declarations should probably be disallowed.

I'd like to understand what you mean here.

@garyb

This comment has been minimized.

Copy link
Member

garyb commented Dec 11, 2014

He means you shouldn't be allowed to make a declaration like (X a) = X b at the module level.

@garyb

This comment has been minimized.

Copy link
Member

garyb commented Dec 11, 2014

(It's because the Let constructor in the AST currently has a list of Declarations inside it, so we'd need to deal with that by changing Let rather than making a binding declaration of some kind)

@paf31

This comment has been minimized.

Copy link
Member

paf31 commented Dec 11, 2014

I would actually rather not implement this, just because I think it will complicate all sorts of things: binding groups calculations, code gen, etc. It's much less trivial than it seems.

@garyb

This comment has been minimized.

Copy link
Member

garyb commented Dec 11, 2014

Is it not just a case of desugaring into cases?

@garyb

This comment has been minimized.

Copy link
Member

garyb commented Dec 11, 2014

I'm on the fence about it too though, as irrefutable patterns and sum types don't mix well.

@paf31

This comment has been minimized.

Copy link
Member

paf31 commented Dec 11, 2014

I don't think so. How would you desugar

let Tuple x y = blah

except into something like

let fresh_name = blah
    x = case fresh_name of Tuple fresh_name_2 y -> fresh_name_2
    y = case fresh_name of x Tuple fresh_name_3 -> fresh_name_3

which isn't exactly going to generate clean JS? :)

@garyb

This comment has been minimized.

Copy link
Member

garyb commented Dec 11, 2014

I was thinking

let (Tuple x y) = blah
in ...

becomes

case blah of
  Tuple x y -> ...
@paf31

This comment has been minimized.

Copy link
Member

paf31 commented Dec 11, 2014

But what about

map f xs = 
  let y : ys = xs
      z = f y
      zs = map f ys
  in z : zs

Edit: point is, your declarations in a let have to be sorted by dependencies, which means you have to do the binding groups pass first, which means that has to change.

@garyb

This comment has been minimized.

Copy link
Member

garyb commented Dec 11, 2014

Ah ok, I see where you're coming from. I think it can still work:

map f xs = 
  case xs of
    y : ys -> 
      let z = f y
          zs = map f ys
      in z : zs

But yeah, it isn't straightforward as you'd have to split the let into nested cases and lets, and use a toposort to determine the order of everything too.

@nrolland

This comment has been minimized.

Copy link

nrolland commented May 7, 2015

wouldn't a version where the ordering has to be linearly specified, no topo sort, be useful ?

I think in many of the cases I use it with only one case or parallel destructors anyway.
having to introduce extraneous binders is strange after being too spoiled. I guess it makes the generated code closer to input code but in most case i think that's moot

@paf31 paf31 changed the title pattern matching in let and where clauses Pattern matching in let and where clauses Nov 24, 2015

@user471

This comment has been minimized.

Copy link

user471 commented Jan 5, 2016

👍

2 similar comments
@AndrasKovacs

This comment has been minimized.

Copy link

AndrasKovacs commented Jan 16, 2016

👍

@libscott

This comment has been minimized.

Copy link

libscott commented Feb 14, 2016

+1

@jdegoes

This comment has been minimized.

Copy link

jdegoes commented Feb 14, 2016

I'd definitely use this.

@felixSchl

This comment has been minimized.

Copy link
Contributor

felixSchl commented Feb 20, 2016

So would I, this would be great to have. 👍

@vstastny

This comment has been minimized.

Copy link

vstastny commented Apr 6, 2016

I would also vote for having this.

@zhangchiqing

This comment has been minimized.

Copy link

zhangchiqing commented Nov 18, 2016

Any update on this feature?

I just ran into this:

  inst <- liftEff now
  let (Milliseconds n) = unInstant inst

psc 0.10.1 will throw this error

  Unable to parse module:
  unexpected (
  expecting local declaration
@paf31

This comment has been minimized.

Copy link
Member

paf31 commented Nov 18, 2016

No update, but it's on the Approved list, so if someone wants to work on it, we can give advice on the design/implementation.

@utatti

This comment has been minimized.

Copy link
Contributor

utatti commented Jan 26, 2017

I would like to work on this. @paf31, can you give me hints on how to implement this? Maybe with desugaring into case expression as mentioned above?

@paf31

This comment has been minimized.

Copy link
Member

paf31 commented Jan 27, 2017

@noraesae Great! This will definitely be implemented as a desugaring pass, but the details need to be worked out:

  • What changes we need in the parser, and how we can minimize backtracking
  • How this fits into the desugaring process, taking into account the need to look at binding groups
@utatti

This comment has been minimized.

Copy link
Contributor

utatti commented Jan 27, 2017

Thanks for your comment. This is my first contribution on PureScript and can take a while to be accustomed to its code base.

As you suggested, I will start from looking into the parser, and see what kind of change is needed. I will update the process as frequently as possible, as I may not be sure if I am doing it right 😄 Surely, further detailed direction will be absolutely welcomed.

@utatti

This comment has been minimized.

Copy link
Contributor

utatti commented Feb 1, 2017

Working on the parser. I have created an example too.

I am thinking about working on desugaring from now on. Review or further direction is super-welcomed.

@utatti

This comment has been minimized.

Copy link
Contributor

utatti commented Feb 4, 2017

I have implemented desugaring of let-in expression. From now, I will implement desugaring for let statement in do expression.

For the content, please refer to the following commit. I really appreciate early reviews, as it helps me to do the job later in the right way.

I will upload a PR after the do let desugaring is done, every tests are passing, and #2611 is merged (cannot make it work without it in my env).

@paf31

This comment has been minimized.

Copy link
Member

paf31 commented Feb 4, 2017

@noraesae let in do notation just desugars to regular let, so you shouldn't need to do any additional work there. Thanks!

@utatti

This comment has been minimized.

Copy link
Contributor

utatti commented Feb 5, 2017

@paf31 Great to know! I may then reorder the desugaring passes and test.

@paf31

This comment has been minimized.

Copy link
Member

paf31 commented Feb 5, 2017

@noraesae You might not be able to just reorder them - the order can be important - but try it and see :)

@utatti

This comment has been minimized.

Copy link
Contributor

utatti commented Feb 5, 2017

As you said, I placed desugarLetPatternModule after desugarDoModule to use the desugared version of let in do notation. Everything seems like working now. 🎉

I will clean up the code and commits, rebase with #2611, test again, and then upload a PR.

Thank you very much for your directions. It helped me a lot.

@utatti

This comment has been minimized.

Copy link
Contributor

utatti commented Feb 10, 2017

This issue can be closed?

@paf31 paf31 closed this Feb 10, 2017

@paf31

This comment has been minimized.

Copy link
Member

paf31 commented Feb 10, 2017

Yes, thanks!

@utatti

This comment has been minimized.

Copy link
Contributor

utatti commented Feb 10, 2017

Thank you very much for your help too! 🎉 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment