Skip to content

slice of allowzero pointer is rejected in for #4361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
LemonBoy opened this issue Feb 1, 2020 · 3 comments · Fixed by #4454
Closed

slice of allowzero pointer is rejected in for #4361

LemonBoy opened this issue Feb 1, 2020 · 3 comments · Fixed by #4454
Labels
bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend.
Milestone

Comments

@LemonBoy
Copy link
Contributor

LemonBoy commented Feb 1, 2020

Reduced from clashos, this may be a regression ¯\_(ツ)_/¯

export fn _start() void {
    var a = [4]u32{ 1, 2, 3, 4 };
    var b = @ptrCast([*]allowzero u32, &a);

    for (b[0..4]) |ch| {
        if (ch > 4) @panic("");
    }
}
const builtin = @import("builtin");
pub fn panic(msg: []const u8, error_return_trace: ?*builtin.StackTrace) noreturn {
    while (true) {}
}
@mikdusan
Copy link
Member

mikdusan commented Feb 6, 2020

This looks to be a regression by #4152. We actually had a reduction that I later marked as off-topic to the PR, but I think it was just side-stepped. This comment is relevant: #4152 (comment)

@fengb
Copy link
Contributor

fengb commented Feb 6, 2020

Forcing a pointer capture works around the compiler error: for (b[0..4]) |*ch| {

The for loop capture seems to be trying to coerce into a non-allowzero pointer:

error: expected type '*const u32', found '*allowzero u32'

@LemonBoy
Copy link
Contributor Author

LemonBoy commented Feb 6, 2020

The problem lies in how the compiler lowers the loop. The ch payload is desugared as a var ch: <elem-type> whose base backing memory (IrInstSrcDeclVar->ptr) is the i-th element of the array/slice/pointer it's looping on. The index i is increased every loop and so every time ch is dereferenced in the loop body the correct value is used.

Of course this trick falls short once you take into account that a pointer may be allowzero.

@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend. labels Feb 10, 2020
@andrewrk andrewrk added this to the 0.6.0 milestone Feb 10, 2020
LemonBoy added a commit to LemonBoy/zig that referenced this issue Feb 12, 2020
Instead of changing the backing pointer of the variable holding the
iteration value we now load the value from the iterator and store it
into a temporary variable.

This pattern is slightly less efficient when compiled w/o any
optimization but it's always correct, even when the backing pointer is
`allowzero`.

Fixes ziglang#4361
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants