zig icon indicating copy to clipboard operation
zig copied to clipboard

std.testing.refAllDeclsRecursive: Stop recursive loops

Open elijahimmer opened this issue 1 year ago • 0 comments

I didn't see much on a contributing guide, so I tried to work off of others. Tell me if I missed anything I need. And thanks for reviewing.

Old behavior:

Whenever a type referenced itself, like

pub const A = struct {
    pub const D = A;
};

refAllDeclsRecursive would crash the test with a status code 11 because it would recurs endlessly.

More complex recursion like two things referencing each other would do the same.

pub const A = struct {
    pub const D = A;
    pub const C = B;
};

pub const B = struct {
    pub const C = A;
};

This happens often in things like std.Thread, where the whole file is the struct, so it declares pub const Thread = @This(), and generally when things link to each other publically.

New Behavior: This recurs down the whole tree and keeps track of the types as it goes. This will go over types multiple times, but it will not recurs endlessly like now it is currently. The only way I could think of to get it to only recurs down each tree once was with a array or comptime allocation, and that had more limitations (or wasn't possible yet).

Limitations: ~~(potentially) slower compile times on large projects using this, but it shouldn't be very much. (and only during tests)~~ It would slow test compile times, but it would not lead to a compiler crash when it recurses too much like the old method.

I did some tests, but I wasn't able to do too many, so someone please help with that.

I am also not good at zig yet, so any ideas on how to improve it are welcome.

I don't know if this is quite a good fit for the standard library, but it doesn't do much more than the function did before, but it is far more useful and (shouldn't) crash if you have something referencing itself in it's tree.

Tests: I made a more complicated example, and it seemed to reference everything (I checked with @compileError()).

pub const A = struct {
    pub const Z = A;
    pub const W = B;
    pub const F = struct {
        pub const U = A;
    };
};

pub const B = struct {
    pub const Z = A;
    pub const W = C;
};

pub const C = struct {
    pub const Z = A;
    pub const W = D;
};

pub const D = struct {
    pub const Z = E;
    pub const W = D;
};

pub const E = struct {
    pub const D = A;
    pub const W = E;
};

Again, if they are very large and deep trees, it would likely be slow to compile the test, but I don't know any way around that, or making this better.

Thanks, this is a wonderful language and I would like to help out where I can.

I also considered just making it so the struct doesn't recurse into itself by doing if (@field(T, decl.name) != T) refAllDeclsRecursive(...) which wouldn't require another function, but it wasn't much more do this and stop all infinite recursions (I could think of)

elijahimmer avatar Apr 30 '24 03:04 elijahimmer