Commit 5eb5d523b5

Andrew Kelley <andrew@ziglang.org>
2023-10-09 05:58:04
give modules friendly names for error reporting
1 parent cd43977
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
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();
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;
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 => {
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
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'
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'
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",