Skip to content

Commit

Permalink
Revert "stage2: add compile log statement (ziglang#7191)"
Browse files Browse the repository at this point in the history
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 1634d45.
  • Loading branch information
andrewrk committed Dec 28, 2020
1 parent 7ed499e commit f75d4cb
Show file tree
Hide file tree
Showing 12 changed files with 40 additions and 120 deletions.
25 changes: 12 additions & 13 deletions src/Compilation.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1350,11 +1350,8 @@ pub fn totalErrorCount(self: *Compilation) usize {
var total: usize = self.failed_c_objects.items().len;

if (self.bin_file.options.module) |module| {
for (module.failed_decls.items()) |entry| {
assert(entry.value.items.len > 0);
total += entry.value.items.len;
}
total += module.failed_exports.items().len +
total += module.failed_decls.items().len +
module.failed_exports.items().len +
module.failed_files.items().len +
@boolToInt(module.failed_root_src_file != null);
}
Expand Down Expand Up @@ -1388,11 +1385,9 @@ pub fn getAllErrorsAlloc(self: *Compilation) !AllErrors {
}
for (module.failed_decls.items()) |entry| {
const decl = entry.key;
const err_msg_list = entry.value;
for (err_msg_list.items) |err_msg| {
const source = try decl.scope.getSource(module);
try AllErrors.add(&arena, &errors, decl.scope.subFilePath(), source, err_msg.*);
}
const err_msg = entry.value;
const source = try decl.scope.getSource(module);
try AllErrors.add(&arena, &errors, decl.scope.subFilePath(), source, err_msg.*);
}
for (module.failed_exports.items()) |entry| {
const decl = entry.key.owner_decl;
Expand Down Expand Up @@ -1485,14 +1480,16 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
}

assert(decl.typed_value.most_recent.typed_value.ty.hasCodeGenBits());

self.bin_file.updateDecl(module, decl) catch |err| {
switch (err) {
error.OutOfMemory => return error.OutOfMemory,
error.AnalysisFail => {
decl.analysis = .dependency_failure;
},
else => {
try module.addDeclErr(decl, try ErrorMsg.create(
try module.failed_decls.ensureCapacity(module.gpa, module.failed_decls.items().len + 1);
module.failed_decls.putAssumeCapacityNoClobber(decl, try ErrorMsg.create(
module.gpa,
decl.src(),
"unable to codegen: {}",
Expand All @@ -1511,7 +1508,8 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
decl.analysis = .dependency_failure;
},
else => {
try module.addDeclErr(decl, try ErrorMsg.create(
try module.failed_decls.ensureCapacity(module.gpa, module.failed_decls.items().len + 1);
module.failed_decls.putAssumeCapacityNoClobber(decl, try ErrorMsg.create(
module.gpa,
decl.src(),
"unable to generate C header: {}",
Expand All @@ -1533,7 +1531,8 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
.update_line_number => |decl| {
const module = self.bin_file.options.module.?;
self.bin_file.updateDeclLineNumber(module, decl) catch |err| {
try module.addDeclErr(decl, try ErrorMsg.create(
try module.failed_decls.ensureCapacity(module.gpa, module.failed_decls.items().len + 1);
module.failed_decls.putAssumeCapacityNoClobber(decl, try ErrorMsg.create(
module.gpa,
decl.src(),
"unable to update line number: {}",
Expand Down
46 changes: 17 additions & 29 deletions src/Module.zig
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ decl_table: std.ArrayHashMapUnmanaged(Scope.NameHash, *Decl, Scope.name_hash_has
/// The ErrorMsg memory is owned by the decl, using Module's general purpose allocator.
/// Note that a Decl can succeed but the Fn it represents can fail. In this case,
/// a Decl can have a failed_decls entry but have analysis status of success.
failed_decls: std.AutoArrayHashMapUnmanaged(*Decl, ArrayListUnmanaged(*Compilation.ErrorMsg)) = .{},
failed_decls: std.AutoArrayHashMapUnmanaged(*Decl, *Compilation.ErrorMsg) = .{},
/// Using a map here for consistency with the other fields here.
/// The ErrorMsg memory is owned by the `Scope`, using Module's general purpose allocator.
failed_files: std.AutoArrayHashMapUnmanaged(*Scope, *Compilation.ErrorMsg) = .{},
Expand Down Expand Up @@ -849,11 +849,8 @@ pub fn deinit(self: *Module) void {
}
self.decl_table.deinit(gpa);

for (self.failed_decls.items()) |*entry| {
for (entry.value.items) |compile_err| {
compile_err.destroy(gpa);
}
entry.value.deinit(gpa);
for (self.failed_decls.items()) |entry| {
entry.value.destroy(gpa);
}
self.failed_decls.deinit(gpa);

Expand Down Expand Up @@ -949,7 +946,8 @@ pub fn ensureDeclAnalyzed(self: *Module, decl: *Decl) InnerError!void {
error.OutOfMemory => return error.OutOfMemory,
error.AnalysisFail => return error.AnalysisFail,
else => {
try self.addDeclErr(decl, try Compilation.ErrorMsg.create(
try self.failed_decls.ensureCapacity(self.gpa, self.failed_decls.items().len + 1);
self.failed_decls.putAssumeCapacityNoClobber(decl, try Compilation.ErrorMsg.create(
self.gpa,
decl.src(),
"unable to analyze: {}",
Expand Down Expand Up @@ -1556,7 +1554,7 @@ pub fn analyzeContainer(self: *Module, container_scope: *Scope.Container) !void
decl.analysis = .sema_failure;
const err_msg = try Compilation.ErrorMsg.create(self.gpa, tree.token_locs[name_tok].start, "redefinition of '{}'", .{decl.name});
errdefer err_msg.destroy(self.gpa);
try self.addDeclErr(decl, err_msg);
try self.failed_decls.putNoClobber(self.gpa, decl, err_msg);
} else {
if (!srcHashEql(decl.contents_hash, contents_hash)) {
try self.markOutdatedDecl(decl);
Expand Down Expand Up @@ -1598,7 +1596,7 @@ pub fn analyzeContainer(self: *Module, container_scope: *Scope.Container) !void
decl.analysis = .sema_failure;
const err_msg = try Compilation.ErrorMsg.create(self.gpa, name_loc.start, "redefinition of '{}'", .{decl.name});
errdefer err_msg.destroy(self.gpa);
try self.addDeclErr(decl, err_msg);
try self.failed_decls.putNoClobber(self.gpa, decl, err_msg);
} else if (!srcHashEql(decl.contents_hash, contents_hash)) {
try self.markOutdatedDecl(decl);
decl.contents_hash = contents_hash;
Expand Down Expand Up @@ -1724,11 +1722,8 @@ pub fn deleteDecl(self: *Module, decl: *Decl) !void {
try self.markOutdatedDecl(dep);
}
}
if (self.failed_decls.remove(decl)) |*entry| {
for (entry.value.items) |compile_err| {
compile_err.destroy(self.gpa);
}
entry.value.deinit(self.gpa);
if (self.failed_decls.remove(decl)) |entry| {
entry.value.destroy(self.gpa);
}
self.deleteDeclExports(decl);
self.comp.bin_file.freeDecl(decl);
Expand Down Expand Up @@ -1807,11 +1802,8 @@ pub fn analyzeFnBody(self: *Module, decl: *Decl, func: *Fn) !void {
fn markOutdatedDecl(self: *Module, decl: *Decl) !void {
log.debug("mark {} outdated\n", .{decl.name});
try self.comp.work_queue.writeItem(.{ .analyze_decl = decl });
if (self.failed_decls.remove(decl)) |*entry| {
for (entry.value.items) |compile_err| {
compile_err.destroy(self.gpa);
}
entry.value.deinit(self.gpa);
if (self.failed_decls.remove(decl)) |entry| {
entry.value.destroy(self.gpa);
}
decl.analysis = .outdated;
}
Expand Down Expand Up @@ -2956,22 +2948,18 @@ pub fn failNode(
return self.fail(scope, src, format, args);
}

pub fn addDeclErr(self: *Module, decl: *Decl, err: *Compilation.ErrorMsg) error{OutOfMemory}!void {
const entry = try self.failed_decls.getOrPutValue(self.gpa, decl, .{});
try entry.value.append(self.gpa, err);
}

fn failWithOwnedErrorMsg(self: *Module, scope: *Scope, src: usize, err_msg: *Compilation.ErrorMsg) InnerError {
{
errdefer err_msg.destroy(self.gpa);
try self.failed_decls.ensureCapacity(self.gpa, self.failed_decls.items().len + 1);
try self.failed_files.ensureCapacity(self.gpa, self.failed_files.items().len + 1);
}
switch (scope.tag) {
.decl => {
const decl = scope.cast(Scope.DeclAnalysis).?.decl;
decl.analysis = .sema_failure;
decl.generation = self.generation;
try self.addDeclErr(decl, err_msg);
self.failed_decls.putAssumeCapacityNoClobber(decl, err_msg);
},
.block => {
const block = scope.cast(Scope.Block).?;
Expand All @@ -2981,25 +2969,25 @@ fn failWithOwnedErrorMsg(self: *Module, scope: *Scope, src: usize, err_msg: *Com
block.decl.analysis = .sema_failure;
block.decl.generation = self.generation;
}
try self.addDeclErr(block.decl, err_msg);
self.failed_decls.putAssumeCapacityNoClobber(block.decl, err_msg);
},
.gen_zir => {
const gen_zir = scope.cast(Scope.GenZIR).?;
gen_zir.decl.analysis = .sema_failure;
gen_zir.decl.generation = self.generation;
try self.addDeclErr(gen_zir.decl, err_msg);
self.failed_decls.putAssumeCapacityNoClobber(gen_zir.decl, err_msg);
},
.local_val => {
const gen_zir = scope.cast(Scope.LocalVal).?.gen_zir;
gen_zir.decl.analysis = .sema_failure;
gen_zir.decl.generation = self.generation;
try self.addDeclErr(gen_zir.decl, err_msg);
self.failed_decls.putAssumeCapacityNoClobber(gen_zir.decl, err_msg);
},
.local_ptr => {
const gen_zir = scope.cast(Scope.LocalPtr).?.gen_zir;
gen_zir.decl.analysis = .sema_failure;
gen_zir.decl.generation = self.generation;
try self.addDeclErr(gen_zir.decl, err_msg);
self.failed_decls.putAssumeCapacityNoClobber(gen_zir.decl, err_msg);
},
.zir_module => {
const zir_module = scope.cast(Scope.ZIRModule).?;
Expand Down
13 changes: 0 additions & 13 deletions src/astgen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2333,17 +2333,6 @@ fn compileError(mod: *Module, scope: *Scope, call: *ast.Node.BuiltinCall) InnerE
return addZIRUnOp(mod, scope, src, .compileerror, target);
}

fn compileLog(mod: *Module, scope: *Scope, call: *ast.Node.BuiltinCall) InnerError!*zir.Inst {
const tree = scope.tree();
const arena = scope.arena();
const src = tree.token_locs[call.builtin_token].start;
const params = call.params();
var targets = try arena.alloc(*zir.Inst, params.len);
for (params) |param, param_i|
targets[param_i] = try expr(mod, scope, .none, param);
return addZIRInst(mod, scope, src, zir.Inst.CompileLog, .{ .to_log = targets }, .{});
}

fn typeOf(mod: *Module, scope: *Scope, rl: ResultLoc, call: *ast.Node.BuiltinCall) InnerError!*zir.Inst {
const tree = scope.tree();
const arena = scope.arena();
Expand Down Expand Up @@ -2389,8 +2378,6 @@ fn builtinCall(mod: *Module, scope: *Scope, rl: ResultLoc, call: *ast.Node.Built
return rlWrap(mod, scope, rl, try import(mod, scope, call));
} else if (mem.eql(u8, builtin_name, "@compileError")) {
return compileError(mod, scope, call);
} else if (mem.eql(u8, builtin_name, "@compileLog")) {
return compileLog(mod, scope, call);
} else {
return mod.failTok(scope, call.builtin_token, "invalid builtin function: '{}'", .{builtin_name});
}
Expand Down
2 changes: 1 addition & 1 deletion src/codegen/c.zig
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub fn generateHeader(
const writer = header.buf.writer();
renderFunctionSignature(&ctx, header, writer, decl) catch |err| {
if (err == error.AnalysisFail) {
try module.addDeclErr(decl, ctx.error_msg);
try module.failed_decls.put(module.gpa, decl, ctx.error_msg);
}
return err;
};
Expand Down
2 changes: 1 addition & 1 deletion src/link/C.zig
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub fn deinit(self: *C) void {
pub fn updateDecl(self: *C, module: *Module, decl: *Module.Decl) !void {
codegen.generate(self, decl) catch |err| {
if (err == error.AnalysisFail) {
try module.addDeclErr(decl, self.error_msg);
try module.failed_decls.put(module.gpa, decl, self.error_msg);
}
return err;
};
Expand Down
2 changes: 1 addition & 1 deletion src/link/Coff.zig
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ pub fn updateDecl(self: *Coff, module: *Module, decl: *Module.Decl) !void {
.appended => code_buffer.items,
.fail => |em| {
decl.analysis = .codegen_failure;
try module.addDeclErr(decl, em);
try module.failed_decls.put(module.gpa, decl, em);
return;
},
};
Expand Down
2 changes: 1 addition & 1 deletion src/link/Elf.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2248,7 +2248,7 @@ pub fn updateDecl(self: *Elf, module: *Module, decl: *Module.Decl) !void {
.appended => code_buffer.items,
.fail => |em| {
decl.analysis = .codegen_failure;
try module.addDeclErr(decl, em);
try module.failed_decls.put(module.gpa, decl, em);
return;
},
};
Expand Down
2 changes: 1 addition & 1 deletion src/link/MachO.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ pub fn updateDecl(self: *MachO, module: *Module, decl: *Module.Decl) !void {
.appended => code_buffer.items,
.fail => |em| {
decl.analysis = .codegen_failure;
try module.addDeclErr(decl, em);
try module.failed_decls.put(module.gpa, decl, em);
return;
},
};
Expand Down
2 changes: 1 addition & 1 deletion src/value.zig
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ pub const Value = extern union {
val = elem_ptr.array_ptr;
},
.empty_array => return out_stream.writeAll(".{}"),
.enum_literal => return out_stream.print(".{z}", .{@fieldParentPtr(Payload.Bytes, "base", self.ptr_otherwise).data}),
.enum_literal => return out_stream.print(".{z}", .{self.cast(Payload.Bytes).?.data}),
.bytes => return out_stream.print("\"{Z}\"", .{self.cast(Payload.Bytes).?.data}),
.repeated => {
try out_stream.writeAll("(repeated) ");
Expand Down
25 changes: 4 additions & 21 deletions src/zir.zig
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ pub const Inst = struct {
coerce_to_ptr_elem,
/// Emit an error message and fail compilation.
compileerror,
/// Log compile time variables and emit an error message.
compilelog,
/// Conditional branch. Splits control flow based on a boolean condition value.
condbr,
/// Special case, has no textual representation.
Expand Down Expand Up @@ -394,7 +392,6 @@ pub const Inst = struct {
.declval => DeclVal,
.declval_in_module => DeclValInModule,
.coerce_result_block_ptr => CoerceResultBlockPtr,
.compilelog => CompileLog,
.loop => Loop,
.@"const" => Const,
.str => Str,
Expand Down Expand Up @@ -524,7 +521,6 @@ pub const Inst = struct {
.slice_start,
.import,
.switch_range,
.compilelog,
.typeof_peer,
=> false,

Expand Down Expand Up @@ -709,19 +705,6 @@ pub const Inst = struct {
kw_args: struct {},
};

pub const CompileLog = struct {
pub const base_tag = Tag.compilelog;
base: Inst,

positionals: struct {
to_log: []*Inst,
},
kw_args: struct {
/// If we have seen it already so don't make another error
seen: bool = false,
},
};

pub const Const = struct {
pub const base_tag = Tag.@"const";
base: Inst,
Expand Down Expand Up @@ -1940,7 +1923,7 @@ const EmitZIR = struct {
.sema_failure,
.sema_failure_retryable,
.dependency_failure,
=> if (self.old_module.failed_decls.get(ir_decl)) |err_msg_list| {
=> if (self.old_module.failed_decls.get(ir_decl)) |err_msg| {
const fail_inst = try self.arena.allocator.create(Inst.UnOp);
fail_inst.* = .{
.base = .{
Expand All @@ -1949,7 +1932,7 @@ const EmitZIR = struct {
},
.positionals = .{
.operand = blk: {
const msg_str = try self.arena.allocator.dupe(u8, err_msg_list.items[0].msg);
const msg_str = try self.arena.allocator.dupe(u8, err_msg.msg);

const str_inst = try self.arena.allocator.create(Inst.Str);
str_inst.* = .{
Expand All @@ -1958,7 +1941,7 @@ const EmitZIR = struct {
.tag = Inst.Str.base_tag,
},
.positionals = .{
.bytes = msg_str,
.bytes = err_msg.msg,
},
.kw_args = .{},
};
Expand Down Expand Up @@ -2085,7 +2068,7 @@ const EmitZIR = struct {
try self.emitBody(body, &inst_table, &instructions);
},
.sema_failure => {
const err_msg = self.old_module.failed_decls.get(module_fn.owner_decl).?.items[0];
const err_msg = self.old_module.failed_decls.get(module_fn.owner_decl).?;
const fail_inst = try self.arena.allocator.create(Inst.UnOp);
fail_inst.* = .{
.base = .{
Expand Down
23 changes: 0 additions & 23 deletions src/zir_sema.zig
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ pub fn analyzeInst(mod: *Module, scope: *Scope, old_inst: *zir.Inst) InnerError!
.coerce_result_ptr => return analyzeInstCoerceResultPtr(mod, scope, old_inst.castTag(.coerce_result_ptr).?),
.coerce_to_ptr_elem => return analyzeInstCoerceToPtrElem(mod, scope, old_inst.castTag(.coerce_to_ptr_elem).?),
.compileerror => return analyzeInstCompileError(mod, scope, old_inst.castTag(.compileerror).?),
.compilelog => return analyzeInstCompileLog(mod, scope, old_inst.castTag(.compilelog).?),
.@"const" => return analyzeInstConst(mod, scope, old_inst.castTag(.@"const").?),
.dbg_stmt => return analyzeInstDbgStmt(mod, scope, old_inst.castTag(.dbg_stmt).?),
.declref => return analyzeInstDeclRef(mod, scope, old_inst.castTag(.declref).?),
Expand Down Expand Up @@ -503,28 +502,6 @@ fn analyzeInstCompileError(mod: *Module, scope: *Scope, inst: *zir.Inst.UnOp) In
return mod.fail(scope, inst.base.src, "{}", .{msg});
}

fn analyzeInstCompileLog(mod: *Module, scope: *Scope, inst: *zir.Inst.CompileLog) InnerError!*Inst {
std.debug.print("| ", .{});
for (inst.positionals.to_log) |item, i| {
const to_log = try resolveInst(mod, scope, item);
if (to_log.value()) |val| {
std.debug.print("{}", .{val});
} else {
std.debug.print("(runtime value)", .{});
}
if (i != inst.positionals.to_log.len - 1) std.debug.print(", ", .{});
}
std.debug.print("\n", .{});
if (!inst.kw_args.seen) {
inst.kw_args.seen = true; // so that we do not give multiple compile errors if it gets evaled twice
switch (mod.fail(scope, inst.base.src, "found compile log statement", .{})) {
error.AnalysisFail => {}, // analysis continues
else => |e| return e,
}
}
return mod.constVoid(scope, inst.base.src);
}

fn analyzeInstArg(mod: *Module, scope: *Scope, inst: *zir.Inst.Arg) InnerError!*Inst {
const b = try mod.requireRuntimeBlock(scope, inst.base.src);
const fn_ty = b.func.?.owner_decl.typed_value.most_recent.typed_value.ty;
Expand Down
Loading

0 comments on commit f75d4cb

Please sign in to comment.