From d1d24b426dd8f12e6d643f45fcb6bb11dddaa8ef Mon Sep 17 00:00:00 2001 From: Veikka Tuominen Date: Tue, 2 Aug 2022 20:44:14 +0300 Subject: [PATCH] AstGen: check loop bodies and else branches for unused result --- src/AstGen.zig | 67 +++++++++++++++---- .../for_loop_body_expression_ignored.zig | 35 ++++++++++ .../obj/for_loop_body_expression_ignored.zig | 18 ----- .../while_loop_body_expression_ignored.zig | 22 ------ .../while_loop_body_expression_ignored.zig | 43 ++++++++++++ 5 files changed, 133 insertions(+), 52 deletions(-) create mode 100644 test/cases/compile_errors/for_loop_body_expression_ignored.zig delete mode 100644 test/cases/compile_errors/stage1/obj/for_loop_body_expression_ignored.zig delete mode 100644 test/cases/compile_errors/stage1/obj/while_loop_body_expression_ignored.zig create mode 100644 test/cases/compile_errors/while_loop_body_expression_ignored.zig diff --git a/src/AstGen.zig b/src/AstGen.zig index 051f1dace8..3850fe84a7 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -768,12 +768,12 @@ fn expr(gz: *GenZir, scope: *Scope, rl: ResultLoc, node: Ast.Node.Index) InnerEr .if_simple => return ifExpr(gz, scope, rl.br(), node, tree.ifSimple(node)), .@"if" => return ifExpr(gz, scope, rl.br(), node, tree.ifFull(node)), - .while_simple => return whileExpr(gz, scope, rl.br(), node, tree.whileSimple(node)), - .while_cont => return whileExpr(gz, scope, rl.br(), node, tree.whileCont(node)), - .@"while" => return whileExpr(gz, scope, rl.br(), node, tree.whileFull(node)), + .while_simple => return whileExpr(gz, scope, rl.br(), node, tree.whileSimple(node), false), + .while_cont => return whileExpr(gz, scope, rl.br(), node, tree.whileCont(node), false), + .@"while" => return whileExpr(gz, scope, rl.br(), node, tree.whileFull(node), false), - .for_simple => return forExpr(gz, scope, rl.br(), node, tree.forSimple(node)), - .@"for" => return forExpr(gz, scope, rl.br(), node, tree.forFull(node)), + .for_simple => return forExpr(gz, scope, rl.br(), node, tree.forSimple(node), false), + .@"for" => return forExpr(gz, scope, rl.br(), node, tree.forFull(node), false), .slice_open => { const lhs = try expr(gz, scope, .ref, node_datas[node].lhs); @@ -2152,6 +2152,7 @@ fn blockExprStmts(gz: *GenZir, parent_scope: *Scope, statements: []const Ast.Nod const astgen = gz.astgen; const tree = astgen.tree; const node_tags = tree.nodes.items(.tag); + const node_data = tree.nodes.items(.data); if (statements.len == 0) return; @@ -2178,8 +2179,10 @@ fn blockExprStmts(gz: *GenZir, parent_scope: *Scope, statements: []const Ast.Nod }, ); } - switch (node_tags[statement]) { - // zig fmt: off + var inner_node = statement; + while (true) { + switch (node_tags[inner_node]) { + // zig fmt: off .global_var_decl => scope = try varDecl(gz, scope, statement, block_arena_allocator, tree.globalVarDecl(statement)), .local_var_decl => scope = try varDecl(gz, scope, statement, block_arena_allocator, tree.localVarDecl(statement)), .simple_var_decl => scope = try varDecl(gz, scope, statement, block_arena_allocator, tree.simpleVarDecl(statement)), @@ -2204,9 +2207,23 @@ fn blockExprStmts(gz: *GenZir, parent_scope: *Scope, statements: []const Ast.Nod .assign_add_wrap => try assignOp(gz, scope, statement, .addwrap), .assign_mul => try assignOp(gz, scope, statement, .mul), .assign_mul_wrap => try assignOp(gz, scope, statement, .mulwrap), + + .grouped_expression => { + inner_node = node_data[statement].lhs; + continue; + }, - else => noreturn_src_node = try unusedResultExpr(gz, scope, statement), + .while_simple => _ = try whileExpr(gz, scope, .discard, inner_node, tree.whileSimple(inner_node), true), + .while_cont => _ = try whileExpr(gz, scope, .discard, inner_node, tree.whileCont(inner_node), true), + .@"while" => _ = try whileExpr(gz, scope, .discard, inner_node, tree.whileFull(inner_node), true), + + .for_simple => _ = try forExpr(gz, scope, .discard, inner_node, tree.forSimple(inner_node), true), + .@"for" => _ = try forExpr(gz, scope, .discard, inner_node, tree.forFull(inner_node), true), + + else => noreturn_src_node = try unusedResultExpr(gz, scope, inner_node), // zig fmt: on + } + break; } } @@ -2245,6 +2262,10 @@ fn unusedResultExpr(gz: *GenZir, scope: *Scope, statement: Ast.Node.Index) Inner // We need to emit an error if the result is not `noreturn` or `void`, but // we want to avoid adding the ZIR instruction if possible for performance. const maybe_unused_result = try expr(gz, scope, .none, statement); + return addEnsureResult(gz, maybe_unused_result, statement); +} + +fn addEnsureResult(gz: *GenZir, maybe_unused_result: Zir.Inst.Ref, statement: Ast.Node.Index) InnerError!Ast.Node.Index { var noreturn_src_node: Ast.Node.Index = 0; const elide_check = if (refToIndex(maybe_unused_result)) |inst| b: { // Note that this array becomes invalid after appending more items to it @@ -5648,6 +5669,7 @@ fn whileExpr( rl: ResultLoc, node: Ast.Node.Index, while_full: Ast.full.While, + is_statement: bool, ) InnerError!Zir.Inst.Ref { const astgen = parent_gz.astgen; const tree = astgen.tree; @@ -5818,6 +5840,8 @@ fn whileExpr( try then_scope.addDbgVar(.dbg_var_val, some, dbg_var_inst); } const then_result = try expr(&then_scope, then_sub_scope, loop_scope.break_result_loc, while_full.ast.then_expr); + _ = try addEnsureResult(&then_scope, then_result, while_full.ast.then_expr); + try checkUsed(parent_gz, &then_scope.base, then_sub_scope); try then_scope.addDbgBlockEnd(); @@ -5860,7 +5884,11 @@ fn whileExpr( // control flow apply to outer loops; not this one. loop_scope.continue_block = 0; loop_scope.break_block = 0; - const e = try expr(&else_scope, sub_scope, loop_scope.break_result_loc, else_node); + const else_result = try expr(&else_scope, sub_scope, loop_scope.break_result_loc, else_node); + if (is_statement) { + _ = try addEnsureResult(&else_scope, else_result, else_node); + } + if (!else_scope.endsWithNoReturn()) { loop_scope.break_count += 1; } @@ -5868,7 +5896,7 @@ fn whileExpr( try else_scope.addDbgBlockEnd(); break :blk .{ .src = else_node, - .result = e, + .result = else_result, }; } else .{ .src = while_full.ast.then_expr, @@ -5881,7 +5909,7 @@ fn whileExpr( } } const break_tag: Zir.Inst.Tag = if (is_inline) .break_inline else .@"break"; - return finishThenElseBlock( + const result = try finishThenElseBlock( parent_gz, rl, node, @@ -5896,6 +5924,10 @@ fn whileExpr( cond_block, break_tag, ); + if (is_statement) { + _ = try parent_gz.addUnNode(.ensure_result_used, result, node); + } + return result; } fn forExpr( @@ -5904,6 +5936,7 @@ fn forExpr( rl: ResultLoc, node: Ast.Node.Index, for_full: Ast.full.While, + is_statement: bool, ) InnerError!Zir.Inst.Ref { const astgen = parent_gz.astgen; @@ -6047,6 +6080,8 @@ fn forExpr( }; const then_result = try expr(&then_scope, then_sub_scope, loop_scope.break_result_loc, for_full.ast.then_expr); + _ = try addEnsureResult(&then_scope, then_result, for_full.ast.then_expr); + try checkUsed(parent_gz, &then_scope.base, then_sub_scope); try then_scope.addDbgBlockEnd(); @@ -6064,6 +6099,10 @@ fn forExpr( loop_scope.continue_block = 0; loop_scope.break_block = 0; const else_result = try expr(&else_scope, sub_scope, loop_scope.break_result_loc, else_node); + if (is_statement) { + _ = try addEnsureResult(&else_scope, else_result, else_node); + } + if (!else_scope.endsWithNoReturn()) { loop_scope.break_count += 1; } @@ -6082,7 +6121,7 @@ fn forExpr( } } const break_tag: Zir.Inst.Tag = if (is_inline) .break_inline else .@"break"; - return finishThenElseBlock( + const result = try finishThenElseBlock( parent_gz, rl, node, @@ -6097,6 +6136,10 @@ fn forExpr( cond_block, break_tag, ); + if (is_statement) { + _ = try parent_gz.addUnNode(.ensure_result_used, result, node); + } + return result; } fn switchExpr( diff --git a/test/cases/compile_errors/for_loop_body_expression_ignored.zig b/test/cases/compile_errors/for_loop_body_expression_ignored.zig new file mode 100644 index 0000000000..3ce73a9fab --- /dev/null +++ b/test/cases/compile_errors/for_loop_body_expression_ignored.zig @@ -0,0 +1,35 @@ +fn returns() usize { + return 2; +} +export fn f1() void { + for ("hello") |_| returns(); +} +export fn f2() void { + var x: anyerror!i32 = error.Bad; + for ("hello") |_| returns() else unreachable; + _ = x; +} +export fn f3() void { + for ("hello") |_| {} else true; +} +export fn f4() void { + const foo = for ("hello") |_| returns() else true; + _ = foo; +} + +// error +// backend=stage2 +// target=native +// +// :5:30: error: value of type 'usize' ignored +// :5:30: note: all non-void values must be used +// :5:30: note: this error can be suppressed by assigning the value to '_' +// :9:30: error: value of type 'usize' ignored +// :9:30: note: all non-void values must be used +// :9:30: note: this error can be suppressed by assigning the value to '_' +// :13:31: error: value of type 'bool' ignored +// :13:31: note: all non-void values must be used +// :13:31: note: this error can be suppressed by assigning the value to '_' +// :16:42: error: value of type 'usize' ignored +// :16:42: note: all non-void values must be used +// :16:42: note: this error can be suppressed by assigning the value to '_' diff --git a/test/cases/compile_errors/stage1/obj/for_loop_body_expression_ignored.zig b/test/cases/compile_errors/stage1/obj/for_loop_body_expression_ignored.zig deleted file mode 100644 index 6281d4b276..0000000000 --- a/test/cases/compile_errors/stage1/obj/for_loop_body_expression_ignored.zig +++ /dev/null @@ -1,18 +0,0 @@ -fn returns() usize { - return 2; -} -export fn f1() void { - for ("hello") |_| returns(); -} -export fn f2() void { - var x: anyerror!i32 = error.Bad; - for ("hello") |_| returns() else unreachable; - _ = x; -} - -// error -// backend=stage1 -// target=native -// -// tmp.zig:5:30: error: expression value is ignored -// tmp.zig:9:30: error: expression value is ignored diff --git a/test/cases/compile_errors/stage1/obj/while_loop_body_expression_ignored.zig b/test/cases/compile_errors/stage1/obj/while_loop_body_expression_ignored.zig deleted file mode 100644 index 9542cbc62f..0000000000 --- a/test/cases/compile_errors/stage1/obj/while_loop_body_expression_ignored.zig +++ /dev/null @@ -1,22 +0,0 @@ -fn returns() usize { - return 2; -} -export fn f1() void { - while (true) returns(); -} -export fn f2() void { - var x: ?i32 = null; - while (x) |_| returns(); -} -export fn f3() void { - var x: anyerror!i32 = error.Bad; - while (x) |_| returns() else |_| unreachable; -} - -// error -// backend=stage1 -// target=native -// -// tmp.zig:5:25: error: expression value is ignored -// tmp.zig:9:26: error: expression value is ignored -// tmp.zig:13:26: error: expression value is ignored diff --git a/test/cases/compile_errors/while_loop_body_expression_ignored.zig b/test/cases/compile_errors/while_loop_body_expression_ignored.zig new file mode 100644 index 0000000000..e33f48e6a5 --- /dev/null +++ b/test/cases/compile_errors/while_loop_body_expression_ignored.zig @@ -0,0 +1,43 @@ +fn returns() usize { + return 2; +} +export fn f1() void { + while (true) returns(); +} +export fn f2() void { + var x: ?i32 = null; + while (x) |_| returns(); +} +export fn f3() void { + var x: anyerror!i32 = error.Bad; + while (x) |_| returns() else |_| unreachable; +} +export fn f4() void { + var a = true; + while (a) {} else true; +} +export fn f5() void { + var a = true; + const foo = while (a) returns() else true; + _ = foo; +} + +// error +// backend=stage2 +// target=native +// +// :5:25: error: value of type 'usize' ignored +// :5:25: note: all non-void values must be used +// :5:25: note: this error can be suppressed by assigning the value to '_' +// :9:26: error: value of type 'usize' ignored +// :9:26: note: all non-void values must be used +// :9:26: note: this error can be suppressed by assigning the value to '_' +// :13:26: error: value of type 'usize' ignored +// :13:26: note: all non-void values must be used +// :13:26: note: this error can be suppressed by assigning the value to '_' +// :17:23: error: value of type 'bool' ignored +// :17:23: note: all non-void values must be used +// :17:23: note: this error can be suppressed by assigning the value to '_' +// :21:34: error: value of type 'usize' ignored +// :21:34: note: all non-void values must be used +// :21:34: note: this error can be suppressed by assigning the value to '_'