Skip to content

Commit 3d8e760

Browse files
committed
Zcu: fix nav_ty dependency on nav_val
In the case where a declaration has no type annotation, the interaction between resolution of `nav_ty` and `nav_val` is a little fiddly because of the fact that resolving `nav_val` actually implicitly resolves the type as well. This means `nav_ty` never gets an opporunity to mark its dependency on the `nav_val`. So, `ensureNavValUpToDate` needs to be the one to do it. It can't do it too early, though; otherwise, our marking of dependees as out-of-date/up-to-date will go wrong. Resolves: #23959
1 parent aeed5f9 commit 3d8e760

File tree

2 files changed

+75
-28
lines changed

2 files changed

+75
-28
lines changed

src/Zcu/PerThread.zig

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,35 @@ pub fn ensureNavValUpToDate(pt: Zcu.PerThread, nav_id: InternPool.Nav.Index) Zcu
10221022
}
10231023
}
10241024

1025+
// If there isn't a type annotation, then we have also just resolved the type. That means the
1026+
// the type is up-to-date, so it won't have the chance to mark its own dependency on the value;
1027+
// we must do that ourselves.
1028+
type_deps_on_val: {
1029+
const inst_resolved = nav.analysis.?.zir_index.resolveFull(ip) orelse break :type_deps_on_val;
1030+
const file = zcu.fileByIndex(inst_resolved.file);
1031+
const zir_decl = file.zir.?.getDeclaration(inst_resolved.inst);
1032+
if (zir_decl.type_body != null) break :type_deps_on_val;
1033+
// The type does indeed depend on the value. We are responsible for populating all state of
1034+
// the `nav_ty`, including exports, references, errors, and dependencies.
1035+
const ty_unit: AnalUnit = .wrap(.{ .nav_ty = nav_id });
1036+
const ty_was_outdated = zcu.outdated.swapRemove(ty_unit) or
1037+
zcu.potentially_outdated.swapRemove(ty_unit);
1038+
if (ty_was_outdated) {
1039+
_ = zcu.outdated_ready.swapRemove(ty_unit);
1040+
zcu.deleteUnitExports(ty_unit);
1041+
zcu.deleteUnitReferences(ty_unit);
1042+
zcu.deleteUnitCompileLogs(ty_unit);
1043+
if (zcu.failed_analysis.fetchSwapRemove(ty_unit)) |kv| {
1044+
kv.value.destroy(gpa);
1045+
}
1046+
_ = zcu.transitive_failed_analysis.swapRemove(ty_unit);
1047+
ip.removeDependenciesForDepender(gpa, ty_unit);
1048+
}
1049+
try pt.addDependency(ty_unit, .{ .nav_val = nav_id });
1050+
if (new_failed) try zcu.transitive_failed_analysis.put(gpa, ty_unit, {});
1051+
if (ty_was_outdated) try zcu.markDependeeOutdated(.marked_po, .{ .nav_ty = nav_id });
1052+
}
1053+
10251054
if (new_failed) return error.AnalysisFail;
10261055
}
10271056

@@ -1266,14 +1295,6 @@ fn analyzeNavVal(pt: Zcu.PerThread, nav_id: InternPool.Nav.Index) Zcu.CompileErr
12661295
// Mark the unit as completed before evaluating the export!
12671296
assert(zcu.analysis_in_progress.swapRemove(anal_unit));
12681297

1269-
if (zir_decl.type_body == null) {
1270-
// In this situation, it's possible that we were triggered by `analyzeNavType` up the stack. In that
1271-
// case, we must also signal that the *type* is now populated to make this export behave correctly.
1272-
// An alternative strategy would be to just put something on the job queue to perform the export, but
1273-
// this is a little more straightforward, if perhaps less elegant.
1274-
_ = zcu.analysis_in_progress.swapRemove(.wrap(.{ .nav_ty = nav_id }));
1275-
}
1276-
12771298
if (zir_decl.linkage == .@"export") {
12781299
const export_src = block.src(.{ .token_offset = @enumFromInt(@intFromBool(zir_decl.is_pub)) });
12791300
const name_slice = zir.nullTerminatedString(zir_decl.name);
@@ -1314,6 +1335,18 @@ pub fn ensureNavTypeUpToDate(pt: Zcu.PerThread, nav_id: InternPool.Nav.Index) Zc
13141335

13151336
log.debug("ensureNavTypeUpToDate {}", .{zcu.fmtAnalUnit(anal_unit)});
13161337

1338+
const type_resolved_by_value: bool = from_val: {
1339+
const analysis = nav.analysis orelse break :from_val false;
1340+
const inst_resolved = analysis.zir_index.resolveFull(ip) orelse break :from_val false;
1341+
const file = zcu.fileByIndex(inst_resolved.file);
1342+
const zir_decl = file.zir.?.getDeclaration(inst_resolved.inst);
1343+
break :from_val zir_decl.type_body == null;
1344+
};
1345+
if (type_resolved_by_value) {
1346+
// Logic at the end of `ensureNavValUpToDate` is directly responsible for populating our state.
1347+
return pt.ensureNavValUpToDate(nav_id);
1348+
}
1349+
13171350
// Determine whether or not this `Nav`'s type is outdated. This also includes checking if the
13181351
// status is `.unresolved`, which indicates that the value is outdated because it has *never*
13191352
// been analyzed so far.
@@ -1421,6 +1454,10 @@ fn analyzeNavType(pt: Zcu.PerThread, nav_id: InternPool.Nav.Index) Zcu.CompileEr
14211454
try zcu.analysis_in_progress.put(gpa, anal_unit, {});
14221455
defer _ = zcu.analysis_in_progress.swapRemove(anal_unit);
14231456

1457+
const zir_decl = zir.getDeclaration(inst_resolved.inst);
1458+
assert(old_nav.is_usingnamespace == (zir_decl.kind == .@"usingnamespace"));
1459+
const type_body = zir_decl.type_body.?;
1460+
14241461
var analysis_arena: std.heap.ArenaAllocator = .init(gpa);
14251462
defer analysis_arena.deinit();
14261463

@@ -1460,33 +1497,13 @@ fn analyzeNavType(pt: Zcu.PerThread, nav_id: InternPool.Nav.Index) Zcu.CompileEr
14601497
};
14611498
defer block.instructions.deinit(gpa);
14621499

1463-
const zir_decl = zir.getDeclaration(inst_resolved.inst);
1464-
assert(old_nav.is_usingnamespace == (zir_decl.kind == .@"usingnamespace"));
1465-
14661500
const ty_src = block.src(.{ .node_offset_var_decl_ty = .zero });
14671501

14681502
block.comptime_reason = .{ .reason = .{
14691503
.src = ty_src,
14701504
.r = .{ .simple = .type },
14711505
} };
14721506

1473-
const type_body = zir_decl.type_body orelse {
1474-
// The type of this `Nav` is inferred from the value.
1475-
// In other words, this `nav_ty` depends on the corresponding `nav_val`.
1476-
try sema.declareDependency(.{ .nav_val = nav_id });
1477-
try pt.ensureNavValUpToDate(nav_id);
1478-
// Note that the above call, if it did any work, has removed our `analysis_in_progress` entry for us.
1479-
// (Our `defer` will run anyway, but it does nothing in this case.)
1480-
1481-
// There's not a great way for us to know whether the type actually changed.
1482-
// For instance, perhaps the `nav_val` was already up-to-date, but this `nav_ty` is being
1483-
// analyzed because this declaration had a type annotation on the *previous* update.
1484-
// However, such cases are rare, and it's not unreasonable to re-analyze in them; and in
1485-
// other cases where we get here, it's because the `nav_val` was already re-analyzed and
1486-
// is outdated.
1487-
return .{ .type_changed = true };
1488-
};
1489-
14901507
const resolved_ty: Type = ty: {
14911508
const uncoerced_type_ref = try sema.resolveInlineBody(&block, type_body, inst_resolved.inst);
14921509
const type_ref = try sema.coerce(&block, .type, uncoerced_type_ref, ty_src);
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#target=x86_64-linux-selfhosted
2+
#target=x86_64-linux-cbe
3+
#target=x86_64-windows-cbe
4+
#target=wasm32-wasi-selfhosted
5+
#update=initial version
6+
#file=main.zig
7+
const foo = @as(u8, 123);
8+
comptime {
9+
// depends on value of `foo`
10+
if (foo != 123) unreachable;
11+
}
12+
comptime {
13+
// depends on type of `foo`
14+
if (@TypeOf(&foo) != *const u8) unreachable;
15+
}
16+
pub fn main() void {}
17+
#expect_stdout=""
18+
#update=change the type
19+
#file=main.zig
20+
const foo = @as(u16, 123);
21+
comptime {
22+
// depends on value of `foo`
23+
if (foo != 123) unreachable;
24+
}
25+
comptime {
26+
// depends on type of `foo`
27+
if (@TypeOf(&foo) != *const u8) unreachable;
28+
}
29+
pub fn main() void {}
30+
#expect_error=main.zig:8:37: error: reached unreachable code

0 commit comments

Comments
 (0)