Commit 183127733c8ab51c05f4d4b5cee2a1497272172a | zig

author	kkHAIKE <kkhaike@gmail.com> 2022-09-15 16:33:56+08:00
committer	Veikka Tuominen <git@vexu.eu> 2022-09-21 20:20:05+03:00
log	183127733c8ab51c05f4d4b5cee2a1497272172a
tree	85cd3267f1f67c77bb9b7ff250a32f3931974335
parent	0799e9856220cde895858ac7ae90ecf66e0b19f2

AstGen: make loop body's ResultLoc .none

Fixes #12555
Fixes #12551
Fixes #12455

5 files changed, 49 insertions(+), 37 deletions(-)

src
- AstGen.zig
test
- behavior.zig
- behavior
  - bugs
    11816.zig
    12551.zig
- cases
  - compile_errors
    unused_value_in_switch_in_loop.zig

src/AstGen.zig+10-37

 @@ -1864,15 +1864,8 @@ fn breakExpr(parent_gz: *GenZir, parent_scope: *Scope, node: Ast.Node.Index) Inn
                 }
                 block_gz.break_count += 1;
-                // The loop scope has a mechanism to prevent rvalue() from emitting a
-                // store to the result location for the loop body (since it is continues
-                // rather than returning a result from the loop) but here is a `break`
-                // which needs to override this behavior.
-                const prev_rvalue_noresult = parent_gz.rvalue_noresult;
-                parent_gz.rvalue_noresult = .none;
                 const operand = try reachableExpr(parent_gz, parent_scope, block_gz.break_result_loc, rhs, node);
                 const search_index = @intCast(Zir.Inst.Index, astgen.instructions.len);
-                parent_gz.rvalue_noresult = prev_rvalue_noresult;
                 try genDefers(parent_gz, scope, parent_scope, .normal_only);
 @@ -2193,7 +2186,7 @@ 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;
 @@ -5789,7 +5782,6 @@ fn whileExpr(
     // make scope now but don't stack on parent_gz until loop_scope
     // gets unstacked after cont_expr is emitted and added below
     var then_scope = parent_gz.makeSubBlock(&continue_scope.base);
-    then_scope.markAsLoopBody(loop_scope);
     then_scope.instructions_top = GenZir.unstacked_top;
     defer then_scope.unstack();
 @@ -5889,7 +5881,7 @@ fn whileExpr(
     if (dbg_var_name) |some| {
         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);
+    const then_result = try expr(&then_scope, then_sub_scope, .none, 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);
 @@ -6067,7 +6059,6 @@ fn forExpr(
     }
     var then_scope = parent_gz.makeSubBlock(&cond_scope.base);
-    then_scope.markAsLoopBody(loop_scope);
     defer then_scope.unstack();
     try then_scope.addDbgBlockBegin();
 @@ -6129,7 +6120,7 @@ fn forExpr(
         break :blk &index_scope.base;
     };
-    const then_result = try expr(&then_scope, then_sub_scope, loop_scope.break_result_loc, for_full.ast.then_expr);
+    const then_result = try expr(&then_scope, then_sub_scope, .none, 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);
 @@ -9465,25 +9456,19 @@ fn rvalue(
             }
         },
         .ptr => |ptr_inst| {
-            if (gz.rvalue_noresult != ptr_inst) {
+            _ = try gz.addPlNode(.store_node, src_node, Zir.Inst.Bin{
-                _ = try gz.addPlNode(.store_node, src_node, Zir.Inst.Bin{
+                .lhs = ptr_inst,
-                    .lhs = ptr_inst,
+                .rhs = result,
-                    .rhs = result,
+            });
-                });
-            }
             return result;
         },
         .inferred_ptr => |alloc| {
-            if (gz.rvalue_noresult != alloc) {
+            _ = try gz.addBin(.store_to_inferred_ptr, alloc, result);
-                _ = try gz.addBin(.store_to_inferred_ptr, alloc, result);
-            }
             return result;
         },
         .block_ptr => |block_scope| {
-            if (gz.rvalue_noresult != block_scope.rl_ptr) {
+            block_scope.rvalue_rl_count += 1;
-                block_scope.rvalue_rl_count += 1;
+            _ = try gz.addBin(.store_to_block_ptr, block_scope.rl_ptr, result);
-                _ = try gz.addBin(.store_to_block_ptr, block_scope.rl_ptr, result);
-            }
             return result;
         },
     }
 @@ -10163,7 +10148,6 @@ const GenZir = struct {
     rl_ptr: Zir.Inst.Ref = .none,
     /// When a block has a type result location, here it is.
     rl_ty_inst: Zir.Inst.Ref = .none,
-    rvalue_noresult: Zir.Inst.Ref = .none,
     /// Keeps track of how many branches of a block did not actually
     /// consume the result location. astgen uses this to figure out
     /// whether to rely on break instructions or writing to the result
 @@ -11558,17 +11542,6 @@ const GenZir = struct {
         try gz.astgen.instructions.append(gpa, .{ .tag = .dbg_block_end, .data = undefined });
         try gz.instructions.insert(gpa, gz.instructions.items.len - 1, new_index);
     }
-    /// Control flow does not fall through the "then" block of a loop; it continues
-    /// back to the while condition. This prevents `rvalue` from
-    /// adding an invalid store to the result location of `then_scope`.
-    fn markAsLoopBody(gz: *GenZir, loop_scope: GenZir) void {
-        gz.rvalue_noresult = switch (loop_scope.break_result_loc) {
-            .ptr, .inferred_ptr => |ptr| ptr,
-            .block_ptr => |block| block.rl_ptr,
-            else => .none,
-        };
-    }
 };
 /// This can only be for short-lived references; the memory becomes invalidated

test/behavior.zig+2

 @@ -81,10 +81,12 @@ test {
     _ = @import("behavior/bugs/11179.zig");
     _ = @import("behavior/bugs/11181.zig");
     _ = @import("behavior/bugs/11213.zig");
+    _ = @import("behavior/bugs/11816.zig");
     _ = @import("behavior/bugs/12003.zig");
     _ = @import("behavior/bugs/12033.zig");
     _ = @import("behavior/bugs/12430.zig");
     _ = @import("behavior/bugs/12486.zig");
+    _ = @import("behavior/bugs/12551.zig");
     _ = @import("behavior/bugs/12680.zig");
     _ = @import("behavior/bugs/12776.zig");
     _ = @import("behavior/bugs/12786.zig");

test/behavior/bugs/11816.zig created+13

...	@@ -0,0 +1,13 @@
1	const std = @import("std");
2	const builtin = @import("builtin");
3
4	test {
5	if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
6
7	var x: u32 = 3;
8	const val: usize = while (true) switch (x) {
9	1 => break 2,
10	else => x -= 1,
11	};
12	try std.testing.expect(val == 2);
13	}

test/behavior/bugs/12551.zig created+10

...	@@ -0,0 +1,10 @@
1	const std = @import("std");
2	const builtin = @import("builtin");
3
4	test {
5	if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
6
7	try std.testing.expect(for ([1]u8{0}) \|x\| {
8	if (x == 0) break true;
9	} else false);
10	}

test/cases/compile_errors/unused_value_in_switch_in_loop.zig created+14

...	@@ -0,0 +1,14 @@
1	export fn entry() void {
2	var a: u32 = 0;
3	while (true) switch (a) {
4	0 => 2,
5	1 => a = 0,
6	else => break,
7	};
8	}
9
10	// error
11	// backend=stage2
12	// target=native
13	//
14	// :3:18: error: incompatible types: 'comptime_int' and 'void'