From 64cd236a9ebe8e134ff20ad9b2a6a06d7d0ea46c Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sat, 2 Jan 2021 14:28:03 -0700 Subject: [PATCH] stage2: fix handling compile error in inline fn call * scopes properly inherit inlining information * compile errors of inline function calls are properly attached to the caller rather than the callee. - added a test case for this * --watch still opens a repl if compile errors happen. --- src/Module.zig | 77 +++++++++++++++----------- src/main.zig | 2 +- src/zir_sema.zig | 125 ++++++++++++++++++++----------------------- test/stage2/test.zig | 51 ++++++++++++++++++ 4 files changed, 157 insertions(+), 98 deletions(-) diff --git a/src/Module.zig b/src/Module.zig index a9aa01a7fca1..75180c89c109 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -759,33 +759,33 @@ pub const Scope = struct { instructions: ArrayListUnmanaged(*Inst), /// Points to the arena allocator of DeclAnalysis arena: *Allocator, - label: Label = Label.none, + label: ?Label = null, + inlining: ?Inlining, is_comptime: bool, - pub const Label = union(enum) { - none, - /// This `Block` maps a block ZIR instruction to the corresponding - /// TZIR instruction for break instruction analysis. - breaking: struct { - zir_block: *zir.Inst.Block, - merges: Merges, - }, - /// This `Block` indicates that an inline function call is happening - /// and return instructions should be analyzed as a break instruction - /// to this TZIR block instruction. - inlining: struct { - /// We use this to count from 0 so that arg instructions know - /// which parameter index they are, without having to store - /// a parameter index with each arg instruction. - param_index: usize, - casted_args: []*Inst, - merges: Merges, - }, + /// This `Block` maps a block ZIR instruction to the corresponding + /// TZIR instruction for break instruction analysis. + pub const Label = struct { + zir_block: *zir.Inst.Block, + merges: Merges, + }; - pub const Merges = struct { - results: ArrayListUnmanaged(*Inst), - block_inst: *Inst.Block, - }; + /// This `Block` indicates that an inline function call is happening + /// and return instructions should be analyzed as a break instruction + /// to this TZIR block instruction. + pub const Inlining = struct { + caller: ?*Fn, + /// We use this to count from 0 so that arg instructions know + /// which parameter index they are, without having to store + /// a parameter index with each arg instruction. + param_index: usize, + casted_args: []*Inst, + merges: Merges, + }; + + pub const Merges = struct { + results: ArrayListUnmanaged(*Inst), + block_inst: *Inst.Block, }; /// For debugging purposes. @@ -1093,6 +1093,7 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { .decl = decl, .instructions = .{}, .arena = &decl_arena.allocator, + .inlining = null, .is_comptime = false, }; defer block_scope.instructions.deinit(self.gpa); @@ -1281,6 +1282,7 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { .decl = decl, .instructions = .{}, .arena = &decl_arena.allocator, + .inlining = null, .is_comptime = true, }; defer block_scope.instructions.deinit(self.gpa); @@ -1346,6 +1348,7 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { .decl = decl, .instructions = .{}, .arena = &gen_scope_arena.allocator, + .inlining = null, .is_comptime = true, }; defer inner_block.instructions.deinit(self.gpa); @@ -1466,6 +1469,7 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { .decl = decl, .instructions = .{}, .arena = &analysis_arena.allocator, + .inlining = null, .is_comptime = true, }; defer block_scope.instructions.deinit(self.gpa); @@ -1843,23 +1847,24 @@ pub fn analyzeFnBody(self: *Module, decl: *Decl, func: *Fn) !void { .decl = decl, .instructions = .{}, .arena = &arena.allocator, + .inlining = null, .is_comptime = false, }; defer inner_block.instructions.deinit(self.gpa); func.state = .in_progress; - log.debug("set {} to in_progress\n", .{decl.name}); + log.debug("set {s} to in_progress\n", .{decl.name}); try zir_sema.analyzeBody(self, &inner_block.base, func.zir); const instructions = try arena.allocator.dupe(*Inst, inner_block.instructions.items); func.state = .success; func.body = .{ .instructions = instructions }; - log.debug("set {} to success\n", .{decl.name}); + log.debug("set {s} to success\n", .{decl.name}); } fn markOutdatedDecl(self: *Module, decl: *Decl) !void { - log.debug("mark {} outdated\n", .{decl.name}); + log.debug("mark {s} outdated\n", .{decl.name}); try self.comp.work_queue.writeItem(.{ .analyze_decl = decl }); if (self.failed_decls.remove(decl)) |entry| { entry.value.destroy(self.gpa); @@ -3050,11 +3055,20 @@ fn failWithOwnedErrorMsg(self: *Module, scope: *Scope, src: usize, err_msg: *Com }, .block => { const block = scope.cast(Scope.Block).?; - if (block.func) |func| { - func.state = .sema_failure; + if (block.inlining) |*inlining| { + if (inlining.caller) |func| { + func.state = .sema_failure; + } else { + block.decl.analysis = .sema_failure; + block.decl.generation = self.generation; + } } else { - block.decl.analysis = .sema_failure; - block.decl.generation = self.generation; + if (block.func) |func| { + func.state = .sema_failure; + } else { + block.decl.analysis = .sema_failure; + block.decl.generation = self.generation; + } } self.failed_decls.putAssumeCapacityNoClobber(block.decl, err_msg); }, @@ -3414,6 +3428,7 @@ pub fn addSafetyCheck(mod: *Module, parent_block: *Scope.Block, ok: *Inst, panic .decl = parent_block.decl, .instructions = .{}, .arena = parent_block.arena, + .inlining = parent_block.inlining, .is_comptime = parent_block.is_comptime, }; defer fail_block.instructions.deinit(mod.gpa); diff --git a/src/main.zig b/src/main.zig index 247f8327f0f9..32eebff38c4c 100644 --- a/src/main.zig +++ b/src/main.zig @@ -1794,7 +1794,7 @@ fn buildOutputType( }; updateModule(gpa, comp, zir_out_path, hook) catch |err| switch (err) { - error.SemanticAnalyzeFail => process.exit(1), + error.SemanticAnalyzeFail => if (!watch) process.exit(1), else => |e| return e, }; try comp.makeBinFileExecutable(); diff --git a/src/zir_sema.zig b/src/zir_sema.zig index 01cbf9800741..7c790122e576 100644 --- a/src/zir_sema.zig +++ b/src/zir_sema.zig @@ -576,13 +576,10 @@ fn analyzeInstCompileError(mod: *Module, scope: *Scope, inst: *zir.Inst.UnOp) In fn analyzeInstArg(mod: *Module, scope: *Scope, inst: *zir.Inst.Arg) InnerError!*Inst { const b = try mod.requireFunctionBlock(scope, inst.base.src); - switch (b.label) { - .none, .breaking => {}, - .inlining => |*inlining| { - const param_index = inlining.param_index; - inlining.param_index += 1; - return inlining.casted_args[param_index]; - }, + if (b.inlining) |*inlining| { + const param_index = inlining.param_index; + inlining.param_index += 1; + return inlining.casted_args[param_index]; } const fn_ty = b.func.?.owner_decl.typed_value.most_recent.typed_value.ty; const param_index = b.instructions.items.len; @@ -620,6 +617,7 @@ fn analyzeInstLoop(mod: *Module, scope: *Scope, inst: *zir.Inst.Loop) InnerError .decl = parent_block.decl, .instructions = .{}, .arena = parent_block.arena, + .inlining = parent_block.inlining, .is_comptime = parent_block.is_comptime, }; defer child_block.instructions.deinit(mod.gpa); @@ -642,7 +640,8 @@ fn analyzeInstBlockFlat(mod: *Module, scope: *Scope, inst: *zir.Inst.Block, is_c .decl = parent_block.decl, .instructions = .{}, .arena = parent_block.arena, - .label = .none, + .label = null, + .inlining = parent_block.inlining, .is_comptime = parent_block.is_comptime or is_comptime, }; defer child_block.instructions.deinit(mod.gpa); @@ -680,18 +679,18 @@ fn analyzeInstBlock(mod: *Module, scope: *Scope, inst: *zir.Inst.Block, is_compt .decl = parent_block.decl, .instructions = .{}, .arena = parent_block.arena, - .label = Scope.Block.Label{ - .breaking = .{ - .zir_block = inst, - .merges = .{ - .results = .{}, - .block_inst = block_inst, - }, + // TODO @as here is working around a stage1 miscompilation bug :( + .label = @as(?Scope.Block.Label, Scope.Block.Label{ + .zir_block = inst, + .merges = .{ + .results = .{}, + .block_inst = block_inst, }, - }, + }), + .inlining = parent_block.inlining, .is_comptime = is_comptime or parent_block.is_comptime, }; - const merges = &child_block.label.breaking.merges; + const merges = &child_block.label.?.merges; defer child_block.instructions.deinit(mod.gpa); defer merges.results.deinit(mod.gpa); @@ -705,7 +704,7 @@ fn analyzeBlockBody( mod: *Module, scope: *Scope, child_block: *Scope.Block, - merges: *Scope.Block.Label.Merges, + merges: *Scope.Block.Merges, ) InnerError!*Inst { const parent_block = scope.cast(Scope.Block).?; @@ -895,19 +894,20 @@ fn analyzeInstCall(mod: *Module, scope: *Scope, inst: *zir.Inst.Call) InnerError .decl = scope.decl().?, .instructions = .{}, .arena = scope.arena(), - .label = Scope.Block.Label{ - .inlining = .{ - .param_index = 0, - .casted_args = casted_args, - .merges = .{ - .results = .{}, - .block_inst = block_inst, - }, + .label = null, + // TODO @as here is working around a stage1 miscompilation bug :( + .inlining = @as(?Scope.Block.Inlining, Scope.Block.Inlining{ + .caller = b.func, + .param_index = 0, + .casted_args = casted_args, + .merges = .{ + .results = .{}, + .block_inst = block_inst, }, - }, + }), .is_comptime = is_comptime_call, }; - const merges = &child_block.label.inlining.merges; + const merges = &child_block.inlining.?.merges; defer child_block.instructions.deinit(mod.gpa); defer merges.results.deinit(mod.gpa); @@ -1416,6 +1416,7 @@ fn analyzeInstSwitchBr(mod: *Module, scope: *Scope, inst: *zir.Inst.SwitchBr) In .decl = parent_block.decl, .instructions = .{}, .arena = parent_block.arena, + .inlining = parent_block.inlining, .is_comptime = parent_block.is_comptime, }; defer case_block.instructions.deinit(mod.gpa); @@ -1955,6 +1956,7 @@ fn analyzeInstCondBr(mod: *Module, scope: *Scope, inst: *zir.Inst.CondBr) InnerE .decl = parent_block.decl, .instructions = .{}, .arena = parent_block.arena, + .inlining = parent_block.inlining, .is_comptime = parent_block.is_comptime, }; defer true_block.instructions.deinit(mod.gpa); @@ -1966,6 +1968,7 @@ fn analyzeInstCondBr(mod: *Module, scope: *Scope, inst: *zir.Inst.CondBr) InnerE .decl = parent_block.decl, .instructions = .{}, .arena = parent_block.arena, + .inlining = parent_block.inlining, .is_comptime = parent_block.is_comptime, }; defer false_block.instructions.deinit(mod.gpa); @@ -1995,40 +1998,34 @@ fn analyzeInstRet(mod: *Module, scope: *Scope, inst: *zir.Inst.UnOp) InnerError! const operand = try resolveInst(mod, scope, inst.positionals.operand); const b = try mod.requireFunctionBlock(scope, inst.base.src); - switch (b.label) { - .inlining => |*inlining| { - // We are inlining a function call; rewrite the `ret` as a `break`. - try inlining.merges.results.append(mod.gpa, operand); - return mod.addBr(b, inst.base.src, inlining.merges.block_inst, operand); - }, - .none, .breaking => { - return mod.addUnOp(b, inst.base.src, Type.initTag(.noreturn), .ret, operand); - }, + if (b.inlining) |*inlining| { + // We are inlining a function call; rewrite the `ret` as a `break`. + try inlining.merges.results.append(mod.gpa, operand); + return mod.addBr(b, inst.base.src, inlining.merges.block_inst, operand); } + + return mod.addUnOp(b, inst.base.src, Type.initTag(.noreturn), .ret, operand); } fn analyzeInstRetVoid(mod: *Module, scope: *Scope, inst: *zir.Inst.NoOp) InnerError!*Inst { const b = try mod.requireFunctionBlock(scope, inst.base.src); - switch (b.label) { - .inlining => |*inlining| { - // We are inlining a function call; rewrite the `retvoid` as a `breakvoid`. - const void_inst = try mod.constVoid(scope, inst.base.src); - try inlining.merges.results.append(mod.gpa, void_inst); - return mod.addBr(b, inst.base.src, inlining.merges.block_inst, void_inst); - }, - .none, .breaking => { - if (b.func) |func| { - // Need to emit a compile error if returning void is not allowed. - const void_inst = try mod.constVoid(scope, inst.base.src); - const fn_ty = func.owner_decl.typed_value.most_recent.typed_value.ty; - const casted_void = try mod.coerce(scope, fn_ty.fnReturnType(), void_inst); - if (casted_void.ty.zigTypeTag() != .Void) { - return mod.addUnOp(b, inst.base.src, Type.initTag(.noreturn), .ret, casted_void); - } - } - return mod.addNoOp(b, inst.base.src, Type.initTag(.noreturn), .retvoid); - }, + if (b.inlining) |*inlining| { + // We are inlining a function call; rewrite the `retvoid` as a `breakvoid`. + const void_inst = try mod.constVoid(scope, inst.base.src); + try inlining.merges.results.append(mod.gpa, void_inst); + return mod.addBr(b, inst.base.src, inlining.merges.block_inst, void_inst); + } + + if (b.func) |func| { + // Need to emit a compile error if returning void is not allowed. + const void_inst = try mod.constVoid(scope, inst.base.src); + const fn_ty = func.owner_decl.typed_value.most_recent.typed_value.ty; + const casted_void = try mod.coerce(scope, fn_ty.fnReturnType(), void_inst); + if (casted_void.ty.zigTypeTag() != .Void) { + return mod.addUnOp(b, inst.base.src, Type.initTag(.noreturn), .ret, casted_void); + } } + return mod.addNoOp(b, inst.base.src, Type.initTag(.noreturn), .retvoid); } fn floatOpAllowed(tag: zir.Inst.Tag) bool { @@ -2048,16 +2045,12 @@ fn analyzeBreak( ) InnerError!*Inst { var opt_block = scope.cast(Scope.Block); while (opt_block) |block| { - switch (block.label) { - .none => {}, - .breaking => |*label| { - if (label.zir_block == zir_block) { - try label.merges.results.append(mod.gpa, operand); - const b = try mod.requireFunctionBlock(scope, src); - return mod.addBr(b, src, label.merges.block_inst, operand); - } - }, - .inlining => unreachable, // Invalid `break` ZIR inside inline function call. + if (block.label) |*label| { + if (label.zir_block == zir_block) { + try label.merges.results.append(mod.gpa, operand); + const b = try mod.requireFunctionBlock(scope, src); + return mod.addBr(b, src, label.merges.block_inst, operand); + } } opt_block = block.parent; } else unreachable; diff --git a/test/stage2/test.zig b/test/stage2/test.zig index 79f5c3a73e8a..8d3fac476096 100644 --- a/test/stage2/test.zig +++ b/test/stage2/test.zig @@ -1379,4 +1379,55 @@ pub fn addCases(ctx: *TestContext) !void { \\} , &[_][]const u8{":2:9: error: variable of type '@Type(.Null)' must be const or comptime"}); } + + { + var case = ctx.exe("compile error in inline fn call fixed", linux_x64); + case.addError( + \\export fn _start() noreturn { + \\ var x: usize = 3; + \\ const y = add(10, 2, x); + \\ exit(y - 6); + \\} + \\ + \\inline fn add(a: usize, b: usize, c: usize) usize { + \\ if (a == 10) @compileError("bad"); + \\ return a + b + c; + \\} + \\ + \\fn exit(code: usize) noreturn { + \\ asm volatile ("syscall" + \\ : + \\ : [number] "{rax}" (231), + \\ [arg1] "{rdi}" (code) + \\ : "rcx", "r11", "memory" + \\ ); + \\ unreachable; + \\} + , &[_][]const u8{":8:18: error: bad"}); + + case.addCompareOutput( + \\export fn _start() noreturn { + \\ var x: usize = 3; + \\ const y = add(1, 2, x); + \\ exit(y - 6); + \\} + \\ + \\inline fn add(a: usize, b: usize, c: usize) usize { + \\ if (a == 10) @compileError("bad"); + \\ return a + b + c; + \\} + \\ + \\fn exit(code: usize) noreturn { + \\ asm volatile ("syscall" + \\ : + \\ : [number] "{rax}" (231), + \\ [arg1] "{rdi}" (code) + \\ : "rcx", "r11", "memory" + \\ ); + \\ unreachable; + \\} + , + "", + ); + } }