Skip to content

Commit 8907dc8

Browse files
committed
Zcu: use shortest reference trace
The logic for computing reference traces was unintentionally finding the *longest* possible trace (approximately). I think I already tried to fix this before, but misunderstood how my own code works. Here, we fix it properly: by slightly reworking the logic to use one ArrayHashMap for both the result and the traversal queue, we trivially get a proper breadth-first traversal so that we can find the shortest possible reference trace for every referenced unit.
1 parent 4e9dd09 commit 8907dc8

File tree

1 file changed

+56
-49
lines changed

1 file changed

+56
-49
lines changed

src/Zcu.zig

Lines changed: 56 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ analysis_roots_len: usize = 0,
272272
/// This is the cached result of `Zcu.resolveReferences`. It is computed on-demand, and
273273
/// reset to `null` when any semantic analysis occurs (since this invalidates the data).
274274
/// Allocated into `gpa`.
275-
resolved_references: ?std.AutoHashMapUnmanaged(AnalUnit, ?ResolvedReference) = null,
275+
resolved_references: ?std.AutoArrayHashMapUnmanaged(AnalUnit, ?ResolvedReference) = null,
276276

277277
/// If `true`, then semantic analysis must not occur on this update due to AstGen errors.
278278
/// Essentially the entire pipeline after AstGen, including Sema, codegen, and link, is skipped.
@@ -3985,45 +3985,42 @@ pub const ResolvedReference = struct {
39853985
/// If an `AnalUnit` is not in the returned map, it is unreferenced.
39863986
/// The returned hashmap is owned by the `Zcu`, so should not be freed by the caller.
39873987
/// This hashmap is cached, so repeated calls to this function are cheap.
3988-
pub fn resolveReferences(zcu: *Zcu) !*const std.AutoHashMapUnmanaged(AnalUnit, ?ResolvedReference) {
3988+
pub fn resolveReferences(zcu: *Zcu) !*const std.AutoArrayHashMapUnmanaged(AnalUnit, ?ResolvedReference) {
39893989
if (zcu.resolved_references == null) {
39903990
zcu.resolved_references = try zcu.resolveReferencesInner();
39913991
}
39923992
return &zcu.resolved_references.?;
39933993
}
3994-
fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?ResolvedReference) {
3994+
fn resolveReferencesInner(zcu: *Zcu) !std.AutoArrayHashMapUnmanaged(AnalUnit, ?ResolvedReference) {
39953995
const gpa = zcu.gpa;
39963996
const comp = zcu.comp;
39973997
const ip = &zcu.intern_pool;
39983998

3999-
var result: std.AutoHashMapUnmanaged(AnalUnit, ?ResolvedReference) = .empty;
4000-
errdefer result.deinit(gpa);
4001-
4002-
var checked_types: std.AutoArrayHashMapUnmanaged(InternPool.Index, void) = .empty;
4003-
var type_queue: std.AutoArrayHashMapUnmanaged(InternPool.Index, ?ResolvedReference) = .empty;
4004-
var unit_queue: std.AutoArrayHashMapUnmanaged(AnalUnit, ?ResolvedReference) = .empty;
3999+
var units: std.AutoArrayHashMapUnmanaged(AnalUnit, ?ResolvedReference) = .empty;
4000+
var types: std.AutoArrayHashMapUnmanaged(InternPool.Index, ?ResolvedReference) = .empty;
40054001
defer {
4006-
checked_types.deinit(gpa);
4007-
type_queue.deinit(gpa);
4008-
unit_queue.deinit(gpa);
4002+
units.deinit(gpa);
4003+
types.deinit(gpa);
40094004
}
40104005

4011-
// This is not a sufficient size, but a lower bound.
4012-
try result.ensureTotalCapacity(gpa, @intCast(zcu.reference_table.count()));
4006+
// This is not a sufficient size, but an approximate lower bound.
4007+
try units.ensureTotalCapacity(gpa, @intCast(zcu.reference_table.count()));
40134008

4014-
try type_queue.ensureTotalCapacity(gpa, zcu.analysis_roots_len);
4009+
try types.ensureTotalCapacity(gpa, zcu.analysis_roots_len);
40154010
for (zcu.analysisRoots()) |mod| {
40164011
const file = zcu.module_roots.get(mod).?.unwrap() orelse continue;
40174012
const root_ty = zcu.fileRootType(file);
40184013
if (root_ty == .none) continue;
4019-
type_queue.putAssumeCapacityNoClobber(root_ty, null);
4014+
types.putAssumeCapacityNoClobber(root_ty, null);
40204015
}
40214016

4017+
var unit_idx: usize = 0;
4018+
var type_idx: usize = 0;
40224019
while (true) {
4023-
if (type_queue.pop()) |kv| {
4024-
const ty = kv.key;
4025-
const referencer = kv.value;
4026-
try checked_types.putNoClobber(gpa, ty, {});
4020+
if (type_idx < types.count()) {
4021+
const ty = types.keys()[type_idx];
4022+
const referencer = types.values()[type_idx];
4023+
type_idx += 1;
40274024

40284025
log.debug("handle type '{f}'", .{Type.fromInterned(ty).containerTypeName(ip).fmt(ip)});
40294026

@@ -4037,8 +4034,7 @@ fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?Resolv
40374034
if (has_resolution) {
40384035
// this should only be referenced by the type
40394036
const unit: AnalUnit = .wrap(.{ .type = ty });
4040-
assert(!result.contains(unit));
4041-
try unit_queue.putNoClobber(gpa, unit, referencer);
4037+
try units.putNoClobber(gpa, unit, referencer);
40424038
}
40434039

40444040
// If this is a union with a generated tag, its tag type is automatically referenced.
@@ -4047,9 +4043,8 @@ fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?Resolv
40474043
const tag_ty = union_obj.enum_tag_ty;
40484044
if (tag_ty != .none) {
40494045
if (ip.indexToKey(tag_ty).enum_type == .generated_tag) {
4050-
if (!checked_types.contains(tag_ty)) {
4051-
try type_queue.put(gpa, tag_ty, referencer);
4052-
}
4046+
const gop = try types.getOrPut(gpa, tag_ty);
4047+
if (!gop.found_existing) gop.value_ptr.* = referencer;
40534048
}
40544049
}
40554050
}
@@ -4060,12 +4055,13 @@ fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?Resolv
40604055
for (zcu.namespacePtr(ns).comptime_decls.items) |cu| {
40614056
// `comptime` decls are always analyzed.
40624057
const unit: AnalUnit = .wrap(.{ .@"comptime" = cu });
4063-
if (!result.contains(unit)) {
4058+
const gop = try units.getOrPut(gpa, unit);
4059+
if (!gop.found_existing) {
40644060
log.debug("type '{f}': ref comptime %{}", .{
40654061
Type.fromInterned(ty).containerTypeName(ip).fmt(ip),
40664062
@intFromEnum(ip.getComptimeUnit(cu).zir_index.resolve(ip) orelse continue),
40674063
});
4068-
try unit_queue.put(gpa, unit, referencer);
4064+
gop.value_ptr.* = referencer;
40694065
}
40704066
}
40714067
for (zcu.namespacePtr(ns).test_decls.items) |nav_id| {
@@ -4092,14 +4088,20 @@ fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?Resolv
40924088
},
40934089
};
40944090
if (want_analysis) {
4095-
log.debug("type '{f}': ref test %{}", .{
4096-
Type.fromInterned(ty).containerTypeName(ip).fmt(ip),
4097-
@intFromEnum(inst_info.inst),
4098-
});
4099-
try unit_queue.put(gpa, .wrap(.{ .nav_val = nav_id }), referencer);
4091+
{
4092+
const gop = try units.getOrPut(gpa, .wrap(.{ .nav_val = nav_id }));
4093+
if (!gop.found_existing) {
4094+
log.debug("type '{f}': ref test %{}", .{
4095+
Type.fromInterned(ty).containerTypeName(ip).fmt(ip),
4096+
@intFromEnum(inst_info.inst),
4097+
});
4098+
gop.value_ptr.* = referencer;
4099+
}
4100+
}
41004101
// Non-fatal AstGen errors could mean this test decl failed
41014102
if (nav.status == .fully_resolved) {
4102-
try unit_queue.put(gpa, .wrap(.{ .func = nav.status.fully_resolved.val }), referencer);
4103+
const gop = try units.getOrPut(gpa, .wrap(.{ .func = nav.status.fully_resolved.val }));
4104+
if (!gop.found_existing) gop.value_ptr.* = referencer;
41034105
}
41044106
}
41054107
}
@@ -4110,12 +4112,13 @@ fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?Resolv
41104112
const decl = file.zir.?.getDeclaration(inst_info.inst);
41114113
if (decl.linkage == .@"export") {
41124114
const unit: AnalUnit = .wrap(.{ .nav_val = nav });
4113-
if (!result.contains(unit)) {
4115+
const gop = try units.getOrPut(gpa, unit);
4116+
if (!gop.found_existing) {
41144117
log.debug("type '{f}': ref named %{}", .{
41154118
Type.fromInterned(ty).containerTypeName(ip).fmt(ip),
41164119
@intFromEnum(inst_info.inst),
41174120
});
4118-
try unit_queue.put(gpa, unit, referencer);
4121+
gop.value_ptr.* = referencer;
41194122
}
41204123
}
41214124
}
@@ -4126,20 +4129,21 @@ fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?Resolv
41264129
const decl = file.zir.?.getDeclaration(inst_info.inst);
41274130
if (decl.linkage == .@"export") {
41284131
const unit: AnalUnit = .wrap(.{ .nav_val = nav });
4129-
if (!result.contains(unit)) {
4132+
const gop = try units.getOrPut(gpa, unit);
4133+
if (!gop.found_existing) {
41304134
log.debug("type '{f}': ref named %{}", .{
41314135
Type.fromInterned(ty).containerTypeName(ip).fmt(ip),
41324136
@intFromEnum(inst_info.inst),
41334137
});
4134-
try unit_queue.put(gpa, unit, referencer);
4138+
gop.value_ptr.* = referencer;
41354139
}
41364140
}
41374141
}
41384142
continue;
41394143
}
4140-
if (unit_queue.pop()) |kv| {
4141-
const unit = kv.key;
4142-
try result.putNoClobber(gpa, unit, kv.value);
4144+
if (unit_idx < units.count()) {
4145+
const unit = units.keys()[unit_idx];
4146+
unit_idx += 1;
41434147

41444148
// `nav_val` and `nav_ty` reference each other *implicitly* to save memory.
41454149
queue_paired: {
@@ -4148,8 +4152,9 @@ fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?Resolv
41484152
.nav_ty => |n| .{ .nav_val = n },
41494153
.@"comptime", .type, .func, .memoized_state => break :queue_paired,
41504154
});
4151-
if (result.contains(other)) break :queue_paired;
4152-
try unit_queue.put(gpa, other, kv.value); // same reference location
4155+
const gop = try units.getOrPut(gpa, other);
4156+
if (gop.found_existing) break :queue_paired;
4157+
gop.value_ptr.* = units.values()[unit_idx]; // same reference location
41534158
}
41544159

41554160
log.debug("handle unit '{f}'", .{zcu.fmtAnalUnit(unit)});
@@ -4159,16 +4164,17 @@ fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?Resolv
41594164
var ref_idx = first_ref_idx;
41604165
while (ref_idx != std.math.maxInt(u32)) {
41614166
const ref = zcu.all_references.items[ref_idx];
4162-
if (!result.contains(ref.referenced)) {
4167+
const gop = try units.getOrPut(gpa, ref.referenced);
4168+
if (!gop.found_existing) {
41634169
log.debug("unit '{f}': ref unit '{f}'", .{
41644170
zcu.fmtAnalUnit(unit),
41654171
zcu.fmtAnalUnit(ref.referenced),
41664172
});
4167-
try unit_queue.put(gpa, ref.referenced, .{
4173+
gop.value_ptr.* = .{
41684174
.referencer = unit,
41694175
.src = ref.src,
41704176
.inline_frame = ref.inline_frame,
4171-
});
4177+
};
41724178
}
41734179
ref_idx = ref.next;
41744180
}
@@ -4178,16 +4184,17 @@ fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?Resolv
41784184
var ref_idx = first_ref_idx;
41794185
while (ref_idx != std.math.maxInt(u32)) {
41804186
const ref = zcu.all_type_references.items[ref_idx];
4181-
if (!checked_types.contains(ref.referenced)) {
4187+
const gop = try types.getOrPut(gpa, ref.referenced);
4188+
if (!gop.found_existing) {
41824189
log.debug("unit '{f}': ref type '{f}'", .{
41834190
zcu.fmtAnalUnit(unit),
41844191
Type.fromInterned(ref.referenced).containerTypeName(ip).fmt(ip),
41854192
});
4186-
try type_queue.put(gpa, ref.referenced, .{
4193+
gop.value_ptr.* = .{
41874194
.referencer = unit,
41884195
.src = ref.src,
41894196
.inline_frame = .none,
4190-
});
4197+
};
41914198
}
41924199
ref_idx = ref.next;
41934200
}
@@ -4197,7 +4204,7 @@ fn resolveReferencesInner(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, ?Resolv
41974204
break;
41984205
}
41994206

4200-
return result;
4207+
return units.move();
42014208
}
42024209

42034210
pub fn analysisRoots(zcu: *Zcu) []*Package.Module {

0 commit comments

Comments
 (0)