From 5eb5d523b50e8e8912cf84bf021dc4c95e521a7a Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 8 Oct 2023 20:58:04 -0700 Subject: [PATCH] give modules friendly names for error reporting --- src/Compilation.zig | 18 ++++++------ src/Package/Module.zig | 2 ++ src/Sema.zig | 7 ++--- src/main.zig | 28 ++++++++++++++----- .../import_of_missing_package.zig | 2 +- .../compile_errors/import_outside_package.zig | 2 +- .../import_outside_package_path.zig | 2 +- test/compile_errors.zig | 2 +- 8 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/Compilation.zig b/src/Compilation.zig index 9f2b07c501..cd4c6ea11b 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -1290,6 +1290,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { const builtin_mod = try Package.Module.create(arena, .{ .root = .{ .root_dir = zig_cache_artifact_directory }, .root_src_path = "builtin.zig", + .fully_qualified_name = "builtin", }); // When you're testing std, the main module is std. In that case, @@ -1319,6 +1320,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { .sub_path = "std", }, .root_src_path = "std.zig", + .fully_qualified_name = "std", }); const root_mod = if (options.is_test) root_mod: { @@ -1329,6 +1331,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { .sub_path = std.fs.path.dirname(test_runner) orelse "", }, .root_src_path = std.fs.path.basename(test_runner), + .fully_qualified_name = "root", }); pkg.deps = try main_mod.deps.clone(arena); @@ -1338,6 +1341,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { .root_dir = options.zig_lib_directory, }, .root_src_path = "test_runner.zig", + .fully_qualified_name = "root", }); break :root_mod test_mod; @@ -1349,6 +1353,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { .root_dir = options.zig_lib_directory, }, .root_src_path = "compiler_rt.zig", + .fully_qualified_name = "compiler_rt", }); } else null; @@ -2616,23 +2621,19 @@ fn reportMultiModuleErrors(mod: *Module) !void { errdefer for (notes[0..i]) |*n| n.deinit(mod.gpa); note.* = switch (ref) { .import => |loc| blk: { - //const name = try loc.file_scope.mod.getName(mod.gpa, mod.*); - //defer mod.gpa.free(name); break :blk try Module.ErrorMsg.init( mod.gpa, loc, - "imported from module {}", - .{loc.file_scope.mod.root}, + "imported from module {s}", + .{loc.file_scope.mod.fully_qualified_name}, ); }, .root => |pkg| blk: { - //const name = try pkg.getName(mod.gpa, mod.*); - //defer mod.gpa.free(name); break :blk try Module.ErrorMsg.init( mod.gpa, .{ .file_scope = file, .parent_decl_node = 0, .lazy = .entire_file }, - "root of module {}", - .{pkg.root}, + "root of module {s}", + .{pkg.fully_qualified_name}, ); }, }; @@ -6362,6 +6363,7 @@ fn buildOutputFromZig( var main_mod: Package.Module = .{ .root = .{ .root_dir = comp.zig_lib_directory }, .root_src_path = src_basename, + .fully_qualified_name = "root", }; const root_name = src_basename[0 .. src_basename.len - std.fs.path.extension(src_basename).len]; const target = comp.getTarget(); diff --git a/src/Package/Module.zig b/src/Package/Module.zig index 3502def385..7e6b518892 100644 --- a/src/Package/Module.zig +++ b/src/Package/Module.zig @@ -6,6 +6,8 @@ root: Package.Path, /// Relative to `root`. May contain path separators. root_src_path: []const u8, +/// Name used in compile errors. Looks like "root.foo.bar". +fully_qualified_name: []const u8, /// The dependency table of this module. Shared dependencies such as 'std', /// 'builtin', and 'root' are not specified in every dependency table, but /// instead only in the table of `main_mod`. `Module.importFile` is diff --git a/src/Sema.zig b/src/Sema.zig index 3cd9baed78..a7073a5f36 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -5798,6 +5798,7 @@ fn zirCImport(sema: *Sema, parent_block: *Block, inst: Zir.Inst.Index) CompileEr .sub_path = std.fs.path.dirname(c_import_res.out_zig_path) orelse "", }, .root_src_path = std.fs.path.basename(c_import_res.out_zig_path), + .fully_qualified_name = c_import_res.out_zig_path, }); const result = mod.importPkg(c_import_mod) catch |err| @@ -13076,10 +13077,8 @@ fn zirImport(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. return sema.fail(block, operand_src, "import of file outside module path: '{s}'", .{operand}); }, error.ModuleNotFound => { - //const name = try block.getFileScope(mod).mod.getName(sema.gpa, mod.*); - //defer sema.gpa.free(name); - return sema.fail(block, operand_src, "no module named '{s}' available within module '{}'", .{ - operand, block.getFileScope(mod).mod.root, + return sema.fail(block, operand_src, "no module named '{s}' available within module {s}", .{ + operand, block.getFileScope(mod).mod.fully_qualified_name, }); }, else => { diff --git a/src/main.zig b/src/main.zig index 2ec2f692ba..f0c03a69ce 100644 --- a/src/main.zig +++ b/src/main.zig @@ -1022,13 +1022,10 @@ fn buildOutputType( } } - var mod_it = modules.iterator(); - while (mod_it.next()) |kv| { - if (std.mem.eql(u8, mod_name, kv.key_ptr.*)) { - fatal("unable to add module '{s}' -> '{s}': already exists as '{s}'", .{ - mod_name, root_src, kv.value_ptr.mod.root_src_path, - }); - } + if (modules.get(mod_name)) |value| { + fatal("unable to add module '{s}' -> '{s}': already exists as '{s}'", .{ + mod_name, root_src, value.mod.root_src_path, + }); } try modules.put(mod_name, .{ @@ -1038,6 +1035,7 @@ fn buildOutputType( .sub_path = fs.path.dirname(root_src) orelse "", }, .root_src_path = fs.path.basename(root_src), + .fully_qualified_name = mod_name, }), .deps_str = deps_str, }); @@ -3247,6 +3245,7 @@ fn buildOutputType( src_path else try fs.path.relative(arena, p, src_path), + .fully_qualified_name = "root", }); } else { break :blk try Package.Module.create(arena, .{ @@ -3255,6 +3254,7 @@ fn buildOutputType( .sub_path = fs.path.dirname(src_path) orelse "", }, .root_src_path = fs.path.basename(src_path), + .fully_qualified_name = "root", }); } } else null; @@ -4820,16 +4820,19 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi .sub_path = fs.path.dirname(build_runner_path) orelse "", }, .root_src_path = fs.path.basename(build_runner_path), + .fully_qualified_name = "root", } else .{ .root = .{ .root_dir = zig_lib_directory }, .root_src_path = "build_runner.zig", + .fully_qualified_name = "root", }; var build_mod: Package.Module = .{ .root = .{ .root_dir = build_root }, .root_src_path = build_zig_basename, + .fully_qualified_name = "root.@build", }; if (build_options.only_core_functionality) { try createEmptyDependenciesModule(arena, &main_mod, local_cache_directory); @@ -4921,6 +4924,11 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi const m = try Package.Module.create(arena, .{ .root = try f.package_root.clone(arena), .root_src_path = Package.build_zig_basename, + .fully_qualified_name = try std.fmt.allocPrint( + arena, + "root.@dependencies.{s}", + .{&hash}, + ), }); const hash_cloned = try arena.dupe(u8, &hash); deps_mod.deps.putAssumeCapacityNoClobber(hash_cloned, m); @@ -5181,6 +5189,7 @@ pub fn cmdFmt(gpa: Allocator, arena: Allocator, args: []const []const u8) !void file.mod = try Package.Module.create(arena, .{ .root = Package.Path.cwd(), .root_src_path = file.sub_file_path, + .fully_qualified_name = "root", }); file.zir = try AstGen.generate(gpa, file.tree); @@ -5389,6 +5398,7 @@ fn fmtPathFile( file.mod = try Package.Module.create(fmt.arena, .{ .root = Package.Path.cwd(), .root_src_path = file.sub_file_path, + .fully_qualified_name = "root", }); if (stat.size > max_src_size) @@ -5472,6 +5482,7 @@ pub fn putAstErrorsIntoBundle( file.mod = try Package.Module.create(gpa, .{ .root = Package.Path.cwd(), .root_src_path = file.sub_file_path, + .fully_qualified_name = "root", }); defer gpa.destroy(file.mod); @@ -6040,6 +6051,7 @@ pub fn cmdAstCheck( file.mod = try Package.Module.create(arena, .{ .root = Package.Path.cwd(), .root_src_path = file.sub_file_path, + .fully_qualified_name = "root", }); file.tree = try Ast.parse(gpa, file.source, .zig); @@ -6211,6 +6223,7 @@ pub fn cmdChangelist( file.mod = try Package.Module.create(arena, .{ .root = Package.Path.cwd(), .root_src_path = file.sub_file_path, + .fully_qualified_name = "root", }); const source = try arena.allocSentinel(u8, @as(usize, @intCast(stat.size)), 0); @@ -6846,6 +6859,7 @@ fn createDependenciesModule( .sub_path = o_dir_sub_path, }, .root_src_path = basename, + .fully_qualified_name = "root.@dependencies", }); try main_mod.deps.put(arena, "@dependencies", deps_mod); return deps_mod; diff --git a/test/cases/compile_errors/import_of_missing_package.zig b/test/cases/compile_errors/import_of_missing_package.zig index d2a64638e8..f2cde57734 100644 --- a/test/cases/compile_errors/import_of_missing_package.zig +++ b/test/cases/compile_errors/import_of_missing_package.zig @@ -7,4 +7,4 @@ comptime { // backend=stage2 // target=native // -// :1:21: error: no package named 'foo' available within package 'root' +// :1:21: error: no module named 'foo' available within module root diff --git a/test/cases/compile_errors/import_outside_package.zig b/test/cases/compile_errors/import_outside_package.zig index 59a75e2745..2a6ead5b3c 100644 --- a/test/cases/compile_errors/import_outside_package.zig +++ b/test/cases/compile_errors/import_outside_package.zig @@ -5,4 +5,4 @@ export fn a() usize { // error // target=native // -// :2:20: error: import of file outside package path: '../../above.zig' +// :2:20: error: import of file outside module path: '../../above.zig' diff --git a/test/cases/compile_errors/import_outside_package_path.zig b/test/cases/compile_errors/import_outside_package_path.zig index 34044e3b0f..e3d2d228a4 100644 --- a/test/cases/compile_errors/import_outside_package_path.zig +++ b/test/cases/compile_errors/import_outside_package_path.zig @@ -6,4 +6,4 @@ comptime { // backend=stage2 // target=native // -// :2:17: error: import of file outside package path: '../a.zig' +// :2:17: error: import of file outside module path: '../a.zig' diff --git a/test/compile_errors.zig b/test/compile_errors.zig index 5f787683c4..a3bdf5b8f1 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -129,7 +129,7 @@ pub fn addCases(ctx: *Cases) !void { \\} , &[_][]const u8{ ":1:1: error: file exists in multiple modules", - ":1:1: note: root of module root.foo", + ":1:1: note: root of module foo", ":3:17: note: imported from module root", }); case.addSourceFile("foo.zig",