Skip to content

Commit

Permalink
Fixes interaction between visited check & nested optionals for ZON ty…
Browse files Browse the repository at this point in the history 
…pe compatability
  • Loading branch information
@MasonRemaley
MasonRemaley committed Feb 2, 2025
1 parent 6275568 commit 196acb0
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 64 deletions.
61 changes: 44 additions & 17 deletions lib/std/zon/parse.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1148,18 +1148,20 @@ fn comptimeAssertCanParseType(T: type) void {
fn canParseType(T: type) bool {
comptime {
var visited: []type = &.{};
return canParseTypeInner(T, false, &visited);
return canParseTypeInner(T, &visited);
}
}

fn canParseTypeInner(T: type, is_opt: bool, visited: *[]type) bool {
// If we've already visited this type, then it must be supported, return true to avoid infinite
// recursion.
fn canParseTypeInner(T: type, visited: *[]type) bool {
// Check if we're a nested optional before we possibly return early due to recursion
if (isNestedOptional(T)) return false;

// Check/update the visited list to avoid infinite recursion
for (visited.*) |V| if (V == T) return true;
comptime var new_visited = visited.*[0..visited.len].* ++ .{T};
visited.* = &new_visited;

// Check if this type is supported, recursive if needed
// Check if this type is supported, recurse if needed
switch (@typeInfo(T)) {
.bool,
.int,
Expand All @@ -1185,35 +1187,53 @@ fn canParseTypeInner(T: type, is_opt: bool, visited: *[]type) bool {

.pointer => |pointer| {
switch (pointer.size) {
.one => return canParseTypeInner(pointer.child, is_opt, visited),
.slice => return canParseTypeInner(pointer.child, false, visited),
.one => return canParseTypeInner(pointer.child, visited),
.slice => return canParseTypeInner(pointer.child, visited),
.many, .c => return false,
}
},
.array => |array| return canParseTypeInner(array.child, false, visited),
.array => |array| return canParseTypeInner(array.child, visited),
.@"struct" => |@"struct"| {
inline for (@"struct".fields) |field| {
if (!field.is_comptime and !canParseTypeInner(field.type, false, visited)) {
if (!field.is_comptime and !canParseTypeInner(field.type, visited)) {
return false;
}
}
return true;
},
.optional => |optional| {
// Forbid nested optionals
if (is_opt) return false;
// Other optionals are fine
return canParseTypeInner(optional.child, true, visited);
},
.optional => |optional| return canParseTypeInner(optional.child, visited),
.@"union" => |@"union"| {
inline for (@"union".fields) |field| {
if (field.type != void and !canParseTypeInner(field.type, false, visited)) {
if (field.type != void and !canParseTypeInner(field.type, visited)) {
return false;
}
}
return true;
},
.vector => |vector| return canParseTypeInner(vector.child, false, visited),
.vector => |vector| return canParseTypeInner(vector.child, visited),
}
}

/// Returns true if this type is optional, and it contains another optional either directly or
/// through a series of single item pointers.
fn isNestedOptional(T: type) bool {
comptime switch (@typeInfo(T)) {
.optional => |optional| return isNestedOptionalInner(optional.child),
else => return false,
};
}

fn isNestedOptionalInner(T: type) bool {
switch (@typeInfo(T)) {
.pointer => |pointer| {
if (pointer.size == .one) {
return isNestedOptionalInner(pointer.child);
} else {
return false;
}
},
.optional => return true,
else => return false,
}
}

Expand Down Expand Up @@ -1248,6 +1268,13 @@ test "std.zon parse canParseType" {
try std.testing.expect(!comptime canParseType(*?*?*u8));
const Recursive = struct { foo: ?*@This() };
try std.testing.expect(comptime canParseType(Recursive));

// Make sure we validate nested optional before we early out due to already having seen
// a type recursion!
try std.testing.expect(!comptime canParseType(struct {
add_to_visited: ?u8,
retrieve_from_visited: ??u8,
}));
}

test "std.zon requiresAllocator" {
Expand Down
59 changes: 42 additions & 17 deletions lib/std/zon/stringify.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,19 @@ fn comptimeAssertCanSerializeType(T: type) void {

fn canSerializeType(T: type) bool {
var visited: []type = &.{};
return canSerializeTypeInner(T, false, &visited);
return canSerializeTypeInner(T, &visited);
}

fn canSerializeTypeInner(T: type, is_opt: bool, visited: *[]type) bool {
// If we've already visited this type, then it must be supported, return true to avoid infinite
// recursion.
fn canSerializeTypeInner(T: type, visited: *[]type) bool {
// Check if we're a nested optional before we possibly return early due to recursion
if (isNestedOptional(T)) return false;

// Check/update the visited list to avoid infinite recursion
for (visited.*) |V| if (V == T) return true;
comptime var new_visited = visited.*[0..visited.len].* ++ .{T};
visited.* = &new_visited;

// Check if this type is supported, recursive if needed
// Check if this type is supported, recurse if needed
switch (@typeInfo(T)) {
.bool,
.int,
Expand All @@ -172,34 +174,50 @@ fn canSerializeTypeInner(T: type, is_opt: bool, visited: *[]type) bool {

.pointer => |pointer| {
switch (pointer.size) {
.one => return canSerializeTypeInner(pointer.child, is_opt, visited),
.slice => return canSerializeTypeInner(pointer.child, false, visited),
.one => return canSerializeTypeInner(pointer.child, visited),
.slice => return canSerializeTypeInner(pointer.child, visited),
.many, .c => return false,
}
},
.array => |array| return canSerializeTypeInner(array.child, false, visited),
.array => |array| return canSerializeTypeInner(array.child, visited),
.@"struct" => |@"struct"| {
inline for (@"struct".fields) |field| {
if (!canSerializeTypeInner(field.type, false, visited)) return false;
if (!canSerializeTypeInner(field.type, visited)) return false;
}
return true;
},
.optional => |optional| {
// Forbid nested optionals
if (is_opt) return false;
// Other optionals are fine
return canSerializeTypeInner(optional.child, true, visited);
},
.optional => |optional| return canSerializeTypeInner(optional.child, visited),
.@"union" => |@"union"| {
if (@"union".tag_type == null) return false;
inline for (@"union".fields) |field| {
if (field.type != void and !canSerializeTypeInner(field.type, false, visited)) {
if (field.type != void and !canSerializeTypeInner(field.type, visited)) {
return false;
}
}
return true;
},
.vector => |vector| return canSerializeTypeInner(vector.child, false, visited),
.vector => |vector| return canSerializeTypeInner(vector.child, visited),
}
}

fn isNestedOptional(T: type) bool {
comptime switch (@typeInfo(T)) {
.optional => |optional| return isNestedOptionalInner(optional.child),
else => return false,
};
}

fn isNestedOptionalInner(T: type) bool {
switch (@typeInfo(T)) {
.pointer => |pointer| {
if (pointer.size == .one) {
return isNestedOptionalInner(pointer.child);
} else {
return false;
}
},
.optional => return true,
else => return false,
}
}

Expand Down Expand Up @@ -234,6 +252,13 @@ test "std.zon stringify canSerializeType" {
try std.testing.expect(!comptime canSerializeType(*?*?*u8));
const Recursive = struct { foo: ?*@This() };
try std.testing.expect(comptime canSerializeType(Recursive));

// Make sure we validate nested optional before we early out due to already having seen
// a type recursion!
try std.testing.expect(!comptime canSerializeType(struct {
add_to_visited: ?u8,
retrieve_from_visited: ??u8,
}));
}

test "std.zon typeIsRecursive" {
Expand Down
64 changes: 34 additions & 30 deletions src/Sema/LowerZon.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,33 +46,47 @@ pub fn lower(
.block = block,
};

// XXX: add tests covering this, including recursive types!
try lower_zon.checkParseType(res_ty);
try lower_zon.checkType(res_ty);

return lower_zon.lowerExpr(.root, res_ty);
}

fn checkParseType(self: *LowerZon, ty: Type) !void {
fn checkType(self: *LowerZon, ty: Type) !void {
var visited: std.AutoHashMapUnmanaged(Type, void) = .empty;
try self.checkParseTypeInner(ty, .none, &visited);
try self.checkTypeInner(ty, &visited);
}

fn checkParseTypeInner(
fn checkTypeInner(
self: *LowerZon,
ty: Type,
opt_ancestor: InternPool.Index,
visited: *std.AutoHashMapUnmanaged(Type, void),
) !void {
const pt = self.sema.pt;

// XXX: wait what if it contains it already but the thing it contains is optional?? need a bool
// for whether or not it's optional or something...or to avoid this some other way. or we record
// a bool in the hashmap and reivist. or whatever.
// ah maybe we only check this when at a pointer? no doesn't help.
// XXX: make sure we have tests for it being ALLOWED to nest the right ways
// Check if we're a nested optional before we possibly return early due to recursion
if (ty.zigTypeTag(pt.zcu) == .optional) {
var curr = ty.optionalChild(pt.zcu);
while (true) {
switch (curr.zigTypeTag(pt.zcu)) {
.optional => return self.failUnsupportedResultType(
"nested optionals not available in ZON; result type contains '{}'",
.{ty.fmt(pt)},
),
.pointer => {
const ptr_info = curr.ptrInfo(pt.zcu);
if (ptr_info.flags.size != .one) break;
curr = .fromInterned(ptr_info.child);
},
else => break,
}
}
}

// Check/update the visited list to avoid infinite recursion
const gop = try visited.getOrPut(pt.zcu.gpa, ty);
if (gop.found_existing) return;

// Check if this type is supported, recurse if needed
switch (ty.zigTypeTag(pt.zcu)) {
.bool,
.int,
Expand Down Expand Up @@ -102,14 +116,12 @@ fn checkParseTypeInner(
.pointer => {
const ptr_info = ty.ptrInfo(pt.zcu);
switch (ptr_info.flags.size) {
.one => try self.checkParseTypeInner(
.one => try self.checkTypeInner(
.fromInterned(ptr_info.child),
opt_ancestor,
visited,
),
.slice => try self.checkParseTypeInner(
.slice => try self.checkTypeInner(
.fromInterned(ptr_info.child),
.none,
visited,
),
.many, .c => return self.failUnsupportedResultType(
Expand All @@ -120,7 +132,7 @@ fn checkParseTypeInner(
},
.array => {
const array_info = ty.arrayInfo(pt.zcu);
try self.checkParseTypeInner(array_info.elem_type, .none, visited);
try self.checkTypeInner(array_info.elem_type, visited);
},
.@"struct" => {
if (ty.isTuple(pt.zcu)) {
Expand All @@ -129,40 +141,32 @@ fn checkParseTypeInner(
const field_comptime_vals = tuple_info.values.get(&pt.zcu.intern_pool);
for (field_types, 0..) |field_type, i| {
if (field_comptime_vals.len == 0 or field_comptime_vals[i] == .none) {
try self.checkParseTypeInner(.fromInterned(field_type), .none, visited);
try self.checkTypeInner(.fromInterned(field_type), visited);
}
}
} else {
try ty.resolveFields(pt);
const struct_info = pt.zcu.typeToStruct(ty).?;
for (struct_info.field_types.get(&pt.zcu.intern_pool), 0..) |field_type, i| {
if (!struct_info.comptime_bits.getBit(&pt.zcu.intern_pool, i)) {
try self.checkParseTypeInner(.fromInterned(field_type), .none, visited);
try self.checkTypeInner(.fromInterned(field_type), visited);
}
}
}
},
.optional => {
if (opt_ancestor != .none) {
return self.failUnsupportedResultType(
"nested optionals not available in ZON; result type contains '{}'",
.{Type.fromInterned(opt_ancestor).fmt(pt)},
);
}
try self.checkParseTypeInner(ty.optionalChild(pt.zcu), ty.toIntern(), visited);
},
.optional => try self.checkTypeInner(ty.optionalChild(pt.zcu), visited),
.@"union" => {
try ty.resolveFields(pt);
const union_info = pt.zcu.typeToUnion(ty).?;
for (union_info.field_types.get(&pt.zcu.intern_pool)) |field_type| {
if (field_type != .void_type) {
try self.checkParseTypeInner(.fromInterned(field_type), .none, visited);
try self.checkTypeInner(.fromInterned(field_type), visited);
}
}
},
.vector => {
const vector_info = pt.zcu.intern_pool.indexToKey(ty.toIntern()).vector_type;
try self.checkParseTypeInner(.fromInterned(vector_info.child), .none, visited);
try self.checkTypeInner(.fromInterned(vector_info.child), visited);
},
}
}
Expand Down Expand Up @@ -221,7 +225,7 @@ fn lowerExpr(self: *LowerZon, node: Zoir.Node.Index, res_ty: Type) CompileError!
.pointer => {
const ptr_info = res_ty.ptrInfo(pt.zcu);
if (ptr_info.flags.size == .one) {
display_ty = .fromInterned(display_ty.ptrInfo(pt.zcu).child);
display_ty = .fromInterned(ptr_info.child);
} else {
break;
}
Expand Down
11 changes: 11 additions & 0 deletions test/cases/compile_errors/@import_zon_bad_type.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@ export fn testTaggedUnionVoid() void {
_ = f;
}

export fn testVisited() void {
const V = struct {
?f32, // Adds `?f32` to the visited list
??f32, // `?f32` is already visited, we need to detect the nested opt anyway
f32,
};
const f: V = @import("zon/neg_inf.zon");
_ = f;
}

// error
// imports=zon/neg_inf.zon
//
Expand All @@ -85,6 +95,7 @@ export fn testTaggedUnionVoid() void {
// tmp.zig:42:29: error: nested optionals not available in ZON; result type contains '??u8'
// tmp.zig:47:30: error: nested optionals not available in ZON; result type contains '?*?u8'
// tmp.zig:52:32: error: nested optionals not available in ZON; result type contains '?*?*u8'
// tmp.zig:82:26: error: nested optionals not available in ZON; result type contains '??f32'
// neg_inf.zon:1:1: error: expected type 'tmp.testComptimeField__struct_471'
// tmp.zig:32:61: note: imported here
// neg_inf.zon:1:1: error: expected type '@Type(.enum_literal)'
Expand Down

0 comments on commit 196acb0

Please sign in to comment.