authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2020-12-28 11:24:53-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2020-12-28 11:24:53-07:00
logf75d4cbe56f9f8212581f00700600a57ce545ba1
tree9061ca185cd38119116333c4fee9f5d26a0619dc
parent7ed499ec4549fa7304b822446b23cff993b8fa6f

Revert "stage2: add compile log statement (#7191)"

The addition of `addDeclErr` introduced a memory leak at every call site, and I also would like to push back on having more than 1 compilation error per `Decl`. This reverts commit 1634d45f1d53c8d7bfefa56ab4d2fa4cc8218b6d.

12 files changed, 40 insertions(+), 120 deletions(-)

src/Compilation.zig+12-13
......@@ -1350,11 +1350,8 @@ pub fn totalErrorCount(self: *Compilation) usize {
13501350 var total: usize = self.failed_c_objects.items().len;
13511351
13521352 if (self.bin_file.options.module) |module| {
1353 for (module.failed_decls.items()) |entry| {
1354 assert(entry.value.items.len > 0);
1355 total += entry.value.items.len;
1356 }
1357 total += module.failed_exports.items().len +
1353 total += module.failed_decls.items().len +
1354 module.failed_exports.items().len +
13581355 module.failed_files.items().len +
13591356 @boolToInt(module.failed_root_src_file != null);
13601357 }
......@@ -1388,11 +1385,9 @@ pub fn getAllErrorsAlloc(self: *Compilation) !AllErrors {
13881385 }
13891386 for (module.failed_decls.items()) |entry| {
13901387 const decl = entry.key;
1391 const err_msg_list = entry.value;
1392 for (err_msg_list.items) |err_msg| {
1393 const source = try decl.scope.getSource(module);
1394 try AllErrors.add(&arena, &errors, decl.scope.subFilePath(), source, err_msg.*);
1395 }
1388 const err_msg = entry.value;
1389 const source = try decl.scope.getSource(module);
1390 try AllErrors.add(&arena, &errors, decl.scope.subFilePath(), source, err_msg.*);
13961391 }
13971392 for (module.failed_exports.items()) |entry| {
13981393 const decl = entry.key.owner_decl;
......@@ -1485,6 +1480,7 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
14851480 }
14861481
14871482 assert(decl.typed_value.most_recent.typed_value.ty.hasCodeGenBits());
1483
14881484 self.bin_file.updateDecl(module, decl) catch |err| {
14891485 switch (err) {
14901486 error.OutOfMemory => return error.OutOfMemory,
......@@ -1492,7 +1488,8 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
14921488 decl.analysis = .dependency_failure;
14931489 },
14941490 else => {
1495 try module.addDeclErr(decl, try ErrorMsg.create(
1491 try module.failed_decls.ensureCapacity(module.gpa, module.failed_decls.items().len + 1);
1492 module.failed_decls.putAssumeCapacityNoClobber(decl, try ErrorMsg.create(
14961493 module.gpa,
14971494 decl.src(),
14981495 "unable to codegen: {}",
......@@ -1511,7 +1508,8 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
15111508 decl.analysis = .dependency_failure;
15121509 },
15131510 else => {
1514 try module.addDeclErr(decl, try ErrorMsg.create(
1511 try module.failed_decls.ensureCapacity(module.gpa, module.failed_decls.items().len + 1);
1512 module.failed_decls.putAssumeCapacityNoClobber(decl, try ErrorMsg.create(
15151513 module.gpa,
15161514 decl.src(),
15171515 "unable to generate C header: {}",
......@@ -1533,7 +1531,8 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
15331531 .update_line_number => |decl| {
15341532 const module = self.bin_file.options.module.?;
15351533 self.bin_file.updateDeclLineNumber(module, decl) catch |err| {
1536 try module.addDeclErr(decl, try ErrorMsg.create(
1534 try module.failed_decls.ensureCapacity(module.gpa, module.failed_decls.items().len + 1);
1535 module.failed_decls.putAssumeCapacityNoClobber(decl, try ErrorMsg.create(
15371536 module.gpa,
15381537 decl.src(),
15391538 "unable to update line number: {}",
src/Module.zig+17-29
......@@ -53,7 +53,7 @@ decl_table: std.ArrayHashMapUnmanaged(Scope.NameHash, *Decl, Scope.name_hash_has
5353/// The ErrorMsg memory is owned by the decl, using Module's general purpose allocator.
5454/// Note that a Decl can succeed but the Fn it represents can fail. In this case,
5555/// a Decl can have a failed_decls entry but have analysis status of success.
56failed_decls: std.AutoArrayHashMapUnmanaged(*Decl, ArrayListUnmanaged(*Compilation.ErrorMsg)) = .{},
56failed_decls: std.AutoArrayHashMapUnmanaged(*Decl, *Compilation.ErrorMsg) = .{},
5757/// Using a map here for consistency with the other fields here.
5858/// The ErrorMsg memory is owned by the `Scope`, using Module's general purpose allocator.
5959failed_files: std.AutoArrayHashMapUnmanaged(*Scope, *Compilation.ErrorMsg) = .{},
......@@ -849,11 +849,8 @@ pub fn deinit(self: *Module) void {
849849 }
850850 self.decl_table.deinit(gpa);
851851
852 for (self.failed_decls.items()) |*entry| {
853 for (entry.value.items) |compile_err| {
854 compile_err.destroy(gpa);
855 }
856 entry.value.deinit(gpa);
852 for (self.failed_decls.items()) |entry| {
853 entry.value.destroy(gpa);
857854 }
858855 self.failed_decls.deinit(gpa);
859856
......@@ -949,7 +946,8 @@ pub fn ensureDeclAnalyzed(self: *Module, decl: *Decl) InnerError!void {
949946 error.OutOfMemory => return error.OutOfMemory,
950947 error.AnalysisFail => return error.AnalysisFail,
951948 else => {
952 try self.addDeclErr(decl, try Compilation.ErrorMsg.create(
949 try self.failed_decls.ensureCapacity(self.gpa, self.failed_decls.items().len + 1);
950 self.failed_decls.putAssumeCapacityNoClobber(decl, try Compilation.ErrorMsg.create(
953951 self.gpa,
954952 decl.src(),
955953 "unable to analyze: {}",
......@@ -1556,7 +1554,7 @@ pub fn analyzeContainer(self: *Module, container_scope: *Scope.Container) !void
15561554 decl.analysis = .sema_failure;
15571555 const err_msg = try Compilation.ErrorMsg.create(self.gpa, tree.token_locs[name_tok].start, "redefinition of '{}'", .{decl.name});
15581556 errdefer err_msg.destroy(self.gpa);
1559 try self.addDeclErr(decl, err_msg);
1557 try self.failed_decls.putNoClobber(self.gpa, decl, err_msg);
15601558 } else {
15611559 if (!srcHashEql(decl.contents_hash, contents_hash)) {
15621560 try self.markOutdatedDecl(decl);
......@@ -1598,7 +1596,7 @@ pub fn analyzeContainer(self: *Module, container_scope: *Scope.Container) !void
15981596 decl.analysis = .sema_failure;
15991597 const err_msg = try Compilation.ErrorMsg.create(self.gpa, name_loc.start, "redefinition of '{}'", .{decl.name});
16001598 errdefer err_msg.destroy(self.gpa);
1601 try self.addDeclErr(decl, err_msg);
1599 try self.failed_decls.putNoClobber(self.gpa, decl, err_msg);
16021600 } else if (!srcHashEql(decl.contents_hash, contents_hash)) {
16031601 try self.markOutdatedDecl(decl);
16041602 decl.contents_hash = contents_hash;
......@@ -1724,11 +1722,8 @@ pub fn deleteDecl(self: *Module, decl: *Decl) !void {
17241722 try self.markOutdatedDecl(dep);
17251723 }
17261724 }
1727 if (self.failed_decls.remove(decl)) |*entry| {
1728 for (entry.value.items) |compile_err| {
1729 compile_err.destroy(self.gpa);
1730 }
1731 entry.value.deinit(self.gpa);
1725 if (self.failed_decls.remove(decl)) |entry| {
1726 entry.value.destroy(self.gpa);
17321727 }
17331728 self.deleteDeclExports(decl);
17341729 self.comp.bin_file.freeDecl(decl);
......@@ -1807,11 +1802,8 @@ pub fn analyzeFnBody(self: *Module, decl: *Decl, func: *Fn) !void {
18071802fn markOutdatedDecl(self: *Module, decl: *Decl) !void {
18081803 log.debug("mark {} outdated\n", .{decl.name});
18091804 try self.comp.work_queue.writeItem(.{ .analyze_decl = decl });
1810 if (self.failed_decls.remove(decl)) |*entry| {
1811 for (entry.value.items) |compile_err| {
1812 compile_err.destroy(self.gpa);
1813 }
1814 entry.value.deinit(self.gpa);
1805 if (self.failed_decls.remove(decl)) |entry| {
1806 entry.value.destroy(self.gpa);
18151807 }
18161808 decl.analysis = .outdated;
18171809}
......@@ -2956,14 +2948,10 @@ pub fn failNode(
29562948 return self.fail(scope, src, format, args);
29572949}
29582950
2959pub fn addDeclErr(self: *Module, decl: *Decl, err: *Compilation.ErrorMsg) error{OutOfMemory}!void {
2960 const entry = try self.failed_decls.getOrPutValue(self.gpa, decl, .{});
2961 try entry.value.append(self.gpa, err);
2962}
2963
29642951fn failWithOwnedErrorMsg(self: *Module, scope: *Scope, src: usize, err_msg: *Compilation.ErrorMsg) InnerError {
29652952 {
29662953 errdefer err_msg.destroy(self.gpa);
2954 try self.failed_decls.ensureCapacity(self.gpa, self.failed_decls.items().len + 1);
29672955 try self.failed_files.ensureCapacity(self.gpa, self.failed_files.items().len + 1);
29682956 }
29692957 switch (scope.tag) {
......@@ -2971,7 +2959,7 @@ fn failWithOwnedErrorMsg(self: *Module, scope: *Scope, src: usize, err_msg: *Com
29712959 const decl = scope.cast(Scope.DeclAnalysis).?.decl;
29722960 decl.analysis = .sema_failure;
29732961 decl.generation = self.generation;
2974 try self.addDeclErr(decl, err_msg);
2962 self.failed_decls.putAssumeCapacityNoClobber(decl, err_msg);
29752963 },
29762964 .block => {
29772965 const block = scope.cast(Scope.Block).?;
......@@ -2981,25 +2969,25 @@ fn failWithOwnedErrorMsg(self: *Module, scope: *Scope, src: usize, err_msg: *Com
29812969 block.decl.analysis = .sema_failure;
29822970 block.decl.generation = self.generation;
29832971 }
2984 try self.addDeclErr(block.decl, err_msg);
2972 self.failed_decls.putAssumeCapacityNoClobber(block.decl, err_msg);
29852973 },
29862974 .gen_zir => {
29872975 const gen_zir = scope.cast(Scope.GenZIR).?;
29882976 gen_zir.decl.analysis = .sema_failure;
29892977 gen_zir.decl.generation = self.generation;
2990 try self.addDeclErr(gen_zir.decl, err_msg);
2978 self.failed_decls.putAssumeCapacityNoClobber(gen_zir.decl, err_msg);
29912979 },
29922980 .local_val => {
29932981 const gen_zir = scope.cast(Scope.LocalVal).?.gen_zir;
29942982 gen_zir.decl.analysis = .sema_failure;
29952983 gen_zir.decl.generation = self.generation;
2996 try self.addDeclErr(gen_zir.decl, err_msg);
2984 self.failed_decls.putAssumeCapacityNoClobber(gen_zir.decl, err_msg);
29972985 },
29982986 .local_ptr => {
29992987 const gen_zir = scope.cast(Scope.LocalPtr).?.gen_zir;
30002988 gen_zir.decl.analysis = .sema_failure;
30012989 gen_zir.decl.generation = self.generation;
3002 try self.addDeclErr(gen_zir.decl, err_msg);
2990 self.failed_decls.putAssumeCapacityNoClobber(gen_zir.decl, err_msg);
30032991 },
30042992 .zir_module => {
30052993 const zir_module = scope.cast(Scope.ZIRModule).?;
src/astgen.zig-13
......@@ -2333,17 +2333,6 @@ fn compileError(mod: *Module, scope: *Scope, call: *ast.Node.BuiltinCall) InnerE
23332333 return addZIRUnOp(mod, scope, src, .compileerror, target);
23342334}
23352335
2336fn compileLog(mod: *Module, scope: *Scope, call: *ast.Node.BuiltinCall) InnerError!*zir.Inst {
2337 const tree = scope.tree();
2338 const arena = scope.arena();
2339 const src = tree.token_locs[call.builtin_token].start;
2340 const params = call.params();
2341 var targets = try arena.alloc(*zir.Inst, params.len);
2342 for (params) |param, param_i|
2343 targets[param_i] = try expr(mod, scope, .none, param);
2344 return addZIRInst(mod, scope, src, zir.Inst.CompileLog, .{ .to_log = targets }, .{});
2345}
2346
23472336fn typeOf(mod: *Module, scope: *Scope, rl: ResultLoc, call: *ast.Node.BuiltinCall) InnerError!*zir.Inst {
23482337 const tree = scope.tree();
23492338 const arena = scope.arena();
......@@ -2389,8 +2378,6 @@ fn builtinCall(mod: *Module, scope: *Scope, rl: ResultLoc, call: *ast.Node.Built
23892378 return rlWrap(mod, scope, rl, try import(mod, scope, call));
23902379 } else if (mem.eql(u8, builtin_name, "@compileError")) {
23912380 return compileError(mod, scope, call);
2392 } else if (mem.eql(u8, builtin_name, "@compileLog")) {
2393 return compileLog(mod, scope, call);
23942381 } else {
23952382 return mod.failTok(scope, call.builtin_token, "invalid builtin function: '{}'", .{builtin_name});
23962383 }
src/codegen/c.zig+1-1
......@@ -106,7 +106,7 @@ pub fn generateHeader(
106106 const writer = header.buf.writer();
107107 renderFunctionSignature(&ctx, header, writer, decl) catch |err| {
108108 if (err == error.AnalysisFail) {
109 try module.addDeclErr(decl, ctx.error_msg);
109 try module.failed_decls.put(module.gpa, decl, ctx.error_msg);
110110 }
111111 return err;
112112 };
src/link/C.zig+1-1
......@@ -106,7 +106,7 @@ pub fn deinit(self: *C) void {
106106pub fn updateDecl(self: *C, module: *Module, decl: *Module.Decl) !void {
107107 codegen.generate(self, decl) catch |err| {
108108 if (err == error.AnalysisFail) {
109 try module.addDeclErr(decl, self.error_msg);
109 try module.failed_decls.put(module.gpa, decl, self.error_msg);
110110 }
111111 return err;
112112 };
src/link/Coff.zig+1-1
......@@ -658,7 +658,7 @@ pub fn updateDecl(self: *Coff, module: *Module, decl: *Module.Decl) !void {
658658 .appended => code_buffer.items,
659659 .fail => |em| {
660660 decl.analysis = .codegen_failure;
661 try module.addDeclErr(decl, em);
661 try module.failed_decls.put(module.gpa, decl, em);
662662 return;
663663 },
664664 };
src/link/Elf.zig+1-1
......@@ -2248,7 +2248,7 @@ pub fn updateDecl(self: *Elf, module: *Module, decl: *Module.Decl) !void {
22482248 .appended => code_buffer.items,
22492249 .fail => |em| {
22502250 decl.analysis = .codegen_failure;
2251 try module.addDeclErr(decl, em);
2251 try module.failed_decls.put(module.gpa, decl, em);
22522252 return;
22532253 },
22542254 };
src/link/MachO.zig+1-1
......@@ -1062,7 +1062,7 @@ pub fn updateDecl(self: *MachO, module: *Module, decl: *Module.Decl) !void {
10621062 .appended => code_buffer.items,
10631063 .fail => |em| {
10641064 decl.analysis = .codegen_failure;
1065 try module.addDeclErr(decl, em);
1065 try module.failed_decls.put(module.gpa, decl, em);
10661066 return;
10671067 },
10681068 };
src/value.zig+1-1
......@@ -350,7 +350,7 @@ pub const Value = extern union {
350350 val = elem_ptr.array_ptr;
351351 },
352352 .empty_array => return out_stream.writeAll(".{}"),
353 .enum_literal => return out_stream.print(".{z}", .{@fieldParentPtr(Payload.Bytes, "base", self.ptr_otherwise).data}),
353 .enum_literal => return out_stream.print(".{z}", .{self.cast(Payload.Bytes).?.data}),
354354 .bytes => return out_stream.print("\"{Z}\"", .{self.cast(Payload.Bytes).?.data}),
355355 .repeated => {
356356 try out_stream.writeAll("(repeated) ");
src/zir.zig+4-21
......@@ -126,8 +126,6 @@ pub const Inst = struct {
126126 coerce_to_ptr_elem,
127127 /// Emit an error message and fail compilation.
128128 compileerror,
129 /// Log compile time variables and emit an error message.
130 compilelog,
131129 /// Conditional branch. Splits control flow based on a boolean condition value.
132130 condbr,
133131 /// Special case, has no textual representation.
......@@ -394,7 +392,6 @@ pub const Inst = struct {
394392 .declval => DeclVal,
395393 .declval_in_module => DeclValInModule,
396394 .coerce_result_block_ptr => CoerceResultBlockPtr,
397 .compilelog => CompileLog,
398395 .loop => Loop,
399396 .@"const" => Const,
400397 .str => Str,
......@@ -524,7 +521,6 @@ pub const Inst = struct {
524521 .slice_start,
525522 .import,
526523 .switch_range,
527 .compilelog,
528524 .typeof_peer,
529525 => false,
530526
......@@ -709,19 +705,6 @@ pub const Inst = struct {
709705 kw_args: struct {},
710706 };
711707
712 pub const CompileLog = struct {
713 pub const base_tag = Tag.compilelog;
714 base: Inst,
715
716 positionals: struct {
717 to_log: []*Inst,
718 },
719 kw_args: struct {
720 /// If we have seen it already so don't make another error
721 seen: bool = false,
722 },
723 };
724
725708 pub const Const = struct {
726709 pub const base_tag = Tag.@"const";
727710 base: Inst,
......@@ -1940,7 +1923,7 @@ const EmitZIR = struct {
19401923 .sema_failure,
19411924 .sema_failure_retryable,
19421925 .dependency_failure,
1943 => if (self.old_module.failed_decls.get(ir_decl)) |err_msg_list| {
1926 => if (self.old_module.failed_decls.get(ir_decl)) |err_msg| {
19441927 const fail_inst = try self.arena.allocator.create(Inst.UnOp);
19451928 fail_inst.* = .{
19461929 .base = .{
......@@ -1949,7 +1932,7 @@ const EmitZIR = struct {
19491932 },
19501933 .positionals = .{
19511934 .operand = blk: {
1952 const msg_str = try self.arena.allocator.dupe(u8, err_msg_list.items[0].msg);
1935 const msg_str = try self.arena.allocator.dupe(u8, err_msg.msg);
19531936
19541937 const str_inst = try self.arena.allocator.create(Inst.Str);
19551938 str_inst.* = .{
......@@ -1958,7 +1941,7 @@ const EmitZIR = struct {
19581941 .tag = Inst.Str.base_tag,
19591942 },
19601943 .positionals = .{
1961 .bytes = msg_str,
1944 .bytes = err_msg.msg,
19621945 },
19631946 .kw_args = .{},
19641947 };
......@@ -2085,7 +2068,7 @@ const EmitZIR = struct {
20852068 try self.emitBody(body, &inst_table, &instructions);
20862069 },
20872070 .sema_failure => {
2088 const err_msg = self.old_module.failed_decls.get(module_fn.owner_decl).?.items[0];
2071 const err_msg = self.old_module.failed_decls.get(module_fn.owner_decl).?;
20892072 const fail_inst = try self.arena.allocator.create(Inst.UnOp);
20902073 fail_inst.* = .{
20912074 .base = .{
src/zir_sema.zig-23
......@@ -45,7 +45,6 @@ pub fn analyzeInst(mod: *Module, scope: *Scope, old_inst: *zir.Inst) InnerError!
4545 .coerce_result_ptr => return analyzeInstCoerceResultPtr(mod, scope, old_inst.castTag(.coerce_result_ptr).?),
4646 .coerce_to_ptr_elem => return analyzeInstCoerceToPtrElem(mod, scope, old_inst.castTag(.coerce_to_ptr_elem).?),
4747 .compileerror => return analyzeInstCompileError(mod, scope, old_inst.castTag(.compileerror).?),
48 .compilelog => return analyzeInstCompileLog(mod, scope, old_inst.castTag(.compilelog).?),
4948 .@"const" => return analyzeInstConst(mod, scope, old_inst.castTag(.@"const").?),
5049 .dbg_stmt => return analyzeInstDbgStmt(mod, scope, old_inst.castTag(.dbg_stmt).?),
5150 .declref => return analyzeInstDeclRef(mod, scope, old_inst.castTag(.declref).?),
......@@ -503,28 +502,6 @@ fn analyzeInstCompileError(mod: *Module, scope: *Scope, inst: *zir.Inst.UnOp) In
503502 return mod.fail(scope, inst.base.src, "{}", .{msg});
504503}
505504
506fn analyzeInstCompileLog(mod: *Module, scope: *Scope, inst: *zir.Inst.CompileLog) InnerError!*Inst {
507 std.debug.print("| ", .{});
508 for (inst.positionals.to_log) |item, i| {
509 const to_log = try resolveInst(mod, scope, item);
510 if (to_log.value()) |val| {
511 std.debug.print("{}", .{val});
512 } else {
513 std.debug.print("(runtime value)", .{});
514 }
515 if (i != inst.positionals.to_log.len - 1) std.debug.print(", ", .{});
516 }
517 std.debug.print("\n", .{});
518 if (!inst.kw_args.seen) {
519 inst.kw_args.seen = true; // so that we do not give multiple compile errors if it gets evaled twice
520 switch (mod.fail(scope, inst.base.src, "found compile log statement", .{})) {
521 error.AnalysisFail => {}, // analysis continues
522 else => |e| return e,
523 }
524 }
525 return mod.constVoid(scope, inst.base.src);
526}
527
528505fn analyzeInstArg(mod: *Module, scope: *Scope, inst: *zir.Inst.Arg) InnerError!*Inst {
529506 const b = try mod.requireRuntimeBlock(scope, inst.base.src);
530507 const fn_ty = b.func.?.owner_decl.typed_value.most_recent.typed_value.ty;
test/stage2/test.zig+1-15
......@@ -1171,27 +1171,13 @@ pub fn addCases(ctx: *TestContext) !void {
11711171 \\fn entry() void {}
11721172 , &[_][]const u8{":2:4: error: redefinition of 'entry'"});
11731173
1174 ctx.compileError("compileLog", linux_x64,
1175 \\export fn _start() noreturn {
1176 \\ const b = true;
1177 \\ var f: u32 = 1;
1178 \\ @compileLog(b, 20, f, x, .foo);
1179 \\ var y: u32 = true;
1180 \\ unreachable;
1181 \\}
1182 \\fn x() void {}
1183 , &[_][]const u8{
1184 ":4:3: error: found compile log statement", ":5:16: error: expected u32, found bool",
1185 });
1186
1187 // "| true, 20, (runtime value), (function)" // TODO if this is here it invalidates the compile error checker. Need a way to check though.
1188
11891174 ctx.compileError("compileError", linux_x64,
11901175 \\export fn _start() noreturn {
11911176 \\ @compileError("this is an error");
11921177 \\ unreachable;
11931178 \\}
11941179 , &[_][]const u8{":2:3: error: this is an error"});
1180
11951181 {
11961182 var case = ctx.obj("variable shadowing", linux_x64);
11971183 case.addError(