zig icon indicating copy to clipboard operation
zig copied to clipboard

zig-reduce enhancements

Open Inve1951 opened this issue 1 year ago • 10 comments

Adds support for local variable is never mutated.

Adds fixups for unused captures and unsused loop and block labels.

Discarding of if/while/for/else/switch playloads is performed in-place, i.e. by capturing into _. There's one exception to this and that's unbounded for-ranges:

for (something, 0..) |_, i| {
    _ = i;
    break;
}

This is because those must not be captured into _ - it's a compile error.

Discarding of catch error payloads is performed by omitting the whole capture.

Adds a fixup for unreachable code.

E.g. a branch reduction could result in 2 return statements:

if (true)
    return a;
return b;

Adds a fixup for break/continue statements outside loops caused by branch reduction. This currently just replaces the statement with unreachable.

Adds a fixup for unknown break/continue labels caused by branch reduction. This currently just replaces the statement with unreachable.

Fixes discarding of destructured assigment identifiers. This would previously output complete nonsense.

Fixes leftover semicolon from branch reduction. E.g.:

if (true) switch (c)
    else => {},
};

could get reduced to

switch (c)
    else => {},
};

Now the trailing semicolon won't be emitted.

Finally, this PR also adds a validation pass after applying all fixups to ensure that they did their job. I had expected this to have a big performance impact but it's not noticable. Most time is probably spent on running the checker.

Inve1951 avatar Dec 18 '23 19:12 Inve1951

Out of curiosity: Did you 1. log files before and after reduction and/or 2. wrote a tracer for the reduction history to debug your problem? Might be a very nice addition to the logging facilities Zig already provides (zig build -h):

..
  -Dlog=[bool]                 Enable debug logging with --debug-log
..

matu3ba avatar Dec 18 '23 19:12 matu3ba

There's one exception to this and that's unbounded for-ranges: This is because those must not be captured into _ - it's a compile error.

this is only mostly true, it is a compile error to discard the capture of an unbounded counter but only when youre also iterating over something else e.g. for (arr, 0..) |e, _|. the example you gave errors regardless because you cant have an unconditionally unbounded for loop, for (0..) does not compile no matter how you capture it. i dont know if your change does anything where this would matter but i thought it should be clarified

xdBronch avatar Dec 18 '23 19:12 xdBronch

Out of curiosity: Did you 1. log files before and after reduction and/or 2. wrote a tracer for the reduction history to debug your problem?

No I have not and I'm still facing the looping issue. (zig reduce gets stuck at N/36 possible transformations for me and fails to get it any lower than that)

I think the next step is to add validation logic to ensure that the fixups actually solved all the problems they were meant to solve. I didn't wanna do this in this PR without discussion since it would mean decent overhead on most iterations.

Regarding logging the file before/after: It's unclear to me what versions to log.

Inve1951 avatar Dec 18 '23 19:12 Inve1951

There's one exception to this and that's unbounded for-ranges: This is because those must not be captured into _ - it's a compile error.

this is only mostly true, it is a compile error to discard the capture of an unbounded counter but only when youre also iterating over something else e.g. for (arr, 0..) |e, _|. the example you gave errors regardless because you cant have an unconditionally unbounded for loop, for (0..) does not compile no matter how you capture it. i dont know if your change does anything where this would matter but i thought it should be clarified

Noted, thanks for clarifying. I was not aware.

This shouldn't change anything here, should still work. At first I tried to strip out the 0.. and got halfway there but it was already really messy and error prone at that point so I switched to the current approach.

Inve1951 avatar Dec 18 '23 19:12 Inve1951

The CI failure is the flaky TCP server test.

Inve1951 avatar Dec 19 '23 11:12 Inve1951

The stack trace looks very broken, but one can guess that the error is in os.accept and some unreachable is reached

Install the project...
-- Install configuration: "Release"
++ pwd
+ stage3/bin/zig build test docs --zig-lib-dir /Users/runner/work/zig/zig/build/../lib -Denable-macos-sdk -Dstatic-llvm -Dskip-non-native --search-prefix /Users/runner/zig+llvm+lld+clang-x86_64-macos-none-0.12.0-dev.467+0345d7866
run test std-native-ivybridge-ReleaseSafe-single: error: 'test.non-blocking tcp server' failed: /Users/runner/work/zig/zig/lib/std/os.zig:0:0: 0x105c3fd5a in accept (test)
/Users/runner/work/zig/zig/lib/std/net.zig:2033:13: 0x105c3fd62 in accept (test)
            return err;
            ^
/Users/runner/work/zig/zig/lib/std/os.zig:0:0: 0x105c3fd5a in accept (test)
/Users/runner/work/zig/zig/lib/std/net.zig:2033:13: 0x105c3fd62 in accept (test)
            return err;
            ^
/Users/runner/work/zig/zig/lib/std/net/test.zig:0:0: 0x105c3fa49 in test.non-blocking tcp server (test)
run test std-native-ivybridge-ReleaseSafe-single: error: while executing test 'test.ampersand', the following test command failed:
/Users/runner/work/zig/zig/build/zig-local-cache/o/c6a10302cf28730bb0a6aec75c00f7d3/test --listen=- 
Build Summary: 5380/5514 steps succeeded; 131 skipped; 1 failed; 46052/48334 tests passed; 2281 skipped; 1 failed (disable with --summary none)
test transitive failure
+- test-std transitive failure
   +- run test std-native-ivybridge-ReleaseSafe-single 2485/2588 passed, 1 failed, 102 skipped
error: the following build command failed with exit code 1:
/Users/runner/work/zig/zig/build/zig-local-cache/o/d4f344701f1c5fa7022a20e1166480b2/build /Users/runner/work/zig/zig/build/stage3/bin/zig /Users/runner/work/zig/zig /Users/runner/work/zig/zig/build/zig-local-cache /Users/runner/work/zig/zig/build/zig-global-cache --seed 0x3b2b3667 test docs --zig-lib-dir /Users/runner/work/zig/zig/build/../lib -Denable-macos-sdk -Dstatic-llvm -Dskip-non-native --search-prefix /Users/runner/zig+llvm+lld+clang-x86_64-macos-none-0.12.0-dev.467+0345d7866

Possible cases

            switch (errno(rc)) {
                .SUCCESS => {
                    break @as(socket_t, @intCast(rc));
                },
                .INTR => continue,
                .AGAIN => return error.WouldBlock,
                .BADF => unreachable, // always a race condition
                .CONNABORTED => return error.ConnectionAborted,
                .FAULT => unreachable,
                .INVAL => return error.SocketNotListening,
                .NOTSOCK => unreachable,
                .MFILE => return error.ProcessFdQuotaExceeded,
                .NFILE => return error.SystemFdQuotaExceeded,
                .NOBUFS => return error.SystemResources,
                .NOMEM => return error.SystemResources,
                .OPNOTSUPP => unreachable,
                .PROTO => return error.ProtocolFailure,
                .PERM => return error.BlockedByFirewall,
                else => |err| return unexpectedErrno(err),
            }

If I had to guess, I'd think that OPNOTSUPP may not be unreachable listed here https://github.com/apple/darwin-xnu/blob/main/bsd/man/man2/accept.2. I'll try to reproduce in CI due to absence of hardware this evening.

matu3ba avatar Dec 19 '23 14:12 matu3ba

It's almost there.

Can you additionally share your test case, and how these changes benefitted it?

zig reduce gets stuck at N/36 possible transformations for me and fails to get it any lower than that

Also I think this should be fixed before any more enhancements such as this are merged. Can you share your test case that is causing this problem?

This PR does not fix the looping problem. Trying to determine the cause, seeing loads of found other ZIR error: ... getting logged, I turned those into panics and fixed them one by one. That's this PR.

It is also the project I tested with but it isn't in a state where I want to make it public. I'll send you a zip file on discord.

Inve1951 avatar Jan 14 '24 08:01 Inve1951

Also tested like so:

zig build-exe checker.zig
zig reduce --skip-smoke-test --seed 43 ./checker reduce-test.zig
// checker.zig
pub fn main() u8 {
    return 2;
}
reduce-test.zig
// reduce-test.zig
pub fn main() void {
    for (0..1) |_|
        for (0..1) |_| {};
    if (true)
        if (true) {};
    if (false)
        if (false) {};
    if (true)
        if (false) {};
    if (false)
        if (true) {};
    while (true)
        while (true) {};
    while (false)
        while (false) {};
    while (true)
        while (false) {};
    while (false)
        while (true) {};
    {}
    {
        {}
    }
    blk: {
        break :blk;
    }
    blk: {
        {
            break :blk;
        }
    }
    for (0..1) |_|
        for (0..1) |_| {} else {};
    if (true)
        if (true) {} else {};
    if (false)
        if (false) {} else {};
    if (true)
        if (false) {} else {};
    if (false)
        if (true) {} else {};
    while (true)
        while (true) {} else {};
    while (false)
        while (false) {} else {};
    while (true)
        while (false) {} else {};
    while (false)
        while (true) {} else {};

    for (0..1) |_| {
        for (0..1) |_| {}
    } else {}
    if (true) {
        if (true) {}
    } else {}
    if (false) {
        if (false) {}
    } else {}
    if (true) {
        if (false) {}
    } else {}
    if (false) {
        if (true) {}
    } else {}
    while (true) {
        while (true) {}
    } else {}
    while (false) {
        while (false) {}
    } else {}
    while (true) {
        while (false) {}
    } else {}
    while (false) {
        while (true) {}
    } else {}

    if (true) {
        if (true) {}
    } else |_| {}
    if (false) {
        if (false) {}
    } else |_| {}
    if (true) {
        if (false) {}
    } else |_| {}
    if (false) {
        if (true) {}
    } else |_| {}
    while (true) {
        while (true) {
            break;
        }
        continue;
    } else |_| {}
    while (false) {
        while (false) {
            continue;
        }
        break;
    } else |_| {}
    while (true) {
        while (false) {}
    } else |_| {}
    while (false) {
        while (true) {}
    } else |_| {}

    if (true)
        if (true) {} else |_| {};
    if (false)
        if (false) {} else |_| {};
    if (true)
        if (false) {} else |_| {};
    if (false)
        if (true) {} else |_| {};
    while (true)
        while (true) {} else |_| {};
    while (false)
        while (false) {} else |_| {};
    while (true)
        while (false) {} else |_| {};
    while (false)
        while (true) {} else |_| {};

    _ = for (0..1) |n|
        break n;
    _ = for (0..1) |n| {
        break n;
    };
    _ = for (0..1) |n|
        break n
    else
        42;
    _ = for (0..1) |n| {
        break n;
    } else 42;
    _ = for ("abc", 0..) |c, i|
        break c + i;
    _ = for ("abc", 0..) |c, i| {
        break c + i;
    };
    _ = for ("abc", 0..) |c, i|
        break c + i
    else
        42;
    _ = for ("abc", 0..) |c, i| {
        break c + i;
    } else 42;

    for (0..1) |_|
        switch (42) {
            _ => {},
        };
    if (true)
        switch (42) {
            _ => {},
        };
    while (true)
        switch (42) {
            _ => {},
        };

    for (0..1) |_| {} else switch (42) {
        _ => {},
    }
    if (true) {} else switch (42) {
        _ => {},
    }
    while (true) {} else switch (42) {
        _ => {},
    }

    switch (42) {
        _ => if (true) {} else {},
    }
    _ = switch (42) {
        _ => if (true) {} else {},
    };

    switch (42) {
        _ => if (true) {} else |_| {},
    }
    _ = switch (42) {
        _ => if (true) {} else |_| {},
    };

    switch (42) {
        _ => for (0..1) |_| {} else {},
    }
    _ = switch (42) {
        _ => for (0..1) |_| {} else {},
    };

    switch (42) {
        _ => while (true) {} else {},
    }
    _ = switch (42) {
        _ => while (true) {} else {},
    };

    switch (42) {
        _ => while (true) {} else |_| {},
    }
    _ = switch (42) {
        _ => while (true) {} else |_| {},
    };

    switch (42) {
        _ => while (true) {},
    }
    _ = switch (42) {
        _ => while (true) {},
    };

    var a: u8 = undefined;
    if (false) a = 1;
    if (true) {
        a = 2;
    }
    const b = a;
    if (false) {} else if (b == 2) {} else {}

    if (true) switch (42) {
        _ => while (true) {},
    };
    _ = if (true) switch (42) {
        _ => b,
    };

    switch (42) {
        41 => |c| {
            _ = c + 1;
        },
        _ => {},
    }
    if (a) |c| {
        _ = c + 1;
    }
    while (a) |c| {
        _ = c + 1;
    }
    a catch |c| {
        _ = c + 1;
    };
    _ = a catch |c| {
        _ = c + 1;
    };

    const c, const d = a;
    {
        _ = c;
        _ = d;
    }

    if (true) blk: {
        if (false) break :blk;
    }

    w: while (true) blk: {
        if (false) break :blk;
        if (true) continue :w;
        if (true) break :w;
    }

    f: for (0..1) |n| blk: {
        if (false) break :blk;
        if (true) continue :f;
        if (true) break :f n;
    }

    w: while (true) blk: {
        switch (42) {
            false => break :blk,
            true => continue :w,
            _ => break :w,
        }
    }

    f: for (0..1) |n| blk: {
        switch (42) {
            false => break :blk n,
            true => continue :f,
            _ => break :f,
        }
    }

    if (true)
        return;
    return;
}

This found some more edge-cases that I fixed in the recent commits. OP is up to date.

Expected:

  • no panics
  • after run to completion reduce-test.zig contents have been restored unaltered

Inve1951 avatar Feb 09 '24 16:02 Inve1951

Note: only not marked ready for review because of

Also I think this should be fixed before any more enhancements such as this are merged.

I believe that the additional fixups allow zig-reduce to produce more minimal output in cases where it would previously encounter errors after reductions, making these changes valuable regardless.

Inve1951 avatar Mar 15 '24 02:03 Inve1951

Sorry for taking so long to get to this. I plan to rebase it myself when I have time.

andrewrk avatar May 09 '24 23:05 andrewrk