Type var declarations as void (fixes #22)

src/checker.rs

@@ -685,7 +685,15 @@ impl Checker {
                     mutable: true,
                 });
 
-                ty
+                // A var declaration is a statement, not an expression: it does
+                // not produce a value. Record the variable's type for the
+                // backends (they read types[id] to size the slot), but report
+                // the declaration itself as Void so an enclosing block or
+                // if-else doesn't treat the declaration as its result. See
+                // issue #22: codegen emits a placeholder i32 0 here, which used
+                // to flow into a merge block typed from the declared type.
+                self.types[id] = ty;
+                return mk_type(Type::Void);
             }
             Expr::Let(name, init, ty) => {
                 let ty = if let Some(ty) = ty { *ty } else { self.fresh() };
@@ -1337,6 +1345,7 @@ impl Checker {
             self._check_decl(decl, decls);
             if let Decl::Func(fd) | Decl::Macro(fd) = decl {
                 check_escape_in_func(fd, &mut self.errors);
+                self.check_void_declarations(fd);
                 self.check_unsolved_types(fd);
             }
         }
@@ -1346,10 +1355,40 @@ impl Checker {
         self._check_decl(decl, decls);
         if let Decl::Func(fd) | Decl::Macro(fd) = decl {
             check_escape_in_func(fd, &mut self.errors);
+            self.check_void_declarations(fd);
             self.check_unsolved_types(fd);
         }
     }
 
+    /// Reject variables declared with type void.
+    ///
+    /// Void isn't a value type in lyte — there's nothing to store and nothing
+    /// you can later do with the binding — so a void-typed declaration is
+    /// always a mistake. Reporting it here points at the declaration rather
+    /// than at whatever first tried to use the variable.
+    ///
+    /// Only runs if no other errors have been reported: unvisited expressions
+    /// keep the Void fill value from `check_fn_decl`, so a function that bailed
+    /// out early would otherwise produce false positives.
+    fn check_void_declarations(&mut self, func_decl: &FuncDecl) {
+        if !self.errors.is_empty() {
+            return;
+        }
+        let solved_types = self.solved_types();
+        for (i, expr) in func_decl.arena.exprs.iter().enumerate() {
+            let name = match expr {
+                Expr::Let(name, _, _) | Expr::Var(name, _, _) => name,
+                _ => continue,
+            };
+            if i < solved_types.len() && matches!(&*solved_types[i], Type::Void) {
+                self.errors.push(TypeError {
+                    location: func_decl.arena.locs[i],
+                    message: format!("variable '{}' cannot have type void", name),
+                });
+            }
+        }
+    }
+
     /// Detect unsolved type variables in non-generic functions.
     /// Only runs if no other errors have been reported (unsolved vars
     /// are usually a symptom of an earlier type error).

src/jit.rs

@@ -720,6 +720,28 @@ impl<'a> FunctionTranslator<'a> {
         self.builder.seal_block(continue_block);
     }
 
+    /// Check that a value about to be passed to an if-else merge block matches
+    /// the type the merge block param was declared with.
+    ///
+    /// The two are derived independently: the param from `decl.types`, the
+    /// value from codegen. They disagree when an expression kind is typed
+    /// non-void by the checker but returns a placeholder here (see issue #22,
+    /// where a `var` declaration was typed f32 but yielded `iconst(I32, 0)`).
+    /// Failing at the mismatch names the branch and both types; letting it
+    /// through yields an opaque Cranelift verifier failure, or — when the
+    /// placeholder happens to be the right Cranelift type — well-formed IR
+    /// that computes the wrong answer.
+    fn check_merge_arg(&self, val: Value, expected: Type, branch: &str) {
+        let actual = self.builder.func.dfg.value_type(val);
+        assert_eq!(
+            actual, expected,
+            "JIT internal error: {} branch of if-else produced a {} value, \
+             but the merge block expects {}. The checker and codegen disagree \
+             about what this branch evaluates to.",
+            branch, actual, expected
+        );
+    }
+
     fn translate_lvalue(&mut self, expr: ExprID, decl: &FuncDecl, decls: &DeclTable) -> Value {
         match &decl.arena[expr] {
             Expr::Id(name) => {
@@ -1467,10 +1489,13 @@ impl<'a> FunctionTranslator<'a> {
                     false
                 };
 
-                if is_value {
+                let merge_ty = if is_value {
                     let cl_ty = result_ty.cranelift_type();
                     self.builder.append_block_param(merge_block, cl_ty);
-                }
+                    Some(cl_ty)
+                } else {
+                    None
+                };
 
                 // Branch based on condition.
                 self.builder
@@ -1482,7 +1507,8 @@ impl<'a> FunctionTranslator<'a> {
                 self.builder.seal_block(then_block);
                 let then_val = self.translate_expr(*then_id, decl, decls);
                 if !self.builder.is_unreachable() {
-                    if is_value {
+                    if let Some(merge_ty) = merge_ty {
+                        self.check_merge_arg(then_val, merge_ty, "then");
                         self.builder
                             .ins()
                             .jump(merge_block, &[codegen::ir::BlockArg::Value(then_val)]);
@@ -1500,7 +1526,8 @@ impl<'a> FunctionTranslator<'a> {
                     self.builder.ins().iconst(I32, 0)
                 };
                 if !self.builder.is_unreachable() {
-                    if is_value {
+                    if let Some(merge_ty) = merge_ty {
+                        self.check_merge_arg(else_val, merge_ty, "else");
                         self.builder
                             .ins()
                             .jump(merge_block, &[codegen::ir::BlockArg::Value(else_val)]);

src/parser.rs

@@ -739,33 +739,38 @@ fn skip_reserved(cx: &mut ParseContext) {
 fn parse_stmt(arena: &mut ExprArena, typevars: &[Name], cx: &mut ParseContext) -> ExprID {
     match &cx.lex.tok {
         Token::Var => {
+            // Capture the location of the `var` keyword: cx.lex.loc has moved
+            // past the initializer by the time the expression is added, which
+            // would anchor diagnostics to the following line.
+            let loc = cx.lex.loc;
             cx.next();
             let name = expect_id(cx);
 
             if cx.lex.tok == Token::Assign {
                 cx.next();
                 let e = parse_lambda(arena, typevars, cx);
-                arena.add(Expr::Var(name, Some(e), None), cx.lex.loc)
+                arena.add(Expr::Var(name, Some(e), None), loc)
             } else if cx.lex.tok == Token::Colon {
                 cx.next();
                 let t = parse_type(typevars, cx);
-                arena.add(Expr::Var(name, None, Some(t)), cx.lex.loc)
+                arena.add(Expr::Var(name, None, Some(t)), loc)
             } else {
                 cx.err(String::from("expected assignment or type"));
-                arena.add(Expr::Var(name, None, None), cx.lex.loc)
+                arena.add(Expr::Var(name, None, None), loc)
             }
         }
         Token::Let => {
+            let loc = cx.lex.loc;
             cx.next();
             let name = expect_id(cx);
 
             if cx.lex.tok == Token::Assign {
                 cx.next();
                 let e = parse_lambda(arena, typevars, cx);
-                arena.add(Expr::Let(name, e, None), cx.lex.loc)
+                arena.add(Expr::Let(name, e, None), loc)
             } else {
                 cx.err(String::from("expected assignment or type"));
-                arena.add(Expr::Error, cx.lex.loc)
+                arena.add(Expr::Error, loc)
             }
         }
         Token::Arena => {

tests/cases/checker/not_a_struct.lyte

@@ -3,9 +3,9 @@
 // ❌ ../tests/cases/checker/not_a_struct.lyte:11:14: ambiguous constraint: i32.foo == ?3
 //         var y = x.foo
 //                  ^
-// ❌ ../tests/cases/checker/not_a_struct.lyte:12:1: ambiguous constraint: ?3 == ?3
-//     }
-//     ^
+// ❌ ../tests/cases/checker/not_a_struct.lyte:11:5: ambiguous constraint: ?3 == ?3
+//         var y = x.foo
+//         ^
 f {
     var x = 42
     var y = x.foo

tests/cases/checker/var_decl_not_a_value.lyte

@@ -0,0 +1,14 @@
+// A `var` declaration is a statement, not an expression, so an if-else whose
+// branches end in one produces void rather than the variable's type. This used
+// to type-check and silently evaluate to 0 (issue #22).
+//
+// Void is not a value type, so binding it is rejected at the declaration.
+//
+// args: --check
+// expected stdout:
+// ❌ ../tests/cases/checker/var_decl_not_a_value.lyte:13:5: variable 'y' cannot have type void
+//         let y = if true { var x = 7 } else { var z = 9 }
+//         ^
+main {
+    let y = if true { var x = 7 } else { var z = 9 }
+}

tests/cases/var_decl_tail_in_if.lyte

@@ -0,0 +1,35 @@
+// Regression test for issue #22: a branch of an if-else ending in a `var`
+// declaration used to make the if-else look like it produced a value of the
+// variable's type, while codegen handed the merge block a placeholder i32 0.
+// With f32 branches that failed Cranelift verification:
+//   jump block7(v16): arg v16 has type i32, expected f32
+// A var declaration is a statement, so this if-else produces no value and
+// both branches simply fall through to the merge block.
+//
+// expected stdout:
+// compilation successful
+// assert(true)
+// assert(true)
+
+main {
+    var acc = 0.0
+    if acc < 1.0 {
+        acc = 2.0
+        var unused = 1.0
+    } else {
+        acc = 3.0
+        var unused = 4.0
+    }
+    assert(acc == 2.0)
+
+    // Same shape with integer branches, which used to silently yield 0.
+    var count = 0
+    if count == 0 {
+        count = 5
+        var ignored = 1
+    } else {
+        count = 6
+        var ignored = 2
+    }
+    assert(count == 5)
+}