From 373488157751dbaa22415ce842c4f5616fc845ff Mon Sep 17 00:00:00 2001 From: Vexu Date: Wed, 12 Aug 2020 23:35:11 +0300 Subject: [PATCH 1/3] add error for unused/duplicate block labels --- lib/std/dwarf.zig | 2 +- lib/std/special/c.zig | 6 ++-- lib/std/zig/render.zig | 2 +- src-self-hosted/translate_c.zig | 12 +++---- src/all_types.hpp | 3 ++ src/ir.cpp | 60 +++++++++++++++++++++++++++++++++ test/compile_errors.zig | 23 +++++++++++++ 7 files changed, 97 insertions(+), 11 deletions(-) diff --git a/lib/std/dwarf.zig b/lib/std/dwarf.zig index c5802240d4..085e394a8f 100644 --- a/lib/std/dwarf.zig +++ b/lib/std/dwarf.zig @@ -322,7 +322,7 @@ fn parseFormValue(allocator: *mem.Allocator, in_stream: anytype, form_id: u64, e FORM_block1 => parseFormValueBlock(allocator, in_stream, endian, 1), FORM_block2 => parseFormValueBlock(allocator, in_stream, endian, 2), FORM_block4 => parseFormValueBlock(allocator, in_stream, endian, 4), - FORM_block => x: { + FORM_block => { const block_len = try nosuspend leb.readULEB128(usize, in_stream); return parseFormValueBlockLen(allocator, in_stream, block_len); }, diff --git a/lib/std/special/c.zig b/lib/std/special/c.zig index c769bc358b..170bb98620 100644 --- a/lib/std/special/c.zig +++ b/lib/std/special/c.zig @@ -536,7 +536,7 @@ fn generic_fmod(comptime T: type, x: T, y: T) T { // normalize x and y if (ex == 0) { i = ux << exp_bits; - while (i >> bits_minus_1 == 0) : (b: { + while (i >> bits_minus_1 == 0) : ({ ex -= 1; i <<= 1; }) {} @@ -547,7 +547,7 @@ fn generic_fmod(comptime T: type, x: T, y: T) T { } if (ey == 0) { i = uy << exp_bits; - while (i >> bits_minus_1 == 0) : (b: { + while (i >> bits_minus_1 == 0) : ({ ey -= 1; i <<= 1; }) {} @@ -573,7 +573,7 @@ fn generic_fmod(comptime T: type, x: T, y: T) T { return 0 * x; ux = i; } - while (ux >> digits == 0) : (b: { + while (ux >> digits == 0) : ({ ux <<= 1; ex -= 1; }) {} diff --git a/lib/std/zig/render.zig b/lib/std/zig/render.zig index 454ddde160..c516250a17 100644 --- a/lib/std/zig/render.zig +++ b/lib/std/zig/render.zig @@ -2385,7 +2385,7 @@ fn renderTokenOffset( } } - if (next_token_id != .LineComment) blk: { + if (next_token_id != .LineComment) { switch (space) { Space.None, Space.NoNewline => return, Space.Newline => { diff --git a/src-self-hosted/translate_c.zig b/src-self-hosted/translate_c.zig index 5af9bbc3c4..8b1c6537f7 100644 --- a/src-self-hosted/translate_c.zig +++ b/src-self-hosted/translate_c.zig @@ -931,7 +931,7 @@ fn transRecordDecl(c: *Context, record_decl: *const ZigClangRecordDecl) Error!?* else => |e| return e, }; - const align_expr = blk: { + const align_expr = blk_2: { const alignment = ZigClangFieldDecl_getAlignedAttribute(field_decl, rp.c.clang_context); if (alignment != 0) { _ = try appendToken(rp.c, .Keyword_align, "align"); @@ -940,9 +940,9 @@ fn transRecordDecl(c: *Context, record_decl: *const ZigClangRecordDecl) Error!?* const expr = try transCreateNodeInt(rp.c, alignment / 8); _ = try appendToken(rp.c, .RParen, ")"); - break :blk expr; + break :blk_2 expr; } - break :blk null; + break :blk_2 null; }; const field_node = try c.arena.create(ast.Node.ContainerField); @@ -1073,9 +1073,9 @@ fn transEnumDecl(c: *Context, enum_decl: *const ZigClangEnumDecl) Error!?*ast.No const field_name_tok = try appendIdentifier(c, field_name); - const int_node = if (!pure_enum) blk: { + const int_node = if (!pure_enum) blk_2: { _ = try appendToken(c, .Colon, "="); - break :blk try transCreateNodeAPInt(c, ZigClangEnumConstantDecl_getInitVal(enum_const)); + break :blk_2 try transCreateNodeAPInt(c, ZigClangEnumConstantDecl_getInitVal(enum_const)); } else null; @@ -2388,7 +2388,7 @@ fn transZeroInitExpr( ty: *const ZigClangType, ) TransError!*ast.Node { switch (ZigClangType_getTypeClass(ty)) { - .Builtin => blk: { + .Builtin => { const builtin_ty = @ptrCast(*const ZigClangBuiltinType, ty); switch (ZigClangBuiltinType_getKind(builtin_ty)) { .Bool => return try transCreateNodeBoolLiteral(rp.c, false), diff --git a/src/all_types.hpp b/src/all_types.hpp index 3939a8ac25..5f142e3dcb 100644 --- a/src/all_types.hpp +++ b/src/all_types.hpp @@ -2438,6 +2438,7 @@ struct ScopeBlock { LVal lval; bool safety_off; bool fast_math_on; + bool name_used; }; // This scope is created from every defer expression. @@ -2488,6 +2489,8 @@ struct ScopeLoop { ZigList *incoming_blocks; ResultLocPeerParent *peer_parent; ScopeExpr *spill_scope; + + bool name_used; }; // This scope blocks certain things from working such as comptime continue diff --git a/src/ir.cpp b/src/ir.cpp index 8934a20545..40be4e147b 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -5476,6 +5476,25 @@ static ResultLocPeer *create_peer_result(ResultLocPeerParent *peer_parent) { return result; } +static bool is_duplicate_label(CodeGen *g, Scope *scope, AstNode *node, Buf *name) { + if (name == nullptr) return false; + + for (;;) { + if (scope == nullptr || scope->id == ScopeIdFnDef) { + break; + } else if (scope->id == ScopeIdBlock || scope->id == ScopeIdLoop) { + Buf *this_block_name = scope->id == ScopeIdBlock ? ((ScopeBlock *)scope)->name : ((ScopeLoop *)scope)->name; + if (this_block_name != nullptr && buf_eql_buf(name, this_block_name)) { + ErrorMsg *msg = add_node_error(g, node, buf_sprintf("redeclaration of label '%s'", buf_ptr(name))); + add_error_note(g, msg, scope->source_node, buf_sprintf("previous declaration is here")); + return true; + } + } + scope = scope->parent; + } + return false; +} + static IrInstSrc *ir_gen_block(IrBuilderSrc *irb, Scope *parent_scope, AstNode *block_node, LVal lval, ResultLoc *result_loc) { @@ -5484,6 +5503,9 @@ static IrInstSrc *ir_gen_block(IrBuilderSrc *irb, Scope *parent_scope, AstNode * ZigList incoming_values = {0}; ZigList incoming_blocks = {0}; + if (is_duplicate_label(irb->codegen, parent_scope, block_node, block_node->data.block.name)) + return irb->codegen->invalid_inst_src; + ScopeBlock *scope_block = create_block_scope(irb->codegen, block_node, parent_scope); Scope *outer_block_scope = &scope_block->base; @@ -5495,6 +5517,9 @@ static IrInstSrc *ir_gen_block(IrBuilderSrc *irb, Scope *parent_scope, AstNode * } if (block_node->data.block.statements.length == 0) { + if (scope_block->name != nullptr) { + add_node_error(irb->codegen, block_node, buf_sprintf("unused block label")); + } // {} return ir_lval_wrap(irb, parent_scope, ir_build_const_void(irb, child_scope, block_node), lval, result_loc); } @@ -5552,6 +5577,10 @@ static IrInstSrc *ir_gen_block(IrBuilderSrc *irb, Scope *parent_scope, AstNode * } } + if (scope_block->name != nullptr && scope_block->name_used == false) { + add_node_error(irb->codegen, block_node, buf_sprintf("unused block label")); + } + if (found_invalid_inst) return irb->codegen->invalid_inst_src; @@ -8152,6 +8181,9 @@ static IrInstSrc *ir_gen_while_expr(IrBuilderSrc *irb, Scope *scope, AstNode *no ZigList incoming_values = {0}; ZigList incoming_blocks = {0}; + if (is_duplicate_label(irb->codegen, payload_scope, node, node->data.while_expr.name)) + return irb->codegen->invalid_inst_src; + ScopeLoop *loop_scope = create_loop_scope(irb->codegen, node, payload_scope); loop_scope->break_block = end_block; loop_scope->continue_block = continue_block; @@ -8169,6 +8201,10 @@ static IrInstSrc *ir_gen_while_expr(IrBuilderSrc *irb, Scope *scope, AstNode *no if (body_result == irb->codegen->invalid_inst_src) return body_result; + if (loop_scope->name != nullptr && loop_scope->name_used == false) { + add_node_error(irb->codegen, node, buf_sprintf("unused while label")); + } + if (!instr_is_unreachable(body_result)) { ir_mark_gen(ir_build_check_statement_is_void(irb, payload_scope, node->data.while_expr.body, body_result)); ir_mark_gen(ir_build_br(irb, payload_scope, node, continue_block, is_comptime)); @@ -8263,6 +8299,9 @@ static IrInstSrc *ir_gen_while_expr(IrBuilderSrc *irb, Scope *scope, AstNode *no ZigList incoming_values = {0}; ZigList incoming_blocks = {0}; + if (is_duplicate_label(irb->codegen, child_scope, node, node->data.while_expr.name)) + return irb->codegen->invalid_inst_src; + ScopeLoop *loop_scope = create_loop_scope(irb->codegen, node, child_scope); loop_scope->break_block = end_block; loop_scope->continue_block = continue_block; @@ -8280,6 +8319,10 @@ static IrInstSrc *ir_gen_while_expr(IrBuilderSrc *irb, Scope *scope, AstNode *no if (body_result == irb->codegen->invalid_inst_src) return body_result; + if (loop_scope->name != nullptr && loop_scope->name_used == false) { + add_node_error(irb->codegen, node, buf_sprintf("unused while label")); + } + if (!instr_is_unreachable(body_result)) { ir_mark_gen(ir_build_check_statement_is_void(irb, child_scope, node->data.while_expr.body, body_result)); ir_mark_gen(ir_build_br(irb, child_scope, node, continue_block, is_comptime)); @@ -8353,6 +8396,9 @@ static IrInstSrc *ir_gen_while_expr(IrBuilderSrc *irb, Scope *scope, AstNode *no Scope *subexpr_scope = create_runtime_scope(irb->codegen, node, scope, is_comptime); + if (is_duplicate_label(irb->codegen, subexpr_scope, node, node->data.while_expr.name)) + return irb->codegen->invalid_inst_src; + ScopeLoop *loop_scope = create_loop_scope(irb->codegen, node, subexpr_scope); loop_scope->break_block = end_block; loop_scope->continue_block = continue_block; @@ -8369,6 +8415,10 @@ static IrInstSrc *ir_gen_while_expr(IrBuilderSrc *irb, Scope *scope, AstNode *no if (body_result == irb->codegen->invalid_inst_src) return body_result; + if (loop_scope->name != nullptr && loop_scope->name_used == false) { + add_node_error(irb->codegen, node, buf_sprintf("unused while label")); + } + if (!instr_is_unreachable(body_result)) { ir_mark_gen(ir_build_check_statement_is_void(irb, scope, node->data.while_expr.body, body_result)); ir_mark_gen(ir_build_br(irb, scope, node, continue_block, is_comptime)); @@ -8501,6 +8551,9 @@ static IrInstSrc *ir_gen_for_expr(IrBuilderSrc *irb, Scope *parent_scope, AstNod elem_ptr : ir_build_load_ptr(irb, &spill_scope->base, elem_node, elem_ptr); build_decl_var_and_init(irb, parent_scope, elem_node, elem_var, elem_value, buf_ptr(elem_var_name), is_comptime); + if (is_duplicate_label(irb->codegen, child_scope, node, node->data.for_expr.name)) + return irb->codegen->invalid_inst_src; + ZigList incoming_values = {0}; ZigList incoming_blocks = {0}; ScopeLoop *loop_scope = create_loop_scope(irb->codegen, node, child_scope); @@ -8520,6 +8573,10 @@ static IrInstSrc *ir_gen_for_expr(IrBuilderSrc *irb, Scope *parent_scope, AstNod if (body_result == irb->codegen->invalid_inst_src) return irb->codegen->invalid_inst_src; + if (loop_scope->name != nullptr && loop_scope->name_used == false) { + add_node_error(irb->codegen, node, buf_sprintf("unused for label")); + } + if (!instr_is_unreachable(body_result)) { ir_mark_gen(ir_build_check_statement_is_void(irb, child_scope, node->data.for_expr.body, body_result)); ir_mark_gen(ir_build_br(irb, child_scope, node, continue_block, is_comptime)); @@ -9464,6 +9521,7 @@ static IrInstSrc *ir_gen_break(IrBuilderSrc *irb, Scope *break_scope, AstNode *n if (node->data.break_expr.name == nullptr || (this_loop_scope->name != nullptr && buf_eql_buf(node->data.break_expr.name, this_loop_scope->name))) { + this_loop_scope->name_used = true; loop_scope = this_loop_scope; break; } @@ -9473,6 +9531,7 @@ static IrInstSrc *ir_gen_break(IrBuilderSrc *irb, Scope *break_scope, AstNode *n (this_block_scope->name != nullptr && buf_eql_buf(node->data.break_expr.name, this_block_scope->name))) { assert(this_block_scope->end_block != nullptr); + this_block_scope->name_used = true; return ir_gen_return_from_block(irb, break_scope, node, this_block_scope); } } else if (search_scope->id == ScopeIdSuspend) { @@ -9540,6 +9599,7 @@ static IrInstSrc *ir_gen_continue(IrBuilderSrc *irb, Scope *continue_scope, AstN if (node->data.continue_expr.name == nullptr || (this_loop_scope->name != nullptr && buf_eql_buf(node->data.continue_expr.name, this_loop_scope->name))) { + this_loop_scope->name_used = true; loop_scope = this_loop_scope; break; } diff --git a/test/compile_errors.zig b/test/compile_errors.zig index 6b231a323d..4adc538602 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -2,6 +2,29 @@ const tests = @import("tests.zig"); const std = @import("std"); pub fn addCases(cases: *tests.CompileErrorContext) void { + cases.addTest("duplicate/unused labels", + \\comptime { + \\ blk: { blk: while (false) {} } + \\ blk: while (false) { blk: for (@as([0]void, undefined)) |_| {} } + \\ blk: for (@as([0]void, undefined)) |_| { blk: {} } + \\} + \\comptime { + \\ blk: {} + \\ blk: while(false) {} + \\ blk: for(@as([0]void, undefined)) |_| {} + \\} + , &[_][]const u8{ + "tmp.zig:2:17: error: redeclaration of label 'blk'", + "tmp.zig:2:10: note: previous declaration is here", + "tmp.zig:3:31: error: redeclaration of label 'blk'", + "tmp.zig:3:10: note: previous declaration is here", + "tmp.zig:4:51: error: redeclaration of label 'blk'", + "tmp.zig:4:10: note: previous declaration is here", + "tmp.zig:7:10: error: unused block label", + "tmp.zig:8:10: error: unused while label", + "tmp.zig:9:10: error: unused for label", + }); + cases.addTest("@alignCast of zero sized types", \\export fn foo() void { \\ const a: *void = undefined; From c5368ba20c79b62d658c71033df9aeb7aa6e4581 Mon Sep 17 00:00:00 2001 From: Vexu Date: Thu, 13 Aug 2020 15:27:29 +0300 Subject: [PATCH 2/3] translate-c: ensure generated labels are unique --- src-self-hosted/translate_c.zig | 124 +++++++++++++++++--------------- test/translate_c.zig | 26 +++---- 2 files changed, 81 insertions(+), 69 deletions(-) diff --git a/src-self-hosted/translate_c.zig b/src-self-hosted/translate_c.zig index 8b1c6537f7..de8f633076 100644 --- a/src-self-hosted/translate_c.zig +++ b/src-self-hosted/translate_c.zig @@ -61,7 +61,8 @@ const Scope = struct { pending_block: Block, cases: []*ast.Node, case_index: usize, - has_default: bool = false, + switch_label: ?[]const u8, + default_label: ?[]const u8, }; /// Used for the scope of condition expressions, for example `if (cond)`. @@ -73,7 +74,7 @@ const Scope = struct { fn getBlockScope(self: *Condition, c: *Context) !*Block { if (self.block) |*b| return b; - self.block = try Block.init(c, &self.base, "blk"); + self.block = try Block.init(c, &self.base, true); return &self.block.?; } @@ -93,21 +94,22 @@ const Scope = struct { mangle_count: u32 = 0, lbrace: ast.TokenIndex, - fn init(c: *Context, parent: *Scope, label: ?[]const u8) !Block { - return Block{ + fn init(c: *Context, parent: *Scope, labeled: bool) !Block { + var blk = Block{ .base = .{ .id = .Block, .parent = parent, }, .statements = std.ArrayList(*ast.Node).init(c.gpa), .variables = AliasList.init(c.gpa), - .label = if (label) |l| blk: { - const ll = try appendIdentifier(c, l); - _ = try appendToken(c, .Colon, ":"); - break :blk ll; - } else null, + .label = null, .lbrace = try appendToken(c, .LBrace, "{"), }; + if (labeled) { + blk.label = try appendIdentifier(c, try blk.makeMangledName(c, "blk")); + _ = try appendToken(c, .Colon, ":"); + } + return blk; } fn deinit(self: *Block) void { @@ -577,7 +579,7 @@ fn visitFnDecl(c: *Context, fn_decl: *const ZigClangFunctionDecl) Error!void { // actual function definition with body const body_stmt = ZigClangFunctionDecl_getBody(fn_decl); - var block_scope = try Scope.Block.init(rp.c, &c.global_scope.base, null); + var block_scope = try Scope.Block.init(rp.c, &c.global_scope.base, false); defer block_scope.deinit(); var scope = &block_scope.base; @@ -1307,7 +1309,7 @@ fn transBinaryOperator( const rhs = try transExpr(rp, &block_scope.base, ZigClangBinaryOperator_getRHS(stmt), .used, .r_value); if (expr) { _ = try appendToken(rp.c, .Semicolon, ";"); - const break_node = try transCreateNodeBreakToken(rp.c, block_scope.label, rhs); + const break_node = try transCreateNodeBreak(rp.c, block_scope.label, rhs); try block_scope.statements.append(&break_node.base); const block_node = try block_scope.complete(rp.c); const rparen = try appendToken(rp.c, .RParen, ")"); @@ -1476,7 +1478,7 @@ fn transCompoundStmtInline( } fn transCompoundStmt(rp: RestorePoint, scope: *Scope, stmt: *const ZigClangCompoundStmt) TransError!*ast.Node { - var block_scope = try Scope.Block.init(rp.c, scope, null); + var block_scope = try Scope.Block.init(rp.c, scope, false); defer block_scope.deinit(); try transCompoundStmtInline(rp, &block_scope.base, stmt, &block_scope); const node = try block_scope.complete(rp.c); @@ -2587,7 +2589,7 @@ fn transForLoop( defer if (block_scope) |*bs| bs.deinit(); if (ZigClangForStmt_getInit(stmt)) |init| { - block_scope = try Scope.Block.init(rp.c, scope, null); + block_scope = try Scope.Block.init(rp.c, scope, false); loop_scope.parent = &block_scope.?.base; const init_node = try transStmt(rp, &block_scope.?.base, init, .unused, .r_value); try block_scope.?.statements.append(init_node); @@ -2673,17 +2675,19 @@ fn transSwitch( .cases = switch_node.cases(), .case_index = 0, .pending_block = undefined, + .default_label = null, + .switch_label = null, }; // tmp block that all statements will go before being picked up by a case or default - var block_scope = try Scope.Block.init(rp.c, &switch_scope.base, null); + var block_scope = try Scope.Block.init(rp.c, &switch_scope.base, false); defer block_scope.deinit(); // Note that we do not defer a deinit here; the switch_scope.pending_block field // has its own memory management. This resource is freed inside `transCase` and // then the final pending_block is freed at the bottom of this function with // pending_block.deinit(). - switch_scope.pending_block = try Scope.Block.init(rp.c, scope, null); + switch_scope.pending_block = try Scope.Block.init(rp.c, scope, false); try switch_scope.pending_block.statements.append(&switch_node.base); const last = try transStmt(rp, &block_scope.base, ZigClangSwitchStmt_getBody(stmt), .unused, .r_value); @@ -2698,11 +2702,19 @@ fn transSwitch( switch_scope.pending_block.statements.appendAssumeCapacity(n); } - switch_scope.pending_block.label = try appendIdentifier(rp.c, "__switch"); - _ = try appendToken(rp.c, .Colon, ":"); - if (!switch_scope.has_default) { + if (switch_scope.default_label == null) { + switch_scope.switch_label = try block_scope.makeMangledName(rp.c, "switch"); + } + if (switch_scope.switch_label) |l| { + switch_scope.pending_block.label = try appendIdentifier(rp.c, l); + _ = try appendToken(rp.c, .Colon, ":"); + } + if (switch_scope.default_label == null) { const else_prong = try transCreateNodeSwitchCase(rp.c, try transCreateNodeSwitchElse(rp.c)); - else_prong.expr = &(try transCreateNodeBreak(rp.c, "__switch", null)).base; + else_prong.expr = blk: { + var br = try CtrlFlow.init(rp.c, .Break, switch_scope.switch_label.?); + break :blk &(try br.finish(null)).base; + }; _ = try appendToken(rp.c, .Comma, ","); if (switch_scope.case_index >= switch_scope.cases.len) @@ -2726,7 +2738,7 @@ fn transCase( ) TransError!*ast.Node { const block_scope = scope.findBlockScope(rp.c) catch unreachable; const switch_scope = scope.getSwitch(); - const label = try std.fmt.allocPrint(rp.c.arena, "__case_{}", .{switch_scope.case_index - @boolToInt(switch_scope.has_default)}); + const label = try block_scope.makeMangledName(rp.c, "case"); _ = try appendToken(rp.c, .Semicolon, ";"); const expr = if (ZigClangCaseStmt_getRHS(stmt)) |rhs| blk: { @@ -2746,7 +2758,10 @@ fn transCase( try transExpr(rp, scope, ZigClangCaseStmt_getLHS(stmt), .used, .r_value); const switch_prong = try transCreateNodeSwitchCase(rp.c, expr); - switch_prong.expr = &(try transCreateNodeBreak(rp.c, label, null)).base; + switch_prong.expr = blk: { + var br = try CtrlFlow.init(rp.c, .Break, label); + break :blk &(try br.finish(null)).base; + }; _ = try appendToken(rp.c, .Comma, ","); if (switch_scope.case_index >= switch_scope.cases.len) @@ -2763,7 +2778,7 @@ fn transCase( const pending_node = try switch_scope.pending_block.complete(rp.c); switch_scope.pending_block.deinit(); - switch_scope.pending_block = try Scope.Block.init(rp.c, scope, null); + switch_scope.pending_block = try Scope.Block.init(rp.c, scope, false); try switch_scope.pending_block.statements.append(&pending_node.base); @@ -2777,12 +2792,14 @@ fn transDefault( ) TransError!*ast.Node { const block_scope = scope.findBlockScope(rp.c) catch unreachable; const switch_scope = scope.getSwitch(); - const label = "__default"; - switch_scope.has_default = true; + switch_scope.default_label = try block_scope.makeMangledName(rp.c, "default"); _ = try appendToken(rp.c, .Semicolon, ";"); const else_prong = try transCreateNodeSwitchCase(rp.c, try transCreateNodeSwitchElse(rp.c)); - else_prong.expr = &(try transCreateNodeBreak(rp.c, label, null)).base; + else_prong.expr = blk: { + var br = try CtrlFlow.init(rp.c, .Break, switch_scope.default_label.?); + break :blk &(try br.finish(null)).base; + }; _ = try appendToken(rp.c, .Comma, ","); if (switch_scope.case_index >= switch_scope.cases.len) @@ -2790,7 +2807,7 @@ fn transDefault( switch_scope.cases[switch_scope.case_index] = &else_prong.base; switch_scope.case_index += 1; - switch_scope.pending_block.label = try appendIdentifier(rp.c, label); + switch_scope.pending_block.label = try appendIdentifier(rp.c, switch_scope.default_label.?); _ = try appendToken(rp.c, .Colon, ":"); // take all pending statements @@ -2799,7 +2816,7 @@ fn transDefault( const pending_node = try switch_scope.pending_block.complete(rp.c); switch_scope.pending_block.deinit(); - switch_scope.pending_block = try Scope.Block.init(rp.c, scope, null); + switch_scope.pending_block = try Scope.Block.init(rp.c, scope, false); try switch_scope.pending_block.statements.append(&pending_node.base); return transStmt(rp, scope, ZigClangDefaultStmt_getSubStmt(stmt), .unused, .r_value); @@ -2894,7 +2911,7 @@ fn transStmtExpr(rp: RestorePoint, scope: *Scope, stmt: *const ZigClangStmtExpr, return transCompoundStmt(rp, scope, comp); } const lparen = try appendToken(rp.c, .LParen, "("); - var block_scope = try Scope.Block.init(rp.c, scope, "blk"); + var block_scope = try Scope.Block.init(rp.c, scope, true); defer block_scope.deinit(); var it = ZigClangCompoundStmt_body_begin(comp); @@ -3209,7 +3226,7 @@ fn transCreatePreCrement( // zig: _ref.* += 1; // zig: break :blk _ref.* // zig: }) - var block_scope = try Scope.Block.init(rp.c, scope, "blk"); + var block_scope = try Scope.Block.init(rp.c, scope, true); defer block_scope.deinit(); const ref = try block_scope.makeMangledName(rp.c, "ref"); @@ -3239,7 +3256,7 @@ fn transCreatePreCrement( const assign = try transCreateNodeInfixOp(rp, scope, ref_node, op, token, one, .used, false); try block_scope.statements.append(assign); - const break_node = try transCreateNodeBreakToken(rp.c, block_scope.label, ref_node); + const break_node = try transCreateNodeBreak(rp.c, block_scope.label, ref_node); try block_scope.statements.append(&break_node.base); const block_node = try block_scope.complete(rp.c); // semicolon must immediately follow rbrace because it is the last token in a block @@ -3283,7 +3300,7 @@ fn transCreatePostCrement( // zig: _ref.* += 1; // zig: break :blk _tmp // zig: }) - var block_scope = try Scope.Block.init(rp.c, scope, "blk"); + var block_scope = try Scope.Block.init(rp.c, scope, true); defer block_scope.deinit(); const ref = try block_scope.makeMangledName(rp.c, "ref"); @@ -3458,7 +3475,7 @@ fn transCreateCompoundAssign( // zig: _ref.* = _ref.* + rhs; // zig: break :blk _ref.* // zig: }) - var block_scope = try Scope.Block.init(rp.c, scope, "blk"); + var block_scope = try Scope.Block.init(rp.c, scope, true); defer block_scope.deinit(); const ref = try block_scope.makeMangledName(rp.c, "ref"); @@ -3526,7 +3543,7 @@ fn transCreateCompoundAssign( try block_scope.statements.append(assign); } - const break_node = try transCreateNodeBreakToken(rp.c, block_scope.label, ref_node); + const break_node = try transCreateNodeBreak(rp.c, block_scope.label, ref_node); try block_scope.statements.append(&break_node.base); const block_node = try block_scope.complete(rp.c); const grouped_expr = try rp.c.arena.create(ast.Node.GroupedExpression); @@ -3602,8 +3619,16 @@ fn transCPtrCast( fn transBreak(rp: RestorePoint, scope: *Scope) TransError!*ast.Node { const break_scope = scope.getBreakableScope(); - const label_text: ?[]const u8 = if (break_scope.id == .Switch) "__switch" else null; - const br = try transCreateNodeBreak(rp.c, label_text, null); + const label_text: ?[]const u8 = if (break_scope.id == .Switch) blk: { + const swtch = @fieldParentPtr(Scope.Switch, "base", break_scope); + const block_scope = try scope.findBlockScope(rp.c); + swtch.switch_label = try block_scope.makeMangledName(rp.c, "switch"); + break :blk swtch.switch_label; + } else + null; + + var cf = try CtrlFlow.init(rp.c, .Break, label_text); + const br = try cf.finish(null); _ = try appendToken(rp.c, .Semicolon, ";"); return &br.base; } @@ -3634,7 +3659,7 @@ fn transBinaryConditionalOperator(rp: RestorePoint, scope: *Scope, stmt: *const // }) const lparen = try appendToken(rp.c, .LParen, "("); - var block_scope = try Scope.Block.init(rp.c, scope, "blk"); + var block_scope = try Scope.Block.init(rp.c, scope, true); defer block_scope.deinit(); const mangled_name = try block_scope.makeMangledName(rp.c, "cond_temp"); @@ -4082,8 +4107,7 @@ fn transCreateNodeAssign( // zig: lhs = _tmp; // zig: break :blk _tmp // zig: }) - const label_name = "blk"; - var block_scope = try Scope.Block.init(rp.c, scope, label_name); + var block_scope = try Scope.Block.init(rp.c, scope, true); defer block_scope.deinit(); const tmp = try block_scope.makeMangledName(rp.c, "tmp"); @@ -4118,7 +4142,7 @@ fn transCreateNodeAssign( try block_scope.statements.append(assign); const break_node = blk: { - var tmp_ctrl_flow = try CtrlFlow.init(rp.c, .Break, label_name); + var tmp_ctrl_flow = try CtrlFlow.init(rp.c, .Break, tokenSlice(rp.c, block_scope.label.?)); const rhs_expr = try transCreateNodeIdentifier(rp.c, tmp); break :blk try tmp_ctrl_flow.finish(rhs_expr); }; @@ -4495,23 +4519,12 @@ fn transCreateNodeElse(c: *Context) !*ast.Node.Else { return node; } -fn transCreateNodeBreakToken( +fn transCreateNodeBreak( c: *Context, label: ?ast.TokenIndex, rhs: ?*ast.Node, ) !*ast.Node.ControlFlowExpression { - const other_token = label orelse return transCreateNodeBreak(c, null, rhs); - const loc = c.token_locs.items[other_token]; - const label_name = c.source_buffer.items[loc.start..loc.end]; - return transCreateNodeBreak(c, label_name, rhs); -} - -fn transCreateNodeBreak( - c: *Context, - label: ?[]const u8, - rhs: ?*ast.Node, -) !*ast.Node.ControlFlowExpression { - var ctrl_flow = try CtrlFlow.init(c, .Break, label); + var ctrl_flow = try CtrlFlow.init(c, .Break, if (label) |l| tokenSlice(c, l) else null); return ctrl_flow.finish(rhs); } @@ -5362,7 +5375,7 @@ fn transMacroDefine(c: *Context, m: *MacroCtx) ParseError!void { } fn transMacroFnDefine(c: *Context, m: *MacroCtx) ParseError!void { - var block_scope = try Scope.Block.init(c, &c.global_scope.base, null); + var block_scope = try Scope.Block.init(c, &c.global_scope.base, false); defer block_scope.deinit(); const scope = &block_scope.base; @@ -5475,8 +5488,7 @@ fn parseCExpr(c: *Context, m: *MacroCtx, scope: *Scope) ParseError!*ast.Node { }, .Comma => { _ = try appendToken(c, .Semicolon, ";"); - const label_name = "blk"; - var block_scope = try Scope.Block.init(c, scope, label_name); + var block_scope = try Scope.Block.init(c, scope, true); defer block_scope.deinit(); var last = node; @@ -5501,7 +5513,7 @@ fn parseCExpr(c: *Context, m: *MacroCtx, scope: *Scope) ParseError!*ast.Node { } } - const break_node = try transCreateNodeBreak(c, label_name, last); + const break_node = try transCreateNodeBreak(c, block_scope.label, last); try block_scope.statements.append(&break_node.base); const block_node = try block_scope.complete(c); return &block_node.base; diff --git a/test/translate_c.zig b/test/translate_c.zig index 38faffe747..c632700bc5 100644 --- a/test/translate_c.zig +++ b/test/translate_c.zig @@ -1730,16 +1730,16 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\pub export fn switch_fn(arg_i: c_int) c_int { \\ var i = arg_i; \\ var res: c_int = 0; - \\ __switch: { - \\ __case_2: { - \\ __default: { - \\ __case_1: { - \\ __case_0: { + \\ @"switch": { + \\ case_2: { + \\ default: { + \\ case_1: { + \\ case: { \\ switch (i) { - \\ @as(c_int, 0) => break :__case_0, - \\ @as(c_int, 1)...@as(c_int, 3) => break :__case_1, - \\ else => break :__default, - \\ @as(c_int, 4) => break :__case_2, + \\ @as(c_int, 0) => break :case, + \\ @as(c_int, 1)...@as(c_int, 3) => break :case_1, + \\ else => break :default, + \\ @as(c_int, 4) => break :case_2, \\ } \\ } \\ res = 1; @@ -1747,7 +1747,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ res = 2; \\ } \\ res = (@as(c_int, 3) * i); - \\ break :__switch; + \\ break :@"switch"; \\ } \\ res = 5; \\ } @@ -2782,11 +2782,11 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ var x = arg_x; \\ return blk: { \\ const tmp = x; - \\ (blk: { + \\ (blk_1: { \\ const ref = &p; - \\ const tmp_1 = ref.*; + \\ const tmp_2 = ref.*; \\ ref.* += 1; - \\ break :blk tmp_1; + \\ break :blk_1 tmp_2; \\ }).?.* = tmp; \\ break :blk tmp; \\ }; From 13e472aa2a8113df6417c09727297a6106127f8e Mon Sep 17 00:00:00 2001 From: Vexu Date: Thu, 13 Aug 2020 16:06:42 +0300 Subject: [PATCH 3/3] translate-c: add return if one is needed --- lib/std/hash/auto_hash.zig | 2 +- src-self-hosted/Module.zig | 4 +-- src-self-hosted/translate_c.zig | 40 +++++++++++++++++++++ test/run_translated_c.zig | 1 - test/translate_c.zig | 61 +++++++++++++++++++++++---------- 5 files changed, 86 insertions(+), 22 deletions(-) diff --git a/lib/std/hash/auto_hash.zig b/lib/std/hash/auto_hash.zig index 85f8e4b0d2..996d6ede38 100644 --- a/lib/std/hash/auto_hash.zig +++ b/lib/std/hash/auto_hash.zig @@ -129,7 +129,7 @@ pub fn hash(hasher: anytype, key: anytype, comptime strat: HashStrategy) void { } }, - .Union => |info| blk: { + .Union => |info| { if (info.tag_type) |tag_type| { const tag = meta.activeTag(key); const s = hash(hasher, tag, strat); diff --git a/src-self-hosted/Module.zig b/src-self-hosted/Module.zig index 6abd4f51e1..88e59b5a2f 100644 --- a/src-self-hosted/Module.zig +++ b/src-self-hosted/Module.zig @@ -2798,7 +2798,7 @@ pub fn floatAdd(self: *Module, scope: *Scope, float_type: Type, src: usize, lhs: val_payload.* = .{ .val = lhs_val + rhs_val }; break :blk &val_payload.base; }, - 128 => blk: { + 128 => { return self.fail(scope, src, "TODO Implement addition for big floats", .{}); }, else => unreachable, @@ -2832,7 +2832,7 @@ pub fn floatSub(self: *Module, scope: *Scope, float_type: Type, src: usize, lhs: val_payload.* = .{ .val = lhs_val - rhs_val }; break :blk &val_payload.base; }, - 128 => blk: { + 128 => { return self.fail(scope, src, "TODO Implement substraction for big floats", .{}); }, else => unreachable, diff --git a/src-self-hosted/translate_c.zig b/src-self-hosted/translate_c.zig index de8f633076..2382375fc5 100644 --- a/src-self-hosted/translate_c.zig +++ b/src-self-hosted/translate_c.zig @@ -628,6 +628,46 @@ fn visitFnDecl(c: *Context, fn_decl: *const ZigClangFunctionDecl) Error!void { error.UnsupportedType, => return failDecl(c, fn_decl_loc, fn_name, "unable to translate function", .{}), }; + // add return statement if the function didn't have one + blk: { + const fn_ty = @ptrCast(*const ZigClangFunctionType, fn_type); + + if (ZigClangFunctionType_getNoReturnAttr(fn_ty)) break :blk; + const return_qt = ZigClangFunctionType_getReturnType(fn_ty); + if (isCVoid(return_qt)) break :blk; + + if (block_scope.statements.items.len > 0) { + var last = block_scope.statements.items[block_scope.statements.items.len - 1]; + while (true) { + switch (last.tag) { + .Block => { + const stmts = last.castTag(.Block).?.statements(); + if (stmts.len == 0) break; + + last = stmts[stmts.len - 1]; + }, + // no extra return needed + .Return => break :blk, + else => break, + } + } + } + + const return_expr = try ast.Node.ControlFlowExpression.create(rp.c.arena, .{ + .ltoken = try appendToken(rp.c, .Keyword_return, "return"), + .tag = .Return, + }, .{ + .rhs = transZeroInitExpr(rp, scope, fn_decl_loc, ZigClangQualType_getTypePtr(return_qt)) catch |err| switch (err) { + error.OutOfMemory => |e| return e, + error.UnsupportedTranslation, + error.UnsupportedType, + => return failDecl(c, fn_decl_loc, fn_name, "unable to create a return value for function", .{}), + }, + }); + _ = try appendToken(rp.c, .Semicolon, ";"); + try block_scope.statements.append(&return_expr.base); + } + const body_node = try block_scope.complete(rp.c); proto_node.setTrailer("body_node", &body_node.base); return addTopLevelDecl(c, fn_name, &proto_node.base); diff --git a/test/run_translated_c.zig b/test/run_translated_c.zig index efdc9702a4..3fa183ce3b 100644 --- a/test/run_translated_c.zig +++ b/test/run_translated_c.zig @@ -15,7 +15,6 @@ pub fn addCases(cases: *tests.RunTranslatedCContext) void { \\ } \\ if (s0 != 1) abort(); \\ if (s1 != 10) abort(); - \\ return 0; \\} , ""); diff --git a/test/translate_c.zig b/test/translate_c.zig index c632700bc5..f7e983276e 100644 --- a/test/translate_c.zig +++ b/test/translate_c.zig @@ -3,12 +3,33 @@ const std = @import("std"); const CrossTarget = std.zig.CrossTarget; pub fn addCases(cases: *tests.TranslateCContext) void { + cases.add("missing return stmt", + \\int foo() {} + \\int bar() { + \\ int a = 2; + \\} + \\int baz() { + \\ return 0; + \\} + , &[_][]const u8{ + \\pub export fn foo() c_int { + \\ return 0; + \\} + \\pub export fn bar() c_int { + \\ var a: c_int = 2; + \\ return 0; + \\} + \\pub export fn baz() c_int { + \\ return 0; + \\} + }); + cases.add("alignof", - \\int main() { + \\void main() { \\ int a = _Alignof(int); \\} , &[_][]const u8{ - \\pub export fn main() c_int { + \\pub export fn main() void { \\ var a: c_int = @bitCast(c_int, @truncate(c_uint, @alignOf(c_int))); \\} }); @@ -539,6 +560,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ c = (a * b); \\ c = @divTrunc(a, b); \\ c = @rem(a, b); + \\ return 0; \\} \\pub export fn u() c_uint { \\ var a: c_uint = undefined; @@ -549,6 +571,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ c = (a *% b); \\ c = (a / b); \\ c = (a % b); + \\ return 0; \\} }); @@ -1596,13 +1619,13 @@ pub fn addCases(cases: *tests.TranslateCContext) void { }); cases.add("worst-case assign", - \\int foo() { + \\void foo() { \\ int a; \\ int b; \\ a = b = 2; \\} , &[_][]const u8{ - \\pub export fn foo() c_int { + \\pub export fn foo() void { \\ var a: c_int = undefined; \\ var b: c_int = undefined; \\ a = blk: { @@ -1650,11 +1673,12 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ a = 7; \\ if (!true) break; \\ } + \\ return 0; \\} }); cases.add("for loops", - \\int foo() { + \\void foo() { \\ for (int i = 2, b = 4; i + 2; i = 2) { \\ int a = 2; \\ a = 6, 5, 7; @@ -1662,7 +1686,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ char i = 2; \\} , &[_][]const u8{ - \\pub export fn foo() c_int { + \\pub export fn foo() void { \\ { \\ var i: c_int = 2; \\ var b: c_int = 4; @@ -1712,7 +1736,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { }); cases.add("switch on int", - \\int switch_fn(int i) { + \\void switch_fn(int i) { \\ int res = 0; \\ switch (i) { \\ case 0: @@ -1727,7 +1751,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ } \\} , &[_][]const u8{ - \\pub export fn switch_fn(arg_i: c_int) c_int { + \\pub export fn switch_fn(arg_i: c_int) void { \\ var i = arg_i; \\ var res: c_int = 0; \\ @"switch": { @@ -1787,13 +1811,13 @@ pub fn addCases(cases: *tests.TranslateCContext) void { }); cases.add("assign", - \\int max(int a) { + \\void max(int a) { \\ int tmp; \\ tmp = a; \\ a = tmp; \\} , &[_][]const u8{ - \\pub export fn max(arg_a: c_int) c_int { + \\pub export fn max(arg_a: c_int) void { \\ var a = arg_a; \\ var tmp: c_int = undefined; \\ tmp = a; @@ -2082,7 +2106,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ int b; \\}a; \\float b = 2.0f; - \\int foo(void) { + \\void foo(void) { \\ struct Foo *c; \\ a.b; \\ c->b; @@ -2093,7 +2117,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\}; \\pub extern var a: struct_Foo; \\pub export var b: f32 = 2; - \\pub export fn foo() c_int { + \\pub export fn foo() void { \\ var c: [*c]struct_Foo = undefined; \\ _ = a.b; \\ _ = c.*.b; @@ -2204,11 +2228,12 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ if (a < b) return b; \\ if (a < b) return b else return a; \\ if (a < b) {} else {} + \\ return 0; \\} }); cases.add("if statements", - \\int foo() { + \\void foo() { \\ if (2) { \\ int a = 2; \\ } @@ -2217,7 +2242,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ } \\} , &[_][]const u8{ - \\pub export fn foo() c_int { + \\pub export fn foo() void { \\ if (true) { \\ var a: c_int = 2; \\ } @@ -2811,12 +2836,12 @@ pub fn addCases(cases: *tests.TranslateCContext) void { }); cases.add("arg name aliasing decl which comes after", - \\int foo(int bar) { + \\void foo(int bar) { \\ bar = 2; \\} \\int bar = 4; , &[_][]const u8{ - \\pub export fn foo(arg_bar_1: c_int) c_int { + \\pub export fn foo(arg_bar_1: c_int) void { \\ var bar_1 = arg_bar_1; \\ bar_1 = 2; \\} @@ -2824,12 +2849,12 @@ pub fn addCases(cases: *tests.TranslateCContext) void { }); cases.add("arg name aliasing macro which comes after", - \\int foo(int bar) { + \\void foo(int bar) { \\ bar = 2; \\} \\#define bar 4 , &[_][]const u8{ - \\pub export fn foo(arg_bar_1: c_int) c_int { + \\pub export fn foo(arg_bar_1: c_int) void { \\ var bar_1 = arg_bar_1; \\ bar_1 = 2; \\}