Preliminary implementation of let-bindings. #133

boomshroom · 2018-09-07T03:34:54Z

This should help going forward. There are several things this does not cover which will needed to be worked on in the future, namely recursion and mutual recursion, as well as type inference. let x : I32; x = 5; in x does not work in this version.

Another huge thing missing right now are tests. At the very least, all previous tests seem to pass and manual checking in the repl does what's expected.

This also includes a few refactors such as pulling code out of the module translation function to reuse for let-bindings and possibly in the future, records. In addition, I added InternalError::Unimplemented{ feat: String } to make it easier to stub behavior without panicking.

brendanzab

Sorry for all the comments - this is really exciting! Thanks for working on it! I'll try to think up some test cases for us to try!

brendanzab · 2018-09-07T03:39:59Z

src/semantics/errors.rs

@@ -24,6 +24,8 @@ pub enum InternalError {
    ProjectedOnNonExistentField { label: syntax::Label },
    #[fail(display = "No patterns matched the given expression.")]
    NoPatternsApplicable,
+    #[fail(display = "Feature unimplemented: {}", feat)]
+    Unimplemented{ feat: String }


SO GOOD! Thanks!

brendanzab · 2018-09-07T03:41:47Z

src/semantics/mod.rs

@@ -356,7 +356,7 @@ where
            let raw_ty_fields = raw_ty_fields.unnest();

            if raw_fields.len() != raw_ty_fields.len() {
-                unimplemented!();
+                return Err(TypeError::Internal(InternalError::Unimplemented{feat: "record size mismatch. (change to RecordSizeMismatch)".to_string()}));


Do you have rustfmt running? I try to have this on by default. Also we can probably remove this whole check because it is already covered in the check you added in #130! 😄

I made that return an error just in case. It should never happen with the previous check, but you can never be too careful.

brendanzab · 2018-09-07T03:42:07Z

src/semantics/mod.rs

@@ -576,6 +576,32 @@ where
            ))
        },

+        // I-LET
+        raw::Term::Let(_, ref raw_scope) => {
+            //return Err(TypeError::Internal(InternalError::Unimplemented{feat: "let-binding".to_string()}));


Stray comment here!

brendanzab · 2018-09-07T03:44:52Z

src/semantics/mod.rs

@@ -576,6 +576,32 @@ where
            ))
        },

+        // I-LET


This refers to an inference rule in theory.md that we'll have to update. It's a bit icky to work with (a bunch of mathjax) so I can update it in a separate PR if you like. I have some ideas for overhauling the specification website - see #109 for that!

brendanzab · 2018-09-07T03:46:21Z

src/syntax/raw.rs

@@ -190,6 +190,7 @@ pub enum Term {
    Case(ByteSpan, RcTerm, Vec<Scope<RcPattern, RcTerm>>),
    /// Array literals
    Array(ByteSpan, Vec<RcTerm>),
+    Let(ByteSpan, Scope<(Binder<String>, Embed<(RcTerm, RcTerm)>), RcTerm>)


Missing a doc comment here

brendanzab · 2018-09-07T03:48:02Z

src/syntax/translation/desugar/mod.rs

+    items: &[concrete::Item],
+    body: &concrete::Term,
+) -> raw::RcTerm {
+    //unimplemented!("let bindings");


Stray comment

brendanzab · 2018-09-07T03:49:45Z

src/syntax/translation/desugar/mod.rs

+    let hole = raw::RcTerm::from(raw::Term::Hole(ByteSpan::default()));
+
+    items.into_iter().rev().fold(body, |acc, item| match item {
+        raw::Item::Declaration{..} => acc, // TODO Let declarations


Super obsessive nit-pick: I try to use a colon in TODOs, eg: // TODO: let declarations

We might want to reuse the module level desugaring for this, but I'm cool with keeping this PR simpler.

brendanzab · 2018-09-07T03:50:04Z

src/syntax/translation/desugar/mod.rs

@@ -189,51 +247,13 @@ fn desugar_record(

 impl Desugar<raw::Module> for concrete::Module {
    fn desugar(&self, env: &DesugarEnv) -> raw::Module {
-        let mut env = env.clone();
+        //let mut env = env.clone();


Stray comment

brendanzab · 2018-09-07T03:51:07Z

src/syntax/translation/resugar.rs

@@ -391,6 +391,7 @@ fn resugar_term(term: &core::Term, prec: Prec) -> concrete::Term {
                vec![resugar_term(arg, Prec::NO_WRAP)], // TODO
            ),
        ),
+        core::Term::Let(ref _scope) => unimplemented!(),//resugar_let(scope, prec),


Stray comment here - did you want to implement this in this PR, or later? I'm cool if you want to defer it!

boomshroom · 2018-09-07T06:47:07Z

Thanks for the comments. I wrote the changes then went to sleep planning to fix things up, but forgot when I decided not to bother with tests.

boomshroom · 2018-09-07T06:54:25Z

Seems like cargo fmt changed more than my changes.

brendanzab · 2018-09-07T09:47:50Z

Yeah, perfectly fine if rustfmt changes more things. I wonder if it is because I'm using the nightly version... 🤔

brendanzab · 2018-09-08T05:58:21Z

woww, the stable version of rustfmt actually looks quite ugly compared to nightly 😅

brendanzab · 2018-09-08T06:37:54Z

Would you be able to rebase this branch onto https://github.com/brendanzab/pikelet/tree/let-binding - I ran the nightly rustfmt on it, and added some tests.

brendanzab requested changes Sep 7, 2018

View reviewed changes

boomshroom and others added 3 commits September 8, 2018 17:43

Preliminary implementation of let-bindings.

092ebe3

Tweaks to Unimplemented error type

d827856

Add some tests

202b89b

boomshroom force-pushed the let-binding branch 2 times, most recently from d41a9a7 to c073242 Compare September 9, 2018 01:11

brendanzab force-pushed the let-binding branch from c073242 to 202b89b Compare September 9, 2018 03:14

brendanzab approved these changes Sep 9, 2018

View reviewed changes

brendanzab merged commit 6a0adb8 into pikelet-lang:master Sep 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preliminary implementation of let-bindings. #133

Preliminary implementation of let-bindings. #133

boomshroom commented Sep 7, 2018

brendanzab left a comment

brendanzab Sep 7, 2018

brendanzab Sep 7, 2018

boomshroom Sep 7, 2018

brendanzab Sep 7, 2018

brendanzab Sep 7, 2018

brendanzab Sep 7, 2018

brendanzab Sep 7, 2018

brendanzab Sep 7, 2018

brendanzab Sep 7, 2018

brendanzab Sep 7, 2018

brendanzab Sep 7, 2018

boomshroom commented Sep 7, 2018

boomshroom commented Sep 7, 2018

brendanzab commented Sep 7, 2018 •

edited

Loading

brendanzab commented Sep 8, 2018

brendanzab commented Sep 8, 2018

Preliminary implementation of let-bindings. #133

Preliminary implementation of let-bindings. #133

Conversation

boomshroom commented Sep 7, 2018

brendanzab left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boomshroom commented Sep 7, 2018

boomshroom commented Sep 7, 2018

brendanzab commented Sep 7, 2018 • edited Loading

brendanzab commented Sep 8, 2018

brendanzab commented Sep 8, 2018

brendanzab commented Sep 7, 2018 •

edited

Loading