authorgravatar for mlugg@mlugg.co.ukMatthew Lugg <mlugg@mlugg.co.uk> 2023-01-22 16:12:40+00:00
committergravatar for mlugg@mlugg.co.ukMatthew Lugg <mlugg@mlugg.co.uk> 2023-01-22 19:00:03+00:00
log6d71d79dc27ddd6f66913a34fd6cd40691a8c959
treed6fd9f56beb9ba26545a12f5fa827d91aec81972
parentc0284e242f7d78955204dc8a627fecd45aa5e521
signature Commit is signed but in an unrecognized format.

Package: store package name directly

By @Vexu's suggestion, since fetching the name from the parent package is error-prone and complex, and optimising Package for size isn't really a priority.

6 files changed, 54 insertions(+), 46 deletions(-)

src/Compilation.zig+10-4
......@@ -1610,6 +1610,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
16101610
16111611 const builtin_pkg = try Package.createWithDir(
16121612 gpa,
1613 "builtin",
16131614 zig_cache_artifact_directory,
16141615 null,
16151616 "builtin.zig",
......@@ -1618,6 +1619,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
16181619
16191620 const std_pkg = try Package.createWithDir(
16201621 gpa,
1622 "std",
16211623 options.zig_lib_directory,
16221624 "std",
16231625 "std.zig",
......@@ -1625,11 +1627,14 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
16251627 errdefer std_pkg.destroy(gpa);
16261628
16271629 const root_pkg = if (options.is_test) root_pkg: {
1630 // TODO: we currently have two packages named 'root' here, which is weird. This
1631 // should be changed as part of the resolution of #12201
16281632 const test_pkg = if (options.test_runner_path) |test_runner|
1629 try Package.create(gpa, null, test_runner)
1633 try Package.create(gpa, "root", null, test_runner)
16301634 else
16311635 try Package.createWithDir(
16321636 gpa,
1637 "root",
16331638 options.zig_lib_directory,
16341639 null,
16351640 "test_runner.zig",
......@@ -1640,9 +1645,9 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
16401645 } else main_pkg;
16411646 errdefer if (options.is_test) root_pkg.destroy(gpa);
16421647
1643 try main_pkg.addAndAdopt(gpa, "builtin", builtin_pkg);
1644 try main_pkg.add(gpa, "root", root_pkg);
1645 try main_pkg.addAndAdopt(gpa, "std", std_pkg);
1648 try main_pkg.addAndAdopt(gpa, builtin_pkg);
1649 try main_pkg.add(gpa, root_pkg);
1650 try main_pkg.addAndAdopt(gpa, std_pkg);
16461651
16471652 const main_pkg_is_std = m: {
16481653 const std_path = try std.fs.path.resolve(arena, &[_][]const u8{
......@@ -5320,6 +5325,7 @@ fn buildOutputFromZig(
53205325 var main_pkg: Package = .{
53215326 .root_src_directory = comp.zig_lib_directory,
53225327 .root_src_path = src_basename,
5328 .name = "root",
53235329 };
53245330 defer main_pkg.deinitTable(comp.gpa);
53255331 const root_name = src_basename[0 .. src_basename.len - std.fs.path.extension(src_basename).len];
src/Module.zig-5
......@@ -3220,16 +3220,11 @@ pub fn deinit(mod: *Module) void {
32203220 // The callsite of `Compilation.create` owns the `main_pkg`, however
32213221 // Module owns the builtin and std packages that it adds.
32223222 if (mod.main_pkg.table.fetchRemove("builtin")) |kv| {
3223 gpa.free(kv.key);
32243223 kv.value.destroy(gpa);
32253224 }
32263225 if (mod.main_pkg.table.fetchRemove("std")) |kv| {
3227 gpa.free(kv.key);
32283226 kv.value.destroy(gpa);
32293227 }
3230 if (mod.main_pkg.table.fetchRemove("root")) |kv| {
3231 gpa.free(kv.key);
3232 }
32333228 if (mod.root_pkg != mod.main_pkg) {
32343229 mod.root_pkg.destroy(gpa);
32353230 }
src/Package.zig+26-13
......@@ -24,10 +24,13 @@ table: Table = .{},
2424parent: ?*Package = null,
2525/// Whether to free `root_src_directory` on `destroy`.
2626root_src_directory_owned: bool = false,
27/// This information can be recovered from 'table', but it's more convenient to store on the package.
28name: []const u8,
2729
2830/// Allocate a Package. No references to the slices passed are kept.
2931pub fn create(
3032 gpa: Allocator,
33 name: []const u8,
3134 /// Null indicates the current working directory
3235 root_src_dir_path: ?[]const u8,
3336 /// Relative to root_src_dir_path
......@@ -42,6 +45,9 @@ pub fn create(
4245 const owned_src_path = try gpa.dupe(u8, root_src_path);
4346 errdefer gpa.free(owned_src_path);
4447
48 const owned_name = try gpa.dupe(u8, name);
49 errdefer gpa.free(owned_name);
50
4551 ptr.* = .{
4652 .root_src_directory = .{
4753 .path = owned_dir_path,
......@@ -49,6 +55,7 @@ pub fn create(
4955 },
5056 .root_src_path = owned_src_path,
5157 .root_src_directory_owned = true,
58 .name = owned_name,
5259 };
5360
5461 return ptr;
......@@ -56,6 +63,7 @@ pub fn create(
5663
5764pub fn createWithDir(
5865 gpa: Allocator,
66 name: []const u8,
5967 directory: Compilation.Directory,
6068 /// Relative to `directory`. If null, means `directory` is the root src dir
6169 /// and is owned externally.
......@@ -69,6 +77,9 @@ pub fn createWithDir(
6977 const owned_src_path = try gpa.dupe(u8, root_src_path);
7078 errdefer gpa.free(owned_src_path);
7179
80 const owned_name = try gpa.dupe(u8, name);
81 errdefer gpa.free(owned_name);
82
7283 if (root_src_dir_path) |p| {
7384 const owned_dir_path = try directory.join(gpa, &[1][]const u8{p});
7485 errdefer gpa.free(owned_dir_path);
......@@ -80,12 +91,14 @@ pub fn createWithDir(
8091 },
8192 .root_src_directory_owned = true,
8293 .root_src_path = owned_src_path,
94 .name = owned_name,
8395 };
8496 } else {
8597 ptr.* = .{
8698 .root_src_directory = directory,
8799 .root_src_directory_owned = false,
88100 .root_src_path = owned_src_path,
101 .name = owned_name,
89102 };
90103 }
91104 return ptr;
......@@ -95,6 +108,7 @@ pub fn createWithDir(
95108/// inside its table; the caller is responsible for calling destroy() on them.
96109pub fn destroy(pkg: *Package, gpa: Allocator) void {
97110 gpa.free(pkg.root_src_path);
111 gpa.free(pkg.name);
98112
99113 if (pkg.root_src_directory_owned) {
100114 // If root_src_directory.path is null then the handle is the cwd()
......@@ -111,24 +125,18 @@ pub fn destroy(pkg: *Package, gpa: Allocator) void {
111125
112126/// Only frees memory associated with the table.
113127pub fn deinitTable(pkg: *Package, gpa: Allocator) void {
114 var it = pkg.table.keyIterator();
115 while (it.next()) |key| {
116 gpa.free(key.*);
117 }
118
119128 pkg.table.deinit(gpa);
120129}
121130
122pub fn add(pkg: *Package, gpa: Allocator, name: []const u8, package: *Package) !void {
131pub fn add(pkg: *Package, gpa: Allocator, package: *Package) !void {
123132 try pkg.table.ensureUnusedCapacity(gpa, 1);
124 const name_dupe = try gpa.dupe(u8, name);
125 pkg.table.putAssumeCapacityNoClobber(name_dupe, package);
133 pkg.table.putAssumeCapacityNoClobber(package.name, package);
126134}
127135
128pub fn addAndAdopt(parent: *Package, gpa: Allocator, name: []const u8, child: *Package) !void {
136pub fn addAndAdopt(parent: *Package, gpa: Allocator, child: *Package) !void {
129137 assert(child.parent == null); // make up your mind, who is the parent??
130138 child.parent = parent;
131 return parent.add(gpa, name, child);
139 return parent.add(gpa, child);
132140}
133141
134142pub const build_zig_basename = "build.zig";
......@@ -237,7 +245,7 @@ pub fn fetchAndAddDependencies(
237245 sub_prefix,
238246 );
239247
240 try addAndAdopt(pkg, gpa, fqn, sub_pkg);
248 try addAndAdopt(pkg, gpa, sub_pkg);
241249
242250 try dependencies_source.writer().print(" pub const {s} = @import(\"{}\");\n", .{
243251 std.zig.fmtId(fqn), std.zig.fmtEscapes(fqn),
......@@ -249,6 +257,7 @@ pub fn fetchAndAddDependencies(
249257
250258pub fn createFilePkg(
251259 gpa: Allocator,
260 name: []const u8,
252261 cache_directory: Compilation.Directory,
253262 basename: []const u8,
254263 contents: []const u8,
......@@ -269,7 +278,7 @@ pub fn createFilePkg(
269278 const o_dir_sub_path = "o" ++ fs.path.sep_str ++ hex_digest;
270279 try renameTmpIntoCache(cache_directory.handle, tmp_dir_sub_path, o_dir_sub_path);
271280
272 return createWithDir(gpa, cache_directory, o_dir_sub_path, basename);
281 return createWithDir(gpa, name, cache_directory, o_dir_sub_path, basename);
273282}
274283
275284fn fetchAndUnpack(
......@@ -312,6 +321,9 @@ fn fetchAndUnpack(
312321 const owned_src_path = try gpa.dupe(u8, build_zig_basename);
313322 errdefer gpa.free(owned_src_path);
314323
324 const owned_name = try gpa.dupe(u8, fqn);
325 errdefer gpa.free(owned_name);
326
315327 const build_root = try global_cache_directory.join(gpa, &.{pkg_dir_sub_path});
316328 errdefer gpa.free(build_root);
317329
......@@ -326,6 +338,7 @@ fn fetchAndUnpack(
326338 },
327339 .root_src_directory_owned = true,
328340 .root_src_path = owned_src_path,
341 .name = owned_name,
329342 };
330343
331344 return ptr;
......@@ -414,7 +427,7 @@ fn fetchAndUnpack(
414427 std.zig.fmtId(fqn), std.zig.fmtEscapes(build_root),
415428 });
416429
417 return createWithDir(gpa, global_cache_directory, pkg_dir_sub_path, build_zig_basename);
430 return createWithDir(gpa, fqn, global_cache_directory, pkg_dir_sub_path, build_zig_basename);
418431}
419432
420433fn reportError(
src/Sema.zig+2-14
......@@ -5211,6 +5211,7 @@ fn zirCImport(sema: *Sema, parent_block: *Block, inst: Zir.Inst.Index) CompileEr
52115211 }
52125212 const c_import_pkg = Package.create(
52135213 sema.gpa,
5214 "c_import", // TODO: should we make this unique?
52145215 null,
52155216 c_import_res.out_zig_path,
52165217 ) catch |err| switch (err) {
......@@ -11663,20 +11664,7 @@ fn zirImport(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.
1166311664 },
1166411665 error.PackageNotFound => {
1166511666 const cur_pkg = block.getFileScope().pkg;
11666 const parent = if (cur_pkg == sema.mod.main_pkg or cur_pkg == sema.mod.root_pkg)
11667 "root"
11668 else if (cur_pkg.parent) |parent| blk: {
11669 var it = parent.table.iterator();
11670 while (it.next()) |pkg| {
11671 if (pkg.value_ptr.* == cur_pkg) {
11672 break :blk pkg.key_ptr.*;
11673 }
11674 }
11675 unreachable;
11676 } else {
11677 return sema.fail(block, operand_src, "no package named '{s}' available", .{operand});
11678 };
11679 return sema.fail(block, operand_src, "no package named '{s}' available within package '{s}'", .{ operand, parent });
11667 return sema.fail(block, operand_src, "no package named '{s}' available within package '{s}'", .{ operand, cur_pkg.name });
1168011668 },
1168111669 else => {
1168211670 // TODO: these errors are file system errors; make sure an update() will
src/main.zig+15-10
......@@ -857,6 +857,7 @@ fn buildOutputType(
857857 var pkg_tree_root: Package = .{
858858 .root_src_directory = .{ .path = null, .handle = fs.cwd() },
859859 .root_src_path = &[0]u8{},
860 .name = &[0]u8{},
860861 };
861862 defer freePkgTree(gpa, &pkg_tree_root, false);
862863 var cur_pkg: *Package = &pkg_tree_root;
......@@ -947,6 +948,7 @@ fn buildOutputType(
947948
948949 const new_cur_pkg = Package.create(
949950 gpa,
951 pkg_name,
950952 fs.path.dirname(pkg_path),
951953 fs.path.basename(pkg_path),
952954 ) catch |err| {
......@@ -958,7 +960,7 @@ fn buildOutputType(
958960 } else if (cur_pkg.table.get(pkg_name)) |prev| {
959961 fatal("unable to add package '{s}' -> '{s}': already exists as '{s}", .{ pkg_name, pkg_path, prev.root_src_path });
960962 }
961 try cur_pkg.addAndAdopt(gpa, pkg_name, new_cur_pkg);
963 try cur_pkg.addAndAdopt(gpa, new_cur_pkg);
962964 cur_pkg = new_cur_pkg;
963965 } else if (mem.eql(u8, arg, "--pkg-end")) {
964966 cur_pkg = cur_pkg.parent orelse
......@@ -2841,14 +2843,14 @@ fn buildOutputType(
28412843 if (main_pkg_path) |unresolved_main_pkg_path| {
28422844 const p = try introspect.resolvePath(arena, unresolved_main_pkg_path);
28432845 if (p.len == 0) {
2844 break :blk try Package.create(gpa, null, src_path);
2846 break :blk try Package.create(gpa, "root", null, src_path);
28452847 } else {
28462848 const rel_src_path = try fs.path.relative(arena, p, src_path);
2847 break :blk try Package.create(gpa, p, rel_src_path);
2849 break :blk try Package.create(gpa, "root", p, rel_src_path);
28482850 }
28492851 } else {
28502852 const root_src_dir_path = fs.path.dirname(src_path);
2851 break :blk Package.create(gpa, root_src_dir_path, fs.path.basename(src_path)) catch |err| {
2853 break :blk Package.create(gpa, "root", root_src_dir_path, fs.path.basename(src_path)) catch |err| {
28522854 if (root_src_dir_path) |p| {
28532855 fatal("unable to open '{s}': {s}", .{ p, @errorName(err) });
28542856 } else {
......@@ -4093,6 +4095,7 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi
40934095 var main_pkg: Package = .{
40944096 .root_src_directory = zig_lib_directory,
40954097 .root_src_path = "build_runner.zig",
4098 .name = "root",
40964099 };
40974100
40984101 if (!build_options.omit_pkg_fetching_code) {
......@@ -4133,20 +4136,22 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi
41334136
41344137 const deps_pkg = try Package.createFilePkg(
41354138 gpa,
4139 "@dependencies",
41364140 local_cache_directory,
41374141 "dependencies.zig",
41384142 dependencies_source.items,
41394143 );
41404144
41414145 mem.swap(Package.Table, &main_pkg.table, &deps_pkg.table);
4142 try main_pkg.addAndAdopt(gpa, "@dependencies", deps_pkg);
4146 try main_pkg.addAndAdopt(gpa, deps_pkg);
41434147 }
41444148
41454149 var build_pkg: Package = .{
41464150 .root_src_directory = build_directory,
41474151 .root_src_path = build_zig_basename,
4152 .name = "@build",
41484153 };
4149 try main_pkg.addAndAdopt(gpa, "@build", &build_pkg);
4154 try main_pkg.addAndAdopt(gpa, &build_pkg);
41504155
41514156 const comp = Compilation.create(gpa, .{
41524157 .zig_lib_directory = zig_lib_directory,
......@@ -4381,7 +4386,7 @@ pub fn cmdFmt(gpa: Allocator, arena: Allocator, args: []const []const u8) !void
43814386 .root_decl = .none,
43824387 };
43834388
4384 file.pkg = try Package.create(gpa, null, file.sub_file_path);
4389 file.pkg = try Package.create(gpa, "root", null, file.sub_file_path);
43854390 defer file.pkg.destroy(gpa);
43864391
43874392 file.zir = try AstGen.generate(gpa, file.tree);
......@@ -4591,7 +4596,7 @@ fn fmtPathFile(
45914596 .root_decl = .none,
45924597 };
45934598
4594 file.pkg = try Package.create(fmt.gpa, null, file.sub_file_path);
4599 file.pkg = try Package.create(fmt.gpa, "root", null, file.sub_file_path);
45954600 defer file.pkg.destroy(fmt.gpa);
45964601
45974602 if (stat.size > max_src_size)
......@@ -5303,7 +5308,7 @@ pub fn cmdAstCheck(
53035308 file.stat.size = source.len;
53045309 }
53055310
5306 file.pkg = try Package.create(gpa, null, file.sub_file_path);
5311 file.pkg = try Package.create(gpa, "root", null, file.sub_file_path);
53075312 defer file.pkg.destroy(gpa);
53085313
53095314 file.tree = try std.zig.parse(gpa, file.source);
......@@ -5422,7 +5427,7 @@ pub fn cmdChangelist(
54225427 .root_decl = .none,
54235428 };
54245429
5425 file.pkg = try Package.create(gpa, null, file.sub_file_path);
5430 file.pkg = try Package.create(gpa, "root", null, file.sub_file_path);
54265431 defer file.pkg.destroy(gpa);
54275432
54285433 const source = try arena.allocSentinel(u8, @intCast(usize, stat.size), 0);
src/test.zig+1
......@@ -1497,6 +1497,7 @@ pub const TestContext = struct {
14971497 var main_pkg: Package = .{
14981498 .root_src_directory = .{ .path = tmp_dir_path, .handle = tmp.dir },
14991499 .root_src_path = tmp_src_path,
1500 .name = "root",
15001501 };
15011502 defer main_pkg.table.deinit(allocator);
15021503