Allow return as expression, allows short-circuit conditionals

In Julia and some other languages, the following is allowed:

  true && return;
  player == this && return true;

etc

Which is a nice abbreviated idiom to avoid large chains of if blocks

This is possible because in those languages "return" is an expression
not a statement.

This is now the case for moor, too.

This also fixes old broken string literal parsing code which returned
E_INVARG instead of compiler error for invalid ints.

crates/compiler/src/ast.rs

@@ -182,6 +182,7 @@ pub enum Expr {
         from: Box<Expr>,
         to: Box<Expr>,
     },
+    Return(Option<Box<Expr>>),
 }
 
 #[derive(Debug, Eq, PartialEq, Clone)]
@@ -280,10 +281,18 @@ pub enum StmtNode {
     Continue {
         exit: Option<UnboundName>,
     },
-    Return(Option<Expr>),
     Expr(Expr),
 }
 
+impl StmtNode {
+    pub fn mk_return(expr: Expr) -> Self {
+        StmtNode::Expr(Expr::Return(Some(Box::new(expr))))
+    }
+    pub fn mk_return_none() -> Self {
+        StmtNode::Expr(Expr::Return(None))
+    }
+}
+
 // Recursive descent compare of two trees, ignoring the parser-provided line numbers, but
 // validating equality for everything else.
 #[cfg(test)]
@@ -293,7 +302,6 @@ pub fn assert_trees_match_recursive(a: &[Stmt], b: &[Stmt]) {
         assert_eq!(left.tree_line_no, right.tree_line_no);
 
         match (&left.node, &right.node) {
-            (StmtNode::Return(_), StmtNode::Return(_)) => {}
             (StmtNode::Expr(e1), StmtNode::Expr(e2)) => {
                 assert_eq!(e1, e2);
             }

crates/compiler/src/codegen.rs

@@ -602,6 +602,14 @@ impl CodegenState {
                 });
                 self.commit_jump_label(end_label);
             }
+            Expr::Return(Some(expr)) => {
+                self.generate_expr(expr)?;
+                self.emit(Op::Return);
+            }
+            Expr::Return(None) => {
+                self.emit(Op::Return0);
+                self.push_stack(1);
+            }
         }
 
         Ok(())
@@ -902,12 +910,6 @@ impl CodegenState {
                 let l = self.find_loop(&l).expect("invalid loop for break/continue");
                 self.emit(Op::ExitId(l.top_label));
             }
-            StmtNode::Return(Some(expr)) => {
-                self.generate_expr(expr)?;
-                self.emit(Op::Return);
-                self.pop_stack(1);
-            }
-            StmtNode::Return(None) => self.emit(Op::Return0),
             StmtNode::Expr(e) => {
                 self.generate_expr(e)?;
                 self.emit(Op::Pop);

crates/compiler/src/codegen_tests.rs

@@ -73,6 +73,7 @@ mod tests {
                 Pop,
                 Push(a),
                 Return,
+                Pop,
                 Done
             ]
         );
@@ -111,6 +112,7 @@ mod tests {
                 If(1.into(), 0),
                 ImmInt(5),
                 Return,
+                Pop,
                 EndScope { num_bindings: 0 },
                 Jump { label: 0.into() },
                 ImmInt(2),
@@ -119,6 +121,7 @@ mod tests {
                 Eif(2.into(), 0),
                 ImmInt(3),
                 Return,
+                Pop,
                 EndScope { num_bindings: 0 },
                 Jump { label: 0.into() },
                 BeginScope {
@@ -127,6 +130,7 @@ mod tests {
                 },
                 ImmInt(6),
                 Return,
+                Pop,
                 EndScope { num_bindings: 0 },
                 Done
             ]
@@ -594,7 +598,7 @@ mod tests {
         let binary = compile(program, CompileOptions::default()).unwrap();
         assert_eq!(
             *binary.main_vector.as_ref(),
-            vec![Imm(0.into()), ImmInt(1), Ref, Return, Done]
+            vec![Imm(0.into()), ImmInt(1), Ref, Return, Pop, Done]
         );
     }
 
@@ -604,7 +608,15 @@ mod tests {
         let binary = compile(program, CompileOptions::default()).unwrap();
         assert_eq!(
             *binary.main_vector.as_ref(),
-            vec![Imm(0.into()), ImmInt(1), ImmInt(2), RangeRef, Return, Done]
+            vec![
+                Imm(0.into()),
+                ImmInt(1),
+                ImmInt(2),
+                RangeRef,
+                Return,
+                Pop,
+                Done
+            ]
         );
     }
 
@@ -671,6 +683,7 @@ mod tests {
                 ImmInt(1),
                 Ref,
                 Return,
+                Pop,
                 Done
             ]
         );
@@ -693,6 +706,7 @@ mod tests {
                 ImmInt(2),
                 RangeRef,
                 Return,
+                Pop,
                 Done
             ]
         );
@@ -770,6 +784,7 @@ mod tests {
                 RangeRef,
                 CheckListForSplice,
                 Return,
+                Pop,
                 Done
             ]
         );
@@ -961,6 +976,7 @@ mod tests {
                 ImmInt(1),
                 Ref,
                 Return,
+                Pop,
                 Done
             ]
         )
@@ -1258,6 +1274,7 @@ mod tests {
                 Push(c),
                 ListAddTail,
                 Return,
+                Pop,
                 Done
             ]
         )
@@ -1367,6 +1384,7 @@ mod tests {
                 Imm(binary.find_literal("stack".into())),
                 GetProp,
                 Return,
+                Pop,
                 Done
             ]
         )
@@ -1393,7 +1411,7 @@ mod tests {
     fn test_0_arg_return() {
         let program = r#"return;"#;
         let binary = compile(program, CompileOptions::default()).unwrap();
-        assert_eq!(*binary.main_vector.as_ref(), vec![Return0, Done])
+        assert_eq!(*binary.main_vector.as_ref(), vec![Return0, Pop, Done])
     }
 
     #[test]
@@ -1438,6 +1456,7 @@ mod tests {
                 Pop,
                 Push(pass),
                 Return,
+                Pop,
                 Done
             ]
         )
@@ -1487,6 +1506,7 @@ mod tests {
                 Length(Offset(0)),
                 RangeRef,
                 Return,
+                Pop,
                 EndExcept(Label(1)),
                 Pop,
                 Done
@@ -1551,6 +1571,7 @@ mod tests {
                 RangeRef,
                 MapInsert,
                 Return,
+                Pop,
                 Done
             ]
         );

crates/compiler/src/decompile.rs

@@ -428,12 +428,10 @@ impl Decompile {
             }
             Op::Return => {
                 let expr = self.pop_expr()?;
-                self.statements
-                    .push(Stmt::new(StmtNode::Return(Some(expr)), line_num));
+                self.push_expr(Expr::Return(Some(Box::new(expr))));
             }
             Op::Return0 => {
-                self.statements
-                    .push(Stmt::new(StmtNode::Return(None), line_num));
+                self.push_expr(Expr::Return(None));
             }
             Op::Done => {
                 let opcode_vector = &self.opcode_vector();

crates/compiler/src/moo.pest

@@ -22,7 +22,7 @@ statement  = {
   | labelled_fork_statement
   | break_statement
   | continue_statement
-  | return_statement
+  | empty_return
   | try_except_statement
   | try_finally_statement
   | begin_statement
@@ -41,6 +41,8 @@ for_statement    = { ^"for" ~ ident ~ "in" ~ (for_range_clause | for_in_clause)
 for_range_clause = { "[" ~ expr ~ ".." ~ expr ~ "]" }
 for_in_clause    = { "(" ~ expr ~ ")" }
 
+empty_return = { ^"return" ~ ";"}
+
 labelled_while_statement = { ^"while" ~ ident ~ "(" ~ expr ~ ")" ~ statements ~ ^"endwhile" }
 while_statement          = { ^"while" ~ "(" ~ expr ~ ")" ~ statements ~ ^"endwhile" }
 
@@ -75,7 +77,6 @@ global_assignment = { ^"global" ~ ident ~ (ASSIGN ~ expr)? ~ ";" }
 codes   = { anycode | exprlist }
 anycode = { ^"any" }
 
-return_statement = { ^"return" ~ (expr)? ~ ";" }
 expr_statement   = { (expr)? ~ ";" }
 
 expr = { (integer | (prefix* ~ primary)) ~ postfix* ~ (infix ~ (integer | (prefix* ~ primary)) ~ postfix*)* }
@@ -135,9 +136,11 @@ prop           =  { "." ~ ident }
 prop_expr      =  { "." ~ "(" ~ expr ~ ")" }
 assign         =  { "=" ~ expr }
 cond_expr      =  { "?" ~ expr ~ "|" ~ expr }
+return_expr    =  { "return" ~ (expr)? }
 
 primary    = _{
     pass_expr
+  | return_expr
   | builtin_call
   | paren_expr
   | sysprop_call