explicitly return from blocks

instead of last statement being expression value

closes #629

Author: andrewrk

doc/docgen.zig

@@ -49,7 +49,7 @@ fn gen(in: &io.InStream, out: &io.OutStream) {
             if (err == error.EndOfStream) {
                 return;
             }
-            std.debug.panic("{}", err)
+            std.debug.panic("{}", err);
         };
         switch (state) {
             State.Start => switch (byte) {

doc/langref.html.in

@@ -3021,14 +3021,13 @@ const assert = @import("std").debug.assert;</code></pre>
       <pre><code class="zig">const assert = @import("std").debug.assert;
 
 // Functions are declared like this
-// The last expression in the function can be used as the return value.
 fn add(a: i8, b: i8) -&gt; i8 {
     if (a == 0) {
         // You can still return manually if needed.
         return b;
     }
 
-    a + b
+    return a + b;
 }
 
 // The export specifier makes a function externally visible in the generated
@@ -5847,7 +5846,7 @@ ParamDeclList = "(" list(ParamDecl, ",") ")"
 
 ParamDecl = option("noalias" | "comptime") option(Symbol ":") (TypeExpr | "...")
 
-Block = option(Symbol ":") "{" many(Statement) option(Expression) "}"
+Block = option(Symbol ":") "{" many(Statement) "}"
 
 Statement = LocalVarDecl ";" | Defer(Block) | Defer(Expression) ";" | BlockExpression(Block) | Expression ";" | ";"
 

example/shared_library/mathtest.zig

@@ -1,3 +1,3 @@
 export fn add(a: i32, b: i32) -> i32 {
-    a + b
+    return a + b;
 }

src-self-hosted/parser.zig

@@ -111,11 +111,11 @@ pub const Parser = struct {
     }
 
     pub fn parse(self: &Parser) -> %&ast.NodeRoot {
-        const result = self.parseInner() %% |err| {
+        const result = self.parseInner() %% |err| x: {
             if (self.cleanup_root_node) |root_node| {
                 self.freeAst(root_node);
             }
-            err
+            break :x err;
         };
         self.cleanup_root_node = null;
         return result;
@@ -125,12 +125,12 @@ pub const Parser = struct {
         var stack = self.initUtilityArrayList(State);
         defer self.deinitUtilityArrayList(stack);
 
-        const root_node = {
+        const root_node = x: {
             const root_node = %return self.createRoot();
             %defer self.allocator.destroy(root_node);
             // This stack append has to succeed for freeAst to work
             %return stack.append(State.TopLevel);
-            root_node
+            break :x root_node;
         };
         assert(self.cleanup_root_node == null);
         self.cleanup_root_node = root_node;
@@ -462,7 +462,7 @@ pub const Parser = struct {
                     } else if (token.id == Token.Id.Keyword_noalias) {
                         param_decl.noalias_token = token;
                         token = self.getNextToken();
-                    };
+                    }
                     if (token.id == Token.Id.Identifier) {
                         const next_token = self.getNextToken();
                         if (next_token.id == Token.Id.Colon) {
@@ -793,14 +793,14 @@ pub const Parser = struct {
     }
 
     fn getNextToken(self: &Parser) -> Token {
-        return if (self.put_back_count != 0) {
+        if (self.put_back_count != 0) {
             const put_back_index = self.put_back_count - 1;
             const put_back_token = self.put_back_tokens[put_back_index];
             self.put_back_count = put_back_index;
-            put_back_token
+            return put_back_token;
         } else {
-            self.tokenizer.next()
-        };
+            return self.tokenizer.next();
+        }
     }
 
     const RenderAstFrame = struct {
@@ -873,7 +873,7 @@ pub const Parser = struct {
                                     Token.Id.Keyword_pub => %return stream.print("pub "),
                                     Token.Id.Keyword_export => %return stream.print("export "),
                                     else => unreachable,
-                                };
+                                }
                             }
                             if (fn_proto.extern_token) |extern_token| {
                                 %return stream.print("{} ", self.tokenizer.getTokenSlice(extern_token));
@@ -1102,7 +1102,7 @@ fn testParse(source: []const u8, allocator: &mem.Allocator) -> %[]u8 {
 // TODO test for memory leaks
 // TODO test for valid frees
 fn testCanonical(source: []const u8) {
-    const needed_alloc_count = {
+    const needed_alloc_count = x: {
         // Try it once with unlimited memory, make sure it works
         var fixed_allocator = mem.FixedBufferAllocator.init(fixed_buffer_mem[0..]);
         var failing_allocator = std.debug.FailingAllocator.init(&fixed_allocator.allocator, @maxValue(usize));
@@ -1116,7 +1116,7 @@ fn testCanonical(source: []const u8) {
             @panic("test failed");
         }
         failing_allocator.allocator.free(result_source);
-        failing_allocator.index
+        break :x failing_allocator.index;
     };
 
     // TODO make this pass

src/all_types.hpp

@@ -26,7 +26,6 @@ struct ScopeFnDef;
 struct TypeTableEntry;
 struct VariableTableEntry;
 struct ErrorTableEntry;
-struct LabelTableEntry;
 struct BuiltinFnEntry;
 struct TypeStructField;
 struct CodeGen;
@@ -54,7 +53,6 @@ struct IrExecutable {
     size_t *backward_branch_count;
     size_t backward_branch_quota;
     bool invalid;
-    ZigList<LabelTableEntry *> all_labels;
     ZigList<IrGotoItem> goto_list;
     bool is_inline;
     FnTableEntry *fn_entry;
@@ -452,7 +450,6 @@ struct AstNodeParamDecl {
 struct AstNodeBlock {
     Buf *name;
     ZigList<AstNode *> statements;
-    bool last_statement_is_result_expression;
 };
 
 enum ReturnKind {
@@ -1644,12 +1641,6 @@ struct ErrorTableEntry {
     ConstExprValue *cached_error_name_val;
 };
 
-struct LabelTableEntry {
-    AstNode *decl_node;
-    IrBasicBlock *bb;
-    bool used;
-};
-
 enum ScopeId {
     ScopeIdDecls,
     ScopeIdBlock,
@@ -1693,7 +1684,12 @@ struct ScopeDecls {
 struct ScopeBlock {
     Scope base;
 
-    HashMap<Buf *, LabelTableEntry *, buf_hash, buf_eql_buf> label_table;
+    Buf *name;
+    IrBasicBlock *end_block;
+    IrInstruction *is_comptime;
+    ZigList<IrInstruction *> *incoming_values;
+    ZigList<IrBasicBlock *> *incoming_blocks;
+
     bool safety_off;
     AstNode *safety_set_node;
     bool fast_math_off;

src/analyze.cpp

@@ -110,7 +110,7 @@ ScopeBlock *create_block_scope(AstNode *node, Scope *parent) {
     assert(node->type == NodeTypeBlock);
     ScopeBlock *scope = allocate<ScopeBlock>(1);
     init_scope(&scope->base, ScopeIdBlock, node, parent);
-    scope->label_table.init(1);
+    scope->name = node->data.block.name;
     return scope;
 }
 

src/ast_render.cpp

@@ -478,10 +478,7 @@ static void render_node_extra(AstRender *ar, AstNode *node, bool grouped) {
                 AstNode *statement = node->data.block.statements.at(i);
                 print_indent(ar);
                 render_node_grouped(ar, statement);
-                if (!(i == node->data.block.statements.length - 1 &&
-                      node->data.block.last_statement_is_result_expression)) {
-                    fprintf(ar->f, ";");
-                }
+                fprintf(ar->f, ";");
                 fprintf(ar->f, "\n");
             }
             ar->indent -= ar->indent_size;

src/ir.cpp

@@ -3514,7 +3514,11 @@ static VariableTableEntry *ir_create_var(IrBuilder *irb, AstNode *node, Scope *s
 static IrInstruction *ir_gen_block(IrBuilder *irb, Scope *parent_scope, AstNode *block_node) {
     assert(block_node->type == NodeTypeBlock);
 
+    ZigList<IrInstruction *> incoming_values = {0};
+    ZigList<IrBasicBlock *> incoming_blocks = {0};
+
     ScopeBlock *scope_block = create_block_scope(block_node, parent_scope);
+
     Scope *outer_block_scope = &scope_block->base;
     Scope *child_scope = outer_block_scope;
 
@@ -3528,9 +3532,15 @@ static IrInstruction *ir_gen_block(IrBuilder *irb, Scope *parent_scope, AstNode
         return ir_mark_gen(ir_build_const_void(irb, child_scope, block_node));
     }
 
+    if (block_node->data.block.name != nullptr) {
+        scope_block->incoming_blocks = &incoming_blocks;
+        scope_block->incoming_values = &incoming_values;
+        scope_block->end_block = ir_build_basic_block(irb, parent_scope, "BlockEnd");
+        scope_block->is_comptime = ir_build_const_bool(irb, parent_scope, block_node, ir_should_inline(irb->exec, parent_scope));
+    }
+
     bool is_continuation_unreachable = false;
     IrInstruction *noreturn_return_value = nullptr;
-    IrInstruction *return_value = nullptr;
     for (size_t i = 0; i < block_node->data.block.statements.length; i += 1) {
         AstNode *statement_node = block_node->data.block.statements.at(i);
 
@@ -3548,39 +3558,31 @@ static IrInstruction *ir_gen_block(IrBuilder *irb, Scope *parent_scope, AstNode
             // variable declarations start a new scope
             IrInstructionDeclVar *decl_var_instruction = (IrInstructionDeclVar *)statement_value;
             child_scope = decl_var_instruction->var->child_scope;
-        } else {
-            // label, defer, variable declaration will never be the result expression
-            if (block_node->data.block.last_statement_is_result_expression &&
-                i == block_node->data.block.statements.length - 1) {
-                // this is the result value statement
-                return_value = statement_value;
-            } else {
-                // there are more statements ahead of this one. this statement's value must be void
-                if (statement_value != irb->codegen->invalid_instruction) {
-                    ir_mark_gen(ir_build_check_statement_is_void(irb, child_scope, statement_node, statement_value));
-                }
-            }
+        } else if (statement_value != irb->codegen->invalid_instruction) {
+            // this statement's value must be void
+            ir_mark_gen(ir_build_check_statement_is_void(irb, child_scope, statement_node, statement_value));
         }
     }
 
     if (is_continuation_unreachable) {
         assert(noreturn_return_value != nullptr);
-        return noreturn_return_value;
+        if (block_node->data.block.name == nullptr || incoming_blocks.length == 0) {
+            return noreturn_return_value;
+        }
+    } else {
+        incoming_blocks.append(irb->current_basic_block);
+        incoming_values.append(ir_mark_gen(ir_build_const_void(irb, parent_scope, block_node)));
     }
-    // control flow falls out of block
 
-    if (block_node->data.block.last_statement_is_result_expression) {
-        // return value was determined by the last statement
-        assert(return_value != nullptr);
+    if (block_node->data.block.name != nullptr) {
+        ir_gen_defers_for_block(irb, child_scope, outer_block_scope, false);
+        ir_mark_gen(ir_build_br(irb, parent_scope, block_node, scope_block->end_block, scope_block->is_comptime));
+        ir_set_cursor_at_end(irb, scope_block->end_block);
+        return ir_build_phi(irb, parent_scope, block_node, incoming_blocks.length, incoming_blocks.items, incoming_values.items);
     } else {
-        // return value is implicitly void
-        assert(return_value == nullptr);
-        return_value = ir_mark_gen(ir_build_const_void(irb, child_scope, block_node));
+        ir_gen_defers_for_block(irb, child_scope, outer_block_scope, false);
+        return ir_mark_gen(ir_mark_gen(ir_build_const_void(irb, child_scope, block_node)));
     }
-
-    ir_gen_defers_for_block(irb, child_scope, outer_block_scope, false);
-
-    return return_value;
 }
 
 static IrInstruction *ir_gen_bin_op_id(IrBuilder *irb, Scope *scope, AstNode *node, IrBinOp op_id) {
@@ -5952,21 +5954,46 @@ static IrInstruction *ir_gen_comptime(IrBuilder *irb, Scope *parent_scope, AstNo
     return ir_gen_node_extra(irb, node->data.comptime_expr.expr, child_scope, lval);
 }
 
+static IrInstruction *ir_gen_return_from_block(IrBuilder *irb, Scope *break_scope, AstNode *node, ScopeBlock *block_scope) {
+    IrInstruction *is_comptime;
+    if (ir_should_inline(irb->exec, break_scope)) {
+        is_comptime = ir_build_const_bool(irb, break_scope, node, true);
+    } else {
+        is_comptime = block_scope->is_comptime;
+    }
+
+    IrInstruction *result_value;
+    if (node->data.break_expr.expr) {
+        result_value = ir_gen_node(irb, node->data.break_expr.expr, break_scope);
+        if (result_value == irb->codegen->invalid_instruction)
+            return irb->codegen->invalid_instruction;
+    } else {
+        result_value = ir_build_const_void(irb, break_scope, node);
+    }
+
+    IrBasicBlock *dest_block = block_scope->end_block;
+    ir_gen_defers_for_block(irb, break_scope, dest_block->scope, false);
+
+    block_scope->incoming_blocks->append(irb->current_basic_block);
+    block_scope->incoming_values->append(result_value);
+    return ir_build_br(irb, break_scope, node, dest_block, is_comptime);
+}
+
 static IrInstruction *ir_gen_break(IrBuilder *irb, Scope *break_scope, AstNode *node) {
     assert(node->type == NodeTypeBreak);
 
     // Search up the scope. We'll find one of these things first:
     // * function definition scope or global scope => error, break outside loop
     // * defer expression scope => error, cannot break out of defer expression
     // * loop scope => OK
+    // * (if it's a labeled break) labeled block => OK
 
     Scope *search_scope = break_scope;
     ScopeLoop *loop_scope;
-    bool saw_any_loop_scope = false;
     for (;;) {
         if (search_scope == nullptr || search_scope->id == ScopeIdFnDef) {
-            if (saw_any_loop_scope) {
-                add_node_error(irb->codegen, node, buf_sprintf("labeled loop not found: '%s'", buf_ptr(node->data.break_expr.name)));
+            if (node->data.break_expr.name != nullptr) {
+                add_node_error(irb->codegen, node, buf_sprintf("label not found: '%s'", buf_ptr(node->data.break_expr.name)));
                 return irb->codegen->invalid_instruction;
             } else {
                 add_node_error(irb->codegen, node, buf_sprintf("break expression outside loop"));
@@ -5977,13 +6004,20 @@ static IrInstruction *ir_gen_break(IrBuilder *irb, Scope *break_scope, AstNode *
             return irb->codegen->invalid_instruction;
         } else if (search_scope->id == ScopeIdLoop) {
             ScopeLoop *this_loop_scope = (ScopeLoop *)search_scope;
-            saw_any_loop_scope = true;
             if (node->data.break_expr.name == nullptr ||
                 (this_loop_scope->name != nullptr && buf_eql_buf(node->data.break_expr.name, this_loop_scope->name)))
             {
                 loop_scope = this_loop_scope;
                 break;
             }
+        } else if (search_scope->id == ScopeIdBlock) {
+            ScopeBlock *this_block_scope = (ScopeBlock *)search_scope;
+            if (node->data.break_expr.name != nullptr &&
+                (this_block_scope->name != nullptr && buf_eql_buf(node->data.break_expr.name, this_block_scope->name)))
+            {
+                assert(this_block_scope->end_block != nullptr);
+                return ir_gen_return_from_block(irb, break_scope, node, this_block_scope);
+            }
         }
         search_scope = search_scope->parent;
     }
@@ -6022,10 +6056,9 @@ static IrInstruction *ir_gen_continue(IrBuilder *irb, Scope *continue_scope, Ast
 
     Scope *search_scope = continue_scope;
     ScopeLoop *loop_scope;
-    bool saw_any_loop_scope = false;
     for (;;) {
         if (search_scope == nullptr || search_scope->id == ScopeIdFnDef) {
-            if (saw_any_loop_scope) {
+            if (node->data.continue_expr.name != nullptr) {
                 add_node_error(irb->codegen, node, buf_sprintf("labeled loop not found: '%s'", buf_ptr(node->data.continue_expr.name)));
                 return irb->codegen->invalid_instruction;
             } else {
@@ -6037,7 +6070,6 @@ static IrInstruction *ir_gen_continue(IrBuilder *irb, Scope *continue_scope, Ast
             return irb->codegen->invalid_instruction;
         } else if (search_scope->id == ScopeIdLoop) {
             ScopeLoop *this_loop_scope = (ScopeLoop *)search_scope;
-            saw_any_loop_scope = true;
             if (node->data.continue_expr.name == nullptr ||
                 (this_loop_scope->name != nullptr && buf_eql_buf(node->data.continue_expr.name, this_loop_scope->name)))
             {