From 9ca22004483a0d759254c4ad9b699dd5480f9719 Mon Sep 17 00:00:00 2001 From: Brett Broadhurst Date: Wed, 25 Mar 2026 23:50:42 -0600 Subject: [PATCH] fix: error reporting for global variables --- src/AstGen.zig | 35 +++++++++-------- src/Sema.zig | 92 +++++++++++++++++++++++++++------------------ src/compile.zig | 2 + src/error_tests.zig | 4 +- src/main.zig | 2 +- 5 files changed, 80 insertions(+), 55 deletions(-) diff --git a/src/AstGen.zig b/src/AstGen.zig index e2b498e..2a1ca5a 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -491,6 +491,7 @@ fn setDeclaration( name: Ir.NullTerminatedString, value: Ir.Inst.Index, gi: *GenIr, + node: *const Ast.Node, }, ) !void { const astgen = args.gi.astgen; @@ -498,17 +499,19 @@ fn setDeclaration( try astgen.extra.ensureUnusedCapacity(astgen.gpa, extra_len); const inst_data = &astgen.instructions.items[@intFromEnum(decl_index)].data; - inst_data.payload.extra_index = astgen.addExtraAssumeCapacity( - Ir.Inst.Declaration{ + inst_data.payload = .{ + .src_offset = @intCast(args.node.loc.start), + .extra_index = astgen.addExtraAssumeCapacity(Ir.Inst.Declaration{ .name = args.name, .value = args.value, - }, - ); + }), + }; } fn setDeclVarPayload( decl_index: Ir.Inst.Index, body_block: *GenIr, + node: *const Ast.Node, ) !void { defer body_block.unstack(); @@ -518,11 +521,12 @@ fn setDeclVarPayload( try astgen.extra.ensureUnusedCapacity(astgen.gpa, extra_len); const inst_data = &astgen.instructions.items[@intFromEnum(decl_index)].data; - inst_data.payload.extra_index = astgen.addExtraAssumeCapacity( - Ir.Inst.Var{ + inst_data.payload = .{ + .src_offset = @intCast(node.loc.start), + .extra_index = astgen.addExtraAssumeCapacity(Ir.Inst.Var{ .body_len = @intCast(body.len), - }, - ); + }), + }; astgen.appendBlockBody(body); } @@ -1266,11 +1270,12 @@ fn varDecl(gi: *GenIr, scope: *Scope, decl_node: *const Ast.Node) !void { _ = try expr(&decl_block, scope, expr_node); const name_str = try astgen.strFromNode(identifier_node); - try setDeclVarPayload(var_inst, &decl_block); + try setDeclVarPayload(var_inst, &decl_block, identifier_node); try setDeclaration(decl_inst, .{ .name = name_str.index, .value = var_inst, .gi = gi, + .node = decl_node, }); try astgen.globals.append(gpa, decl_inst); } @@ -1328,13 +1333,14 @@ fn defaultBlock( .name = decl_str.index, .value = knot_inst, .gi = gi, + .node = body_node, }); } fn stitchDeclInner( gi: *GenIr, scope: *Scope, - _: *const Ast.Node, + node: *const Ast.Node, prototype_node: *const Ast.Node, body_node: ?*const Ast.Node, ) InnerError!void { @@ -1377,6 +1383,7 @@ fn stitchDeclInner( .name = decl_str.index, .value = stitch_inst, .gi = gi, + .node = node, }); } @@ -1450,6 +1457,7 @@ fn knotDecl(gi: *GenIr, parent_scope: *Scope, decl_node: *const Ast.Node) InnerE .name = name_str.index, .value = knot_inst, .gi = gi, + .node = decl_node, }); } @@ -1461,7 +1469,6 @@ fn file(gi: *GenIr, scope: *Scope, file_node: *const Ast.Node) InnerError!void { }, }); - var start_index: usize = 0; var file_scope = gi.makeSubBlock(); defer file_scope.unstack(); @@ -1472,12 +1479,8 @@ fn file(gi: *GenIr, scope: *Scope, file_node: *const Ast.Node) InnerError!void { const first_child = nested_decls_list[0]; if (first_child.tag == .block_stmt) { try defaultBlock(&file_scope, scope, first_child); - if (nested_decls_list.len > 1) - start_index += 1 - else - return file_scope.setBlockBody(file_inst); } - for (nested_decls_list[start_index..]) |child_node| { + for (nested_decls_list[1..]) |child_node| { switch (child_node.tag) { .knot_decl => try knotDecl(gi, scope, child_node), .stitch_decl => try stitchDecl(gi, scope, child_node), diff --git a/src/Sema.zig b/src/Sema.zig index 4f563e3..1b66069 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -12,6 +12,7 @@ gpa: std.mem.Allocator, arena: std.mem.Allocator, module: *compile.Module, ir: Ir, +inst_map: std.AutoHashMapUnmanaged(Ir.Inst.Index, Ref) = .empty, errors: *std.ArrayListUnmanaged(Module.Error), const InnerError = error{ @@ -75,14 +76,16 @@ pub fn lookupIdentifier( sema: *Sema, chunk: *Chunk, ident: InternPool.Constant.Index, + src: SrcLoc, ) !Ref { - return sema.lookupInNamespace(chunk.namespace, ident); + return sema.lookupInNamespace(chunk.namespace, ident, src); } pub fn lookupInNamespace( sema: *Sema, namespace: *Module.Namespace, ident: InternPool.Constant.Index, + src: SrcLoc, ) !Ref { var scope: ?*Module.Namespace = namespace; while (scope) |s| : (scope = s.parent) { @@ -95,10 +98,11 @@ pub fn lookupInNamespace( }; } // FIXME: This is temporary - return sema.fail(.{ .src_offset = 0 }, "unknown identifier", .{}); + return sema.fail(src, "unknown identifier", .{}); } pub fn deinit(sema: *Sema) void { + sema.inst_map.deinit(sema.gpa); sema.* = undefined; } @@ -106,7 +110,6 @@ pub const Chunk = struct { sema: *Sema, namespace: *Module.Namespace, code: *Module.CodeChunk, - inst_map: std.AutoHashMapUnmanaged(Ir.Inst.Index, Ref) = .empty, constants_map: std.AutoHashMapUnmanaged(InternPool.Constant.Index, u8) = .empty, labels: std.ArrayListUnmanaged(Label) = .empty, fixups: std.ArrayListUnmanaged(Fixup) = .empty, @@ -127,7 +130,6 @@ pub const Chunk = struct { const dummy_address = 0xffffffff; pub fn deinit(chunk: *Chunk, gpa: std.mem.Allocator) void { - chunk.inst_map.deinit(gpa); chunk.constants_map.deinit(gpa); chunk.labels.deinit(gpa); chunk.fixups.deinit(gpa); @@ -223,7 +225,7 @@ pub const Chunk = struct { fn resolveInst(chunk: *Chunk, ref: Ir.Inst.Ref) Ref { if (ref.toIndex()) |index| { - return chunk.inst_map.get(index).?; + return chunk.sema.inst_map.get(index).?; } switch (ref) { .bool_true => return .bool_true, @@ -511,8 +513,9 @@ fn irImplicitRet(_: *Sema, chunk: *Chunk, _: Ir.Inst.Index) InnerError!Ref { fn irDeclRef(sema: *Sema, chunk: *Chunk, inst: Ir.Inst.Index) InnerError!Ref { const data = sema.ir.instructions[@intFromEnum(inst)].data.str_tok; + const src_loc: SrcLoc = .{ .src_offset = data.src_offset }; const decl_name = try sema.getOrPutStr(data.start); - return sema.lookupIdentifier(chunk, decl_name.constant); + return sema.lookupIdentifier(chunk, decl_name.constant, src_loc); } fn irCall(_: *Sema, _: *Chunk, _: Ir.Inst.Index) !Ref { @@ -532,19 +535,22 @@ fn irDivert( const data = sema.ir.instructions[@intFromEnum(inst)].data.payload; const extra = sema.ir.extraData(ExtraType, data.extra_index); const body = sema.ir.extra[extra.end..]; + const callee_src: SrcLoc = .{ .src_offset = data.src_offset }; switch (kind) { .direct => { const callee = chunk.resolveInst(extra.data.callee); - const callee_src: SrcLoc = .{ .src_offset = data.src_offset }; _ = try analyzeDivertTarget(sema, chunk, callee_src, callee); }, .field => { const callee = chunk.resolveInst(extra.data.obj_ptr); - const callee_src: SrcLoc = .{ .src_offset = data.src_offset }; const field_name = try sema.getOrPutStr(extra.data.field_name_start); _ = try analyzeDivertTarget(sema, chunk, callee_src, callee); - const e = try sema.lookupInNamespace(callee.knot.namespace, field_name.constant); + const e = try sema.lookupInNamespace( + callee.knot.namespace, + field_name.constant, + callee_src, + ); switch (e) { .knot => |knot| { const local_index = try chunk.getOrPutConstantIndex(knot.const_index); @@ -682,7 +688,7 @@ fn analyzeBodyInner(sema: *Sema, chunk: *Chunk, body: []const Ir.Inst.Index) Inn .field_ptr => try irFieldPtr(sema, chunk, inst), .param => try irParam(sema, chunk, inst), }; - try chunk.inst_map.put(sema.gpa, inst, ref); + try sema.inst_map.put(sema.gpa, inst, ref); } } @@ -738,6 +744,21 @@ fn analyzeNestedDecl( } } +pub fn analyzeGlobalBlockInner(sema: *Sema, chunk: *Chunk, inst: Ir.Inst.Index) !void { + const data = sema.ir.instructions[@intFromEnum(inst)].data.payload; + const extra = sema.ir.extraData(Ir.Inst.Var, data.payload.extra_index); + const body = sema.ir.bodySlice(extra.end, extra.data.body_len); + try analyzeBodyInner(sema, chunk, body); + // FIXME: hack + { + const last_inst = body[body.len - 1].toRef(); + const val = chunk.resolveInst(last_inst); + _ = try chunk.doLoad(val); + } + _ = try chunk.addConstOp(.store_global, @intCast(@intFromEnum(0))); + _ = try chunk.addByteOp(.pop); +} + pub fn analyzeTopLevelDecl( sema: *Sema, namespace: *Module.Namespace, @@ -747,42 +768,41 @@ pub fn analyzeTopLevelDecl( const extra = sema.ir.extraData(Ir.Inst.Declaration, data.extra_index).data; const decl_inst = sema.ir.instructions[@intFromEnum(extra.value)]; const decl_name = try sema.module.intern_pool.getOrPutStr(sema.gpa, extra.name); + const src_loc: SrcLoc = .{ .src_offset = data.src_offset }; + switch (decl_inst.tag) { .decl_var => { - const decl_extra = sema.ir.extraData(Ir.Inst.Var, decl_inst.data.payload.extra_index); - const body = sema.ir.bodySlice(decl_extra.end, decl_extra.data.body_len); - - try namespace.decls.put(sema.gpa, decl_name, .{ - .tag = .variable, - .namespace = null, - .decl_inst = extra.value, - .args_count = 0, - }); - - // FIXME: Broken - var chunk: *Chunk = undefined; - try analyzeBodyInner(sema, chunk, body); - // FIXME: hack - { - const last_inst = body[body.len - 1].toRef(); - const val = chunk.resolveInst(last_inst); - _ = try chunk.doLoad(val); + const gop = try namespace.decls.getOrPut(sema.arena, decl_name); + if (gop.found_existing) { + return sema.fail(src_loc, "duplicate identifier", .{}); + } else { + gop.value_ptr.* = .{ + .tag = .variable, + .namespace = null, + .decl_inst = extra.value, + .args_count = 0, + }; } - _ = try chunk.addConstOp(.store_global, @intCast(@intFromEnum(decl_name))); - _ = try chunk.addByteOp(.pop); }, .decl_knot => { const _data = sema.ir.instructions[@intFromEnum(extra.value)].data.payload; const _extra = sema.ir.extraData(Ir.Inst.Knot, _data.extra_index); const _body = sema.ir.bodySlice(_extra.end, _extra.data.body_len); const _stitches = sema.ir.bodySlice(_extra.end + _body.len, _extra.data.stitches_len); + const child_namespace = try sema.module.createNamespace(namespace); - try namespace.decls.put(sema.arena, decl_name, .{ - .tag = .knot, - .decl_inst = extra.value, - .args_count = 0, - .namespace = child_namespace, - }); + const gop = try namespace.decls.getOrPut(sema.arena, decl_name); + if (gop.found_existing) { + return sema.fail(src_loc, "duplicate identifier", .{}); + } else { + gop.value_ptr.* = .{ + .tag = .knot, + .decl_inst = extra.value, + .args_count = 0, + .namespace = child_namespace, + }; + } + try sema.module.queueWorkItem(.{ .tag = .knot, .decl_name = decl_name, diff --git a/src/compile.zig b/src/compile.zig index a55ee7a..90baa3d 100644 --- a/src/compile.zig +++ b/src/compile.zig @@ -345,8 +345,10 @@ pub const Module = struct { }); } } else { + // TODO: Revisit this. module.generateFile() catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, + error.AnalysisFail => return module, else => |e| @panic(@errorName(e)), }; } diff --git a/src/error_tests.zig b/src/error_tests.zig index b818bc0..f3eb4ba 100644 --- a/src/error_tests.zig +++ b/src/error_tests.zig @@ -6,7 +6,7 @@ test "compiler: unknown global variable" { try testEqual( \\{a} , - \\:1:2: error: unknown global variable + \\:1:2: error: unknown identifier \\1 | {a} \\ | ^ \\ @@ -20,7 +20,7 @@ test "compiler: global variable shadowing" { \\VAR b = 2 \\VAR a = 1 , - \\:3:1: error: redefined identifier + \\:3:1: error: duplicate identifier \\3 | VAR a = 1 \\ | ^ \\ diff --git a/src/main.zig b/src/main.zig index 804d756..0406be3 100644 --- a/src/main.zig +++ b/src/main.zig @@ -96,7 +96,7 @@ fn mainArgs( .dump_ir = dump_ir, .dump_trace = dump_trace, }) catch |err| switch (err) { - error.LoadFailed => std.process.exit(1), + //error.LoadFailed => std.process.exit(1), else => |e| return e, }; defer story.deinit();