ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.97k stars 2.55k forks source link

Spurious "dependency loop detected" and "missing struct field" errors #17255

Open dweiller opened 1 year ago

dweiller commented 1 year ago

Zig Version

0.12.0-dev.494+a8d2ed806

Steps to Reproduce and Observed Behavior

Clone https://github.com/dweiller/zimalloc and run

git checkout d577c4a3
zig build

~I will try to make a smaller repro case and bisect the Zig commit introducing it when I get time (and llvm17 finishes compiling).~ A minimal repro case is below in https://github.com/ziglang/zig/issues/17255#issuecomment-1763884168.

Here is the full error message:

Build Summary: 1/4 steps succeeded; 1 failed (disable with --summary none)
test transitive failure
└─ run test transitive failure
   └─ zig test Debug native 3 errors
src/Segment.zig:9:5: error: dependency loop detected
pub const Ptr = *align(segment_alignment) @This();
~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/zimalloc.zig:50:32: error: missing struct field: thread_heaps
    var gpa = Allocator(config){};
              ~~~~~~~~~~~~~~~~~^~
src/zimalloc.zig:50:32: note: missing struct field: thread_heaps_lock
src/allocator.zig:8:12: note: struct 'allocator.Allocator(.{.thread_data_prealloc = 128, .thread_safe = true, .safety_checks = false})' declared here
    return struct {
           ^~~~~~
src/allocator.zig:10:79: error: missing struct field: prealloc_segment
        thread_heaps: std.SegmentedList(Heap, config.thread_data_prealloc) = .{},
                                                                             ~^~
src/allocator.zig:10:79: note: missing struct field: dynamic_segments
src/allocator.zig:10:79: note: missing struct field: len
/path/to/zig/zig-linux-x86_64-0.12.0-dev.494+a8d2ed806/lib/std/segmented_list.zig:79:12: note: struct 'segmented_list.SegmentedList(Heap,128)' declared here
    return struct {
           ^~~~~~

These errors are not encountered when compiling with 0.12.0-dev.464+a63a1c5cb (the commit that merged llvm17, though I can't find the tarball for this to test locally now, but my CI worked using it), so it seems the issue was introduced between that commit and when #17172 got merged.

The dependency loop issue shouldn't be the same issue as other dependency issue in the issue tracker (e.g. #16932, #16419, #14353) which are all about function pointers and affect earlier compiler versions than 0.12.0-dev.464+a63a1c5cb.

Expected Behavior

The code should compile as far as I can tell - the dependency loop is (according to the compile error) caused by a decl that is creates a type alias like pub const Ptr = *align(alignment) @This(); and the missing struct fields all have default values.

Vexu commented 1 year ago

a8d2ed806 merged #17172 which is likely to be the cause.

dweiller commented 1 year ago

I haven't managed to produce a minimal repro case yet, but have found a patch to workaround the issue:

--- a/src/Heap.zig
+++ b/src/Heap.zig
@@ -1,6 +1,6 @@
 thread_id: std.Thread.Id,
 pages: [size_class_count]Page.List,
-segments: ?Segment.Ptr,
+segments: ?*align(constants.segment_alignment) Segment,
 huge_allocations: HugeAllocTable,

Note that Segment.Ptr is defined in Segment.zig like this:

const pub const Ptr = *align(alignment) @This();
const alignment = constants.segment_alignment;

I have also tested the changes in #17490 as the described changes to structs seemed possibly related, but unfortunately that PR doesn't seem to affect this issue (as of commit 3a8932445 in the PR).

dweiller commented 1 year ago

The error seems to be related to the use of the align property on the pointers. Instead of the above workaround, the compiler does not error with this patch:

--- a/src/Segment.zig
+++ b/src/Segment.zig
@@ -6,8 +6,8 @@ heap: *Heap,
 next: ?Ptr,
 prev: ?Ptr,

-pub const Ptr = *align(segment_alignment) @This();
-pub const ConstPtr = *align(segment_alignment) const @This();
+pub const Ptr = *@This();
+pub const ConstPtr = *const @This();

 pub const PageSize = union(enum) {
     small,
@@ -92,7 +92,7 @@ fn allocateSegment() ?*align(segment_alignment) [segment_size]u8 {
 }

 fn deallocateSegment(self: Ptr) void {
-    const ptr: *align(segment_alignment) [segment_size]u8 = @ptrCast(self);
+    const ptr: *align(segment_alignment) [segment_size]u8 = @ptrCast(@alignCast(self));
     huge_alignment.deallocate(ptr);
 }

With the added @alignCast, the compiler errors with the align(segment_alignment) property on Ptr and ConstPtr and does not error if the align(segment_alignment)s are removed.

dweiller commented 1 year ago

I have managed to make a minimal repro case:

const Slab = struct {
    pub const Ptr = *align(1 << 16) Slab;

    pub const List = struct {
        head: ?Ptr = null,
    };

    next: Ptr,
};

const MeshingPool = struct {
    partial_slabs: Slab.List = .{},
};

test {
    _ = MeshingPool{};
}

If you run zig test on the above the compiler reports:

repro.zig:2:9: error: dependency loop detected
    pub const Ptr = *align(1 << 16) Slab;
    ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I have 6 different 1-line diffs that allow the above case to compile successfully:

@@ -1,5 +1,5 @@
 const Slab = struct {
-    pub const Ptr = *align(1 << 16) Slab;
+    pub const Ptr = *Slab;

     pub const List = struct {
         head: ?Ptr = null,
@@ -2,7 +2,6 @@
     pub const Ptr = *align(1 << 16) Slab;

     pub const List = struct {
-        head: ?Ptr = null,
     };
@@ -2,7 +2,7 @@
     pub const Ptr = *align(1 << 16) Slab;

     pub const List = struct {
-        head: ?Ptr = null,
+        head: ?*align(1 << 16) Slab = null,
     };

     next: Ptr,
@@ -5,7 +5,6 @@
         head: ?Ptr = null,
     };

-    next: Ptr,
 };
@@ -5,7 +5,7 @@
         head: ?Ptr = null,
     };

-    next: Ptr,
+    next: *align(1 << 16) Slab,
 };

 const MeshingPool = struct {
@@ -13,5 +13,5 @@
 };

 test {
-    _ = MeshingPool{};
+    _ = MeshingPool;
 }

(I wasn't sure if the it's surprising that the last diff changes whether it compiles or not).