Lift lambdas

[hmontero1205]

Opening a draft to get some feedback in. Need advice on how to type lifted lambdas and general monadic programming critiques :)

Closes #32

[j-hui]

Thanks Hans! This is great start. Wrote some style suggestions (:

[hmontero1205]

@XijiaoLi I ran into a very interesting issue while I was trying to make my code pass the regression tests. Consider main routine of doubleblink.ssl:

main(led : &Int) -> () =
  par ((slow: &Int -> ()) (led: &Int): ())
         ((fast: &Int -> ()) (led: &Int): ())

While the type annotation says main returns a unit type, I don't think that's technically true right? Because the type of the body is ((), ()). In my code, I have to expand chained lambdas and then collect them which requires me to reconstruct the lambda chain (and therefore type annotate each lambda). I was originally ignoring the original type annotation on the overall function because I assumed it'd be exactly the same as the type annotations in the body after type inference. However, I was wrong because I was failing the regression tests. My code, upon reconstructing the main function, incorrectly(?) annotated it as `(&Int) -> ((), ()) which makes codegen fail since we don't have tuples yet.

Is the behavior I encountered expected? Does your type inference unify(?) ((), ()) and ()?

[XijiaoLi]

@XijiaoLi I ran into a very interesting issue while I was trying to make my code pass the regression tests. Consider main routine of doubleblink.ssl:

main(led : &Int) -> () =
  par ((slow: &Int -> ()) (led: &Int): ())
         ((fast: &Int -> ()) (led: &Int): ())

While the type annotation says main returns a unit type, I don't think that's technically true right? Because the type of the body is ((), ()). In my code, I have to expand chained lambdas and then collect them which requires me to reconstruct the lambda chain (and therefore type annotate each lambda). I was originally ignoring the original type annotation on the overall function because I assumed it'd be exactly the same as the type annotations in the body after type inference. However, I was wrong because I was failing the regression tests. My code, upon reconstructing the main function, incorrectly(?) annotated it as `(&Int) -> ((), ()) which makes codegen fail since we don't have tuples yet.

Is the behavior I encountered expected? Does your type inference unify(?) ((), ()) and ()?

It should be ((), ()). I think it was a mistake that was made in the first version of doubleblink.ssl and we just haven't changed it. My type inference should infer the type of par as Tuple [t] Builtin type (e.g., ((), ()) in this case). However, I believe that the current Codegen does not support Tuple type yet based on Codegen.hs#L164.

[j-hui]

Thanks @hmontero1205 ! This is awesome work.

This looks good overall. I left a bunch of suggestions inline, mostly places where you could have used existing helpers.

I'm requesting changes because this is still missing Haddock comments, and a test suite demonstrating correctness. For the test suite, add a new test suite named ir-to-ir where we can write tests that assert the correctness of source-to-source transformations.

[hmontero1205]

It should be ((), ()). I think it was a mistake that was made in the first version of doubleblink.ssl and we just haven't changed it. My type inference should infer the type of par as Tuple [t] Builtin type (e.g., ((), ()) in this case). However, I believe that the current Codegen does not support Tuple type yet based on Codegen.hs#L164.

@XijiaoLi I'm confused then -- does your type inference logic compare main's annotated return type of () to ((), ())?

I'm still not sure which type annotation I should be following when I am reconstructing lambdas -- the type annotation on the lambda itself or the type annotation on the lambda bodies. To reiterate, I was doing the latter but I'm not sure if I am supposed to be doing the former or if it really shouldn't matter to this compiler pass? @j-hui any thoughts?

[j-hui]

`par` should return a tuple of all its expressions.

I'm still not sure which type annotation I should be following when I am

reconstructing lambdas -- the type annotation on the lambda itself or the type annotation on the lambda bodies The two should be more or less equivalent; I think the difference you're seeing is not intentional. I think it's easier to use the type annotation on the overall lambda.

…

On Fri, Nov 26, 2021 at 1:40 PM Hans Montero ***@***.***> wrote: It should be ((), ()). I think it was a mistake that was made in the first version of doubleblink.ssl and we just haven't changed it. My type inference should infer the type of par as Tuple [t] Builtin type (e.g., ((), ()) in this case). However, I believe that the current Codegen does not support Tuple type yet based on Codegen.hs#L164 <https://github.com/ssm-lang/sslang/blob/main/src/Codegen/Codegen.hs#L164> . @XijiaoLi <https://github.com/XijiaoLi> I'm confused then -- does your type inference logic compare main's annotated return type of () to ((), ())? I'm still not sure which type annotation I should be following when I am reconstructing lambdas -- the type annotation on the lambda itself or the type annotation on the lambda bodies. To reiterate, I was doing the latter but I'm not sure if I am supposed to be doing the former or if it really shouldn't matter to this compiler pass? @j-hui <https://github.com/j-hui> any thoughts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC2A5DDRXMWMT2BMQEYGUGTUN7ICFANCNFSM5IA3ILPQ> .

[XijiaoLi]

@XijiaoLi I'm confused then -- does your type inference logic compare main's annotated return type of () to ((), ())?

I'm still not sure which type annotation I should be following when I am reconstructing lambdas -- the type annotation on the lambda itself or the type annotation on the lambda bodies. To reiterate, I was doing the latter but I'm not sure if I am supposed to be doing the former or if it really shouldn't matter to this compiler pass? @j-hui any thoughts?

I mean, the current type annotation for main's return type is wrong - it should be ((), ()) since the type of par should be a tuple. The type annotation on the lambda bodies should always be the same as the type annotation on the lambda's return type. We should really fix this regression test.

[j-hui]

@XijiaoLi could you issue a PR to fix that? Happy to stamp it.

…

On Fri, Nov 26, 2021 at 1:47 PM Xijiao ***@***.***> wrote: @XijiaoLi <https://github.com/XijiaoLi> I'm confused then -- does your type inference logic compare main's annotated return type of () to ((), ())? I'm still not sure which type annotation I should be following when I am reconstructing lambdas -- the type annotation on the lambda itself or the type annotation on the lambda bodies. To reiterate, I was doing the latter but I'm not sure if I am supposed to be doing the former or if it really shouldn't matter to this compiler pass? @j-hui <https://github.com/j-hui> any thoughts? I mean, the current type annotation for main's return type is wrong - it should be ((), ()) since the type of par should be a tuple. The type annotation on the lambda bodies should always be the same as the type annotation on the lambda's return type. We should really fix this regression test. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC2A5DGTRQM34P3BX5OGKGTUN7I3PANCNFSM5IA3ILPQ> .

[hmontero1205]

@j-hui check this out, now with tests and documentation 😳

[j-hui]

LGTM overall! I'm going to stamp this for now, but there still are a few things to follow up on, for which I left comments inline.

Also, could we add a more deeply nested example? Something like:

f x y =
  let z = 4
  let g a =
    let h b = a + b + x
    h z
  g y